[PATCH 6/9] dm: core: Support accessing core tags

AKASHI Takahiro takahiro.akashi at linaro.org
Mon May 9 06:52:26 CEST 2022


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 <sjg at 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 at 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
> 


More information about the U-Boot-Custodians mailing list