Hi Simon,
On Sun, May 08, 2022 at 04:39:24AM -0600, Simon Glass wrote:
> At present tag numbers are only allocated for non-core data, meaning that
> the 'core' data, like priv and plat, are accessed through dedicated
> functions.
>
> For debugging and consistency it is convenient to use tags for this 'core'
> data too. Add support for this, with new tag numbers and functions to
> access the pointer and size for each.
>
> Update one of the test drivers so that the uclass-private data can be
> tested here.
>
> There is some code duplication with functions like device_alloc_priv() but
> this is not addressed for now. At some point, some rationalisation may
> help to reduce code size, but more thought it needed on that.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> drivers/core/device.c | 65 +++++++++++++++++++++++++++++++++
> drivers/misc/test_drv.c | 4 ++-
> include/dm/device.h | 25 +++++++++++++
> include/dm/tag.h | 13 ++++++-
> test/dm/core.c | 80 +++++++++++++++++++++++++++++++++++++++++
> tools/dtoc/test_dtoc.py | 4 +++
> 6 files changed, 189 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 3ab2583df38..d7936a46732 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -680,6 +680,71 @@ void *dev_get_parent_priv(const struct udevice *dev)
> return dm_priv_to_rw(dev->parent_priv_);
> }
>
> +void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag)
Why not (enhance and) re-use dev_tag_get_ptr()?
Both functions, at least, share the syntax and even the semantics
from user's viewpoint.
> +{
> + switch (tag) {
> + case DM_TAG_PLAT:
> + return dev_get_plat(dev);
> + case DM_TAG_PARENT_PLAT:
> + return dev_get_parent_plat(dev);
> + case DM_TAG_UC_PLAT:
> + return dev_get_uclass_plat(dev);
> + case DM_TAG_PRIV:
> + return dev_get_priv(dev);
> + case DM_TAG_PARENT_PRIV:
> + return dev_get_parent_priv(dev);
> + case DM_TAG_UC_PRIV:
> + return dev_get_uclass_priv(dev);
> + default:
> + return NULL;
> + }
> +}
> +
> +int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag)
> +{
> + const struct udevice *parent = dev_get_parent(dev);
> + const struct uclass *uc = dev->uclass;
> + const struct uclass_driver *uc_drv = uc->uc_drv;
> + const struct driver *parent_drv = NULL;
> + int size = 0;
> +
> + if (parent)
> + parent_drv = parent->driver;
> +
> + switch (tag) {
> + case DM_TAG_PLAT:
> + size = dev->driver->plat_auto;
> + break;
> + case DM_TAG_PARENT_PLAT:
> + if (parent) {
> + size = parent_drv->per_child_plat_auto;
> + if (!size)
> + size =
> parent->uclass->uc_drv->per_child_plat_auto;
> + }
> + break;
> + case DM_TAG_UC_PLAT:
> + size = uc_drv->per_device_plat_auto;
> + break;
> + case DM_TAG_PRIV:
> + size = dev->driver->priv_auto;
> + break;
> + case DM_TAG_PARENT_PRIV:
> + if (parent) {
> + size = parent_drv->per_child_auto;
> + if (!size)
> + size = parent->uclass->uc_drv->per_child_auto;
> + }
> + break;
> + case DM_TAG_UC_PRIV:
> + size = uc_drv->per_device_auto;
> + break;
> + default:
> + break;
> + }
> +
> + return size;
> +}
> +
> static int device_get_device_tail(struct udevice *dev, int ret,
> struct udevice **devp)
> {
> diff --git a/drivers/misc/test_drv.c b/drivers/misc/test_drv.c
> index b6df1189032..927618256f0 100644
> --- a/drivers/misc/test_drv.c
> +++ b/drivers/misc/test_drv.c
> @@ -108,7 +108,9 @@ UCLASS_DRIVER(testbus) = {
> .child_pre_probe = testbus_child_pre_probe_uclass,
> .child_post_probe = testbus_child_post_probe_uclass,
>
> - /* This is for dtoc testing only */
> + .per_device_auto = sizeof(struct dm_test_uclass_priv),
> +
> + /* Note: this is for dtoc testing as well as tags*/
> .per_device_plat_auto = sizeof(struct dm_test_uclass_plat),
> };
>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 5bdb10653f8..12c6ba37ff3 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -11,6 +11,7 @@
> #define _DM_DEVICE_H
>
> #include <dm/ofnode.h>
> +#include <dm/tag.h>
> #include <dm/uclass-id.h>
> #include <fdtdec.h>
> #include <linker_lists.h>
> @@ -546,6 +547,30 @@ void *dev_get_parent_priv(const struct udevice *dev);
> */
> void *dev_get_uclass_priv(const struct udevice *dev);
>
> +/**
> + * dev_get_attach_ptr() - Get the value of an attached pointed tag
> + *
> + * The tag is assumed to hold a pointer, if it exists
> + *
> + * @dev: Device to look at
> + * @tag: Tag to access
> + * @return value of tag, or NULL if there is no tag of this type
> + */
> +void *dev_get_attach_ptr(const struct udevice *dev, enum dm_tag_t tag);
> +
> +/**
> + * dev_get_attach_size() - Get the size of an attached tag
> + *
> + * Core tags have an automatic-allocation mechanism where the allocated size
> is
> + * defined by the device, parent or uclass. This returns the size associated
> + * with a particular tag
> + *
> + * @dev: Device to look at
> + * @tag: Tag to access
> + * @return size of auto-allocated data, 0 if none
> + */
> +int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag);
> +
> /**
> * dev_get_parent() - Get the parent of a device
> *
> diff --git a/include/dm/tag.h b/include/dm/tag.h
> index 54fc31eb153..9cb5d68f0a3 100644
> --- a/include/dm/tag.h
> +++ b/include/dm/tag.h
> @@ -13,8 +13,19 @@
> struct udevice;
>
> enum dm_tag_t {
> + /* Types of core tags that can be attached to devices */
> + DM_TAG_PLAT,
> + DM_TAG_PARENT_PLAT,
> + DM_TAG_UC_PLAT,
> +
> + DM_TAG_PRIV,
> + DM_TAG_PARENT_PRIV,
> + DM_TAG_UC_PRIV,
> + DM_TAG_DRIVER_DATA,
> + DM_TAG_ATTACH_COUNT,
> +
> /* EFI_LOADER */
> - DM_TAG_EFI = 0,
> + DM_TAG_EFI = DM_TAG_ATTACH_COUNT,
What does DM_TAG_ATTACH_COUNT mean?
-Takahiro Akashi
>
> DM_TAG_COUNT,
> };
> diff --git a/test/dm/core.c b/test/dm/core.c
> index ebd504427d1..26e2fd56619 100644
> --- a/test/dm/core.c
> +++ b/test/dm/core.c
> @@ -1275,3 +1275,83 @@ static int dm_test_uclass_find_device(struct
> unit_test_state *uts)
> return 0;
> }
> DM_TEST(dm_test_uclass_find_device, UT_TESTF_SCAN_FDT);
> +
> +/* Test getting information about tags attached to devices */
> +static int dm_test_dev_get_attach(struct unit_test_state *uts)
> +{
> + struct udevice *dev;
> +
> + ut_assertok(uclass_first_device_err(UCLASS_TEST_FDT, &dev));
> + ut_asserteq_str("a-test", dev->name);
> +
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
> +
> + ut_asserteq(sizeof(struct dm_test_pdata),
> + dev_get_attach_size(dev, DM_TAG_PLAT));
> + ut_asserteq(sizeof(struct dm_test_priv),
> + dev_get_attach_size(dev, DM_TAG_PRIV));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PRIV));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_UC_PLAT));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
> +
> + return 0;
> +}
> +DM_TEST(dm_test_dev_get_attach, UT_TESTF_SCAN_FDT);
> +
> +/* Test getting information about tags attached to bus devices */
> +static int dm_test_dev_get_attach_bus(struct unit_test_state *uts)
> +{
> + struct udevice *dev, *child;
> +
> + ut_assertok(uclass_first_device_err(UCLASS_TEST_BUS, &dev));
> + ut_asserteq_str("some-bus", dev->name);
> +
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PLAT));
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_PRIV));
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PRIV));
> + ut_assertnonnull(dev_get_attach_ptr(dev, DM_TAG_UC_PLAT));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PLAT));
> + ut_assertnull(dev_get_attach_ptr(dev, DM_TAG_PARENT_PRIV));
> +
> + ut_asserteq(sizeof(struct dm_test_pdata),
> + dev_get_attach_size(dev, DM_TAG_PLAT));
> + ut_asserteq(sizeof(struct dm_test_priv),
> + dev_get_attach_size(dev, DM_TAG_PRIV));
> + ut_asserteq(sizeof(struct dm_test_uclass_priv),
> + dev_get_attach_size(dev, DM_TAG_UC_PRIV));
> + ut_asserteq(sizeof(struct dm_test_uclass_plat),
> + dev_get_attach_size(dev, DM_TAG_UC_PLAT));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PLAT));
> + ut_asserteq(0, dev_get_attach_size(dev, DM_TAG_PARENT_PRIV));
> +
> + /* Now try the child of the bus */
> + ut_assertok(device_first_child_err(dev, &child));
> + ut_asserteq_str("c-test@5", child->name);
> +
> + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PLAT));
> + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PRIV));
> + ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PRIV));
> + ut_assertnull(dev_get_attach_ptr(child, DM_TAG_UC_PLAT));
> + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PLAT));
> + ut_assertnonnull(dev_get_attach_ptr(child, DM_TAG_PARENT_PRIV));
> +
> + ut_asserteq(sizeof(struct dm_test_pdata),
> + dev_get_attach_size(child, DM_TAG_PLAT));
> + ut_asserteq(sizeof(struct dm_test_priv),
> + dev_get_attach_size(child, DM_TAG_PRIV));
> + ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PRIV));
> + ut_asserteq(0, dev_get_attach_size(child, DM_TAG_UC_PLAT));
> + ut_asserteq(sizeof(struct dm_test_parent_plat),
> + dev_get_attach_size(child, DM_TAG_PARENT_PLAT));
> + ut_asserteq(sizeof(struct dm_test_parent_data),
> + dev_get_attach_size(child, DM_TAG_PARENT_PRIV));
> +
> + return 0;
> +}
> +DM_TEST(dm_test_dev_get_attach_bus, UT_TESTF_SCAN_FDT);
> diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> index 8bac2076214..879ca2ab2bf 100755
> --- a/tools/dtoc/test_dtoc.py
> +++ b/tools/dtoc/test_dtoc.py
> @@ -618,6 +618,9 @@ u8 _denx_u_boot_test_bus_priv_some_bus[sizeof(struct
> dm_test_priv)]
> #include <dm/test.h>
> u8 _denx_u_boot_test_bus_ucplat_some_bus[sizeof(struct dm_test_uclass_plat)]
> \t__attribute__ ((section (".priv_data")));
> +#include <dm/test.h>
> +u8 _denx_u_boot_test_bus_uc_priv_some_bus[sizeof(struct dm_test_uclass_priv)]
> + __attribute__ ((section (".priv_data")));
> #include <test.h>
>
> DM_DEVICE_INST(some_bus) = {
> @@ -628,6 +631,7 @@ DM_DEVICE_INST(some_bus) = {
> \t.driver_data\t= DM_TEST_TYPE_FIRST,
> \t.priv_\t\t= _denx_u_boot_test_bus_priv_some_bus,
> \t.uclass\t\t= DM_UCLASS_REF(testbus),
> +\t.uclass_priv_ = _denx_u_boot_test_bus_uc_priv_some_bus,
> \t.uclass_node\t= {
> \t\t.prev = &DM_UCLASS_REF(testbus)->dev_head,
> \t\t.next = &DM_UCLASS_REF(testbus)->dev_head,
> --
> 2.36.0.512.ge40c2bad7a-goog
>
