[U-Boot] [RFC PATCH 16/22] dm: Add a way to indicate a preferred device within a uclass

Jon Loeliger
2014-05-27T15:42:21Z

> The preferred device can be specified with a DM_FLAG_PREFER flag or a
> 'dm,prefer' property in the device tree node.
>
> It is possible that a better approach will come to light in the future, but
> this gets around the problem as it currently stands.

Here's your clue that something isn't quite right here. Again, this looks like
the us of a DTS property to describe a SW functionality rather than describe
the HW itself.

I'm not sure what needs to be done here either, but maybe it centers on
a better understanding of *why* something is being preferred? And what
if it is "preferred", then what does that really mean? And what if two devices
are labelled as "preferred" -- Is that OK? Who checks and enforces it?

Sure, this may be a hack for now, but it needs more thought.

HTH,
jdl

On Sat, May 24, 2014 at 4:21 PM, Simon Glass <sjg@chromium.org> wrote:
> Where there are serveral device options that can be chosen, often one is
> preferred. This can normally be handled by aliases in the device tree.
>
> However, when a device can be specified either with platform data or
> with a device tree node, which one should dm use? This situation happens
> with sandbox, where we want to use the device tree version if we have
> a device tree, and fall back to the platform data version if not. We need
> this to work because without a console U-Boot will not function.
>
> The original approach was just to take the first device in the uclass and
> use that, but this does not work because the ordering is unknown.
>
> The preferred device can be specified with a DM_FLAG_PREFER flag or a
> 'dm,prefer' property in the device tree node.
>
> It is possible that a better approach will come to light in the future, but
> this gets around the problem as it currently stands.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> drivers/core/device.c | 5 +++++
> drivers/core/uclass.c | 15 ++++++++++++++-
> include/dm/device.h | 3 +++
> include/dm/uclass-internal.h | 3 ++-
> include/dm/uclass.h | 15 +++++++++++++++
> test/dm/test-fdt.c | 14 ++++++++++++++
> test/dm/test.dts | 1 +
> 7 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 2c2634e..6b2c8f9 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -10,6 +10,7 @@
> */
>
> #include <common.h>
> +#include <fdtdec.h>
> #include <malloc.h>
> #include <dm/device.h>
> #include <dm/device-internal.h>
> @@ -21,6 +22,8 @@
> #include <linux/err.h>
> #include <linux/list.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> /**
> * device_chld_unbind() - Unbind all device's children from the device
> *
> @@ -95,6 +98,8 @@ int device_bind(struct device *parent, struct driver *drv, const char *name,
> dev->parent = parent;
> dev->driver = drv;
> dev->uclass = uc;
> + if (fdtdec_get_bool(gd->fdt_blob, of_offset, "dm,prefer"))
> + dev->flags |= DM_FLAG_PREFER;
> if (!dev->platdata && drv->platdata_auto_alloc_size)
> dev->flags |= DM_FLAG_ALLOC_PDATA;
>
> diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
> index afb5fdc..d02ea5e 100644
> --- a/drivers/core/uclass.c
> +++ b/drivers/core/uclass.c
> @@ -149,7 +149,8 @@ int uclass_find_device(enum uclass_id id, int index, struct device **devp)
> return ret;
>
> list_for_each_entry(dev, &uc->dev_head, uclass_node) {
> - if (!index--) {
> + if (index == -1 ? (dev->flags & DM_FLAG_PREFER) :
> + !index--) {
> *devp = dev;
> return 0;
> }
> @@ -177,6 +178,18 @@ int uclass_get_device(enum uclass_id id, int index, struct device **devp)
> return 0;
> }
>
> +int uclass_get_preferred_device(enum uclass_id id, struct device **devp)
> +{
> + int err;
> +
> + err = uclass_get_device(id, -1, devp);
> + if (!err || err != -ENODEV)
> + return err;
> +
> + /* Nothing found, just pick the first device */
> + return uclass_get_device(id, 0, devp);
> +}
> +
> int uclass_first_device(enum uclass_id id, struct device **devp)
> {
> struct uclass *uc;
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 32650fd..d3c04ed 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -26,6 +26,9 @@ struct driver_info;
> /* DM should init this device prior to relocation */
> #define DM_FLAG_PRE_RELOC (1 << 2)
>
> +/* DM should prefer this driver when searching a uclass */
> +#define DM_FLAG_PREFER (1 << 3)
> +
> /**
> * struct device - An instance of a driver
> *
> diff --git a/include/dm/uclass-internal.h b/include/dm/uclass-internal.h
> index cc65d52..79b4d9e 100644
> --- a/include/dm/uclass-internal.h
> +++ b/include/dm/uclass-internal.h
> @@ -13,7 +13,8 @@
> /**
> * uclass_find_device() - Return n-th child of uclass
> * @id: Id number of the uclass
> - * @index: Position of the child in uclass's list
> + * @index: Position of the child in uclass's list. If -1 then the
> + * device marked with DM_FLAG_PREFER is returned, if any
> * #devp: Returns pointer to device, or NULL on error
> *
> * The device is not prepared for use - this is an internal function
> diff --git a/include/dm/uclass.h b/include/dm/uclass.h
> index ac5c147..2ca87fc 100644
> --- a/include/dm/uclass.h
> +++ b/include/dm/uclass.h
> @@ -106,6 +106,21 @@ int uclass_get(enum uclass_id key, struct uclass **ucp);
> int uclass_get_device(enum uclass_id id, int index, struct device **devp);
>
> /**
> + * uclass_get_preferred_device() - Get the preferred uclass device
> + *
> + * id: ID to look up
> + * @ucp: Returns pointer to device (there is only one per for each ID)
> + *
> + * This returns the device with the DM_FLAG_PREFER flag set, if any.
> + * Otherwise it returns the first device.
> + *
> + * The device is probed to activate it ready for use.
> + *
> + * @return 0 if OK, -ve on error
> + */
> +int uclass_get_preferred_device(enum uclass_id id, struct device **devp);
> +
> +/**
> * uclass_first_device() - Get the first device in a uclass
> *
> * @id: Uclass ID to look up
> diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
> index d6f5bb8..5dce48e 100644
> --- a/test/dm/test-fdt.c
> +++ b/test/dm/test-fdt.c
> @@ -160,3 +160,17 @@ static int dm_test_fdt_pre_reloc(struct dm_test_state *dms)
> return 0;
> }
> DM_TEST(dm_test_fdt_pre_reloc, 0);
> +
> +/* Test that autoprobe finds all the expected devices */
> +static int dm_test_prefer(struct dm_test_state *dms)
> +{
> + struct device *dev;
> +
> + ut_assertok(uclass_get_preferred_device(UCLASS_TEST_FDT, &dev));
> + ut_assert(dev);
> +
> + ut_asserteq_str("b-test", dev->name);
> +
> + return 0;
> +}
> +DM_TEST(dm_test_prefer, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> diff --git a/test/dm/test.dts b/test/dm/test.dts
> index b2eaddd..c42d4e7 100644
> --- a/test/dm/test.dts
> +++ b/test/dm/test.dts
> @@ -36,6 +36,7 @@
> reg = <3>;
> compatible = "denx,u-boot-fdt-test";
> ping-add = <3>;
> + prefer;
> };
>
> some-bus {
> --
> 1.9.1.423.g4596e3a
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot