[PATCH v4 1/2] dm: core: add function uclass_probe_all() to probe all devices
Vabhav Sharma (OSS)
vabhav.sharma at oss.nxp.com
Mon Nov 23 09:06:03 CET 2020
> -----Original Message-----
> From: Sean Anderson <seanga2 at gmail.com>
> Sent: Thursday, November 19, 2020 9:05 AM
> To: Vabhav Sharma (OSS) <vabhav.sharma at oss.nxp.com>;
> sjg at chromium.org; sr at denx.de
> Cc: u-boot at lists.denx.de; Varun Sethi <V.Sethi at nxp.com>;
> andre.przywara at arm.com; Vabhav Sharma <vabhav.sharma at nxp.com>
> Subject: Re: [PATCH v4 1/2] dm: core: add function uclass_probe_all() to
> probe all devices
>
> On 11/17/20 10:00 AM, Vabhav Sharma wrote:
> > From: Vabhav Sharma <vabhav.sharma at nxp.com>
> >
> > Support a common method to probe all devices associated with uclass.
> >
> > This includes data structures and code for finding the first device
> > and looping for remaining devices associated with uclasses (groups of
> > devices with the same purpose, e.g. all SERIAL ports will be in the same
> uclass).
> >
> > An example is SBSA compliant PL011 UART IP, where firmware does the
> > serial port initialization and prepare uart device to let the kernel
> > use it for sending and reveiving the characters.SERIAL uclass will use
> > this function to initialize PL011 UART ports.
> >
> > The feature is enabled with CONFIG_DM.
> >
> > Signed-off-by: Vabhav Sharma <vabhav.sharma at nxp.com>
> > Reviewed-by: Stefan Roese <sr at denx.de>
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> > --
> > v4:
> > Incorporated review comments of Simon
> > Removed if (dev).. conditional check
> >
> > v3:
> > Incorporated review comments of Stephan,Simon
> > Related discussion
> > https://patchwork.ozlabs.org/project/uboot/patch/1601400
> > 385-11854-1-git-send-email-vabhav.sharma at oss.nxp.com/
> > ---
> > drivers/core/uclass.c | 16 ++++++++++++++++
> > include/dm/uclass.h | 12 ++++++++++++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index
> > c3f1b73..a1dc8bb 100644
> > --- a/drivers/core/uclass.c
> > +++ b/drivers/core/uclass.c
> > @@ -792,6 +792,22 @@ int uclass_pre_remove_device(struct udevice
> *dev)
> > } #endif
> >
> > +int uclass_probe_all(enum uclass_id id) {
> > + struct udevice *dev;
> > + int ret;
> > +
> > + ret = uclass_first_device(id, &dev);
> > + if (ret || !dev)
> > + return ret;
> > +
> > + /* Scanning uclass to probe all devices */
> > + for (; dev; uclass_next_device(&dev))
>
> You must check the return value of this function.
Error check is done for first device before passing the device to uclass_next_device(),
I think of other implementation is to combine the first device check and iterating through device list of u-class as
for (ret = uclass_first_device(id, &dev); dev && !ret; ret = uclass_next_device(&dev))
;
But iteration is not required if first device is not found and current changes seems to be ok
Please share valuable feedback
>
> Also, I would suggest using a while loop instead of an empty for loop.
Please elaborate, Found for loop best suitable to use here
>
> > + ;
> > +
> > + return 0;
> > +}
> > +
> > UCLASS_DRIVER(nop) = {
> > .id = UCLASS_NOP,
> > .name = "nop",
> > diff --git a/include/dm/uclass.h b/include/dm/uclass.h index
> > 7188304..7ac0aaa 100644
> > --- a/include/dm/uclass.h
> > +++ b/include/dm/uclass.h
> > @@ -381,6 +381,18 @@ int uclass_first_device_drvdata(enum uclass_id
> > id, ulong driver_data, int uclass_resolve_seq(struct udevice *dev);
> >
> > /**
> > + * uclass_probe_all() - Probe all devices based on an uclass ID
> > + *
> > + * Every uclass is identified by an ID, a number from 0 to n-1 where
> > + n is
> > + * the number of uclasses. This function probe all devices asocciated
> > + with
>
> nit: probes associated
Ok
>
> > + * a uclass by looking its ID.
>
> nit: for its
Sure
>
> AFAICT uclass_find_next_device walks the linked-list of devices in a uclass,
> and does not care about the ID. So this documentation is incorrect.
This documentation is for new function uclass_probe_all() and understand each Uclass is identified by enum ID e.g. UCLASS_SERIAL for serial devices.
According used the statement "Every uclass is identified by an ID"
Please suggest.
>
> > + *
> > + * @id: uclass ID to look up
> > + * @return 0 if OK, other -ve on error */ int uclass_probe_all(enum
> > +uclass_id id);
> > +
> > +/**
> > * uclass_id_foreach_dev() - Helper function to iteration through devices
> > *
> > * This creates a for() loop which works through the available
> > devices in
> >
More information about the U-Boot
mailing list