| Message ID | 1400966481-14131-17-git-send-email-sjg@chromium.org |
|---|---|
| State | RFC |
| Headers | show |
> 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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Jon, On 27 May 2014 09:42, Jon Loeliger <loeliger@gmail.com> wrote: >> 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. Agreed, but I've done all the thinking I can so far, and this is the result. Another way of doing this would be to designate the platform data device as a 'backup' device, only to be used if we don't get a similar device from the device tree. But I'm not sure that it would be better that way. This is not intended to be a widely-used mechanism - it just deals with the issue of device tree vs. not. My preference would be to require that all devices be represented by a device tree with driver model. However, I think that would discourage adoption in the short term. Also for sandbox we really do want to test all the code, and so always having a device tree would reduce test coverage. Regards, Simon
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 {
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(-)