> -----Original Message----- > From: Sean Anderson <sean...@gmail.com> > Sent: Thursday, November 19, 2020 9:05 AM > To: Vabhav Sharma (OSS) <vabhav.sha...@oss.nxp.com>; > s...@chromium.org; s...@denx.de > Cc: u-boot@lists.denx.de; Varun Sethi <v.se...@nxp.com>; > andre.przyw...@arm.com; Vabhav Sharma <vabhav.sha...@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.sha...@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.sha...@nxp.com> > > Reviewed-by: Stefan Roese <s...@denx.de> > > Reviewed-by: Simon Glass <s...@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.sha...@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 > >
