On Sat, Dec 18, 2021 at 11:55:41AM +0100, Heinrich Schuchardt wrote: > On 12/14/21 08:01, AKASHI Takahiro wrote: > > Hi Simon, > > > > Thank you for your review on this series. > > > > On Mon, Dec 13, 2021 at 05:51:40AM -0700, Simon Glass wrote: > > > Hi Takahiro, > > > > > > On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.aka...@linaro.org> > > > wrote: > > > > > > > > This function will be commonly used in block device drivers > > > > in the succeeding patches. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > > > --- > > > > drivers/block/blk-uclass.c | 13 +++++++++++++ > > > > include/blk.h | 12 ++++++++++++ > > > > 2 files changed, 25 insertions(+) > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > But please add a test for this. > > > > Well, how can we test this simple function? > > I can't simply imagine what the meaningful test scenario is. > > > > > > > > > > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c > > > > index 83682dcc181a..f7ad90e8890f 100644 > > > > --- a/drivers/block/blk-uclass.c > > > > +++ b/drivers/block/blk-uclass.c > > > > @@ -670,6 +670,19 @@ int blk_create_devicef(struct udevice *parent, > > > > const char *drv_name, > > > > return 0; > > > > } > > > > > > > > +int blk_probe_or_unbind(struct udevice *dev) > > > > +{ > > > > + int ret; > > > > + > > > > + ret = device_probe(dev); > > > > + if (ret) { > > > > + debug("probing %s failed\n", dev->name); > > %s/debug(/log_debug(/
I used "debug()" here as it is commonly used in this file(blk-uclass.c). > > > > + device_unbind(dev); > > > > + } > > > > + > > > > + return ret; > > > > +} > > > > + > > > > int blk_unbind_all(int if_type) > > > > { > > > > struct uclass *uc; > > > > diff --git a/include/blk.h b/include/blk.h > > > > index f0cc7ca1a28c..ef79e7b27b0a 100644 > > > > --- a/include/blk.h > > > > +++ b/include/blk.h > > > > @@ -369,6 +369,18 @@ int blk_create_devicef(struct udevice *parent, > > > > const char *drv_name, > > > > const char *name, int if_type, int devnum, int > > > > blksz, > > > > lbaint_t lba, struct udevice **devp); > > > > > > > > +/** > > > > + * blk_probe_or_unbind() - Try to probe > > > > + * > > > > + * Try to probe the device, primarily for enumelating partitions. > > > > > > enumerating > > > > Ah, OK. > > > > -Takahiro Akashi > > > > > > > > > > > > + * If it fails, the device itself is unbound since it means that it > > > > won't > > > > + * work any more. > > > > + * > > > > + * @dev: The device to probe > > > > + * @return 0 if OK, -ve on error > > %s/@return/Return:/ I used "@return" here as it is commonly used in this file(blk.h). > Please, stick to Spinx style comments as documented in > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation > > Otherwise 'make htmldocs' will fail. If so, why don't other uses of "@return" cause failures as well? > I will fix the remaining issues when merging. Thank you. We'd better fix all the occurrences in those files at the same time to avoid possible confusion. -Takahiro Akashi > Best regards > > Heinrich > > > > > + */ > > > > +int blk_probe_or_unbind(struct udevice *dev); > > > > + > > > > /** > > > > * blk_unbind_all() - Unbind all device of the given interface type > > > > * > > > > -- > > > > 2.33.0 > > > > >