From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Simon Glass <sjg@chromium.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
Tom Rini <trini@konsulko.com>,
U-Boot Custodians <u-boot-custodians@lists.denx.de>,
Marek Vasut <marex@denx.de>,
Pavel Herrmann <morpheus.ibis@gmail.com>
Subject: Re: [PATCH 6/9] dm: core: Support accessing core tags
Date: Mon, 9 May 2022 13:52:26 +0900 [thread overview]
Message-ID: <20220509045226.GA34398@laputa> (raw)
In-Reply-To: <20220508103927.912854-7-sjg@chromium.org>
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@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
>
next prev parent reply other threads:[~2022-05-09 4:52 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-08 10:39 [PATCH 0/9] dm: core: Support collecting and reporting stats Simon Glass
2022-05-08 10:39 ` [PATCH 1/9] dm: core: Rename dm_dump_all() Simon Glass
2022-06-28 13:38 ` Simon Glass
2022-05-08 10:39 ` [PATCH 2/9] dm: core: Sort dm subcommands Simon Glass
2022-06-28 13:38 ` Simon Glass
2022-05-08 10:39 ` [PATCH 3/9] dm: core: Fix addresses in the dm static command Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-05-08 10:39 ` [PATCH 4/9] dm: core: Add documentation for the dm command Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-05-08 10:39 ` [PATCH 5/9] dm: core: Switch the testbus driver to use a new struct Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-05-08 10:39 ` [PATCH 6/9] dm: core: Support accessing core tags Simon Glass
2022-05-09 4:52 ` AKASHI Takahiro [this message]
2022-06-28 13:37 ` Simon Glass
2022-06-28 14:18 ` AKASHI Takahiro
2022-07-05 12:39 ` Tom Rini
2022-07-05 13:27 ` Simon Glass
2022-07-06 1:27 ` AKASHI Takahiro
2022-07-06 8:38 ` Simon Glass
2022-05-08 10:39 ` [PATCH 7/9] dm: core: Add a way to collect memory usage Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-05-08 10:39 ` [PATCH 8/9] dm: core: Add a command to show driver model statistics Simon Glass
2022-06-28 13:37 ` Simon Glass
2022-05-08 10:39 ` [PATCH 9/9] dm: spl: Allow SPL to show memory usage Simon Glass
2022-06-28 13:37 ` Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220509045226.GA34398@laputa \
--to=takahiro.akashi@linaro.org \
--cc=marex@denx.de \
--cc=morpheus.ibis@gmail.com \
--cc=sjg@chromium.org \
--cc=trini@konsulko.com \
--cc=u-boot-custodians@lists.denx.de \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.