| Message ID | 20211210064947.73361-3-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: more tightly integrate UEFI disks to driver model | expand |
Hi Takahiro, On Thu, 9 Dec 2021 at 23:59, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > This function will be commonly used in block device drivers > in the succeeding patches. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > drivers/block/blk-uclass.c | 13 +++++++++++++ > include/blk.h | 12 ++++++++++++ > 2 files changed, 25 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> But please add a test for this. > > 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); > + 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 > + * 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 > + */ > +int blk_probe_or_unbind(struct udevice *dev); > + > /** > * blk_unbind_all() - Unbind all device of the given interface type > * > -- > 2.33.0 >
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.akashi@linaro.org> wrote: > > > > This function will be commonly used in block device drivers > > in the succeeding patches. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > drivers/block/blk-uclass.c | 13 +++++++++++++ > > include/blk.h | 12 ++++++++++++ > > 2 files changed, 25 insertions(+) > > Reviewed-by: Simon Glass <sjg@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); > > + 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 > > + */ > > +int blk_probe_or_unbind(struct udevice *dev); > > + > > /** > > * blk_unbind_all() - Unbind all device of the given interface type > > * > > -- > > 2.33.0 > >
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.akashi@linaro.org> wrote: >>> >>> This function will be commonly used in block device drivers >>> in the succeeding patches. >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> drivers/block/blk-uclass.c | 13 +++++++++++++ >>> include/blk.h | 12 ++++++++++++ >>> 2 files changed, 25 insertions(+) >> >> Reviewed-by: Simon Glass <sjg@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(/ >>> + 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:/ 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. I will fix the remaining issues when merging. 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 >>>
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.akashi@linaro.org> wrote: > > > > > > > > This function will be commonly used in block device drivers > > > > in the succeeding patches. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > --- > > > > drivers/block/blk-uclass.c | 13 +++++++++++++ > > > > include/blk.h | 12 ++++++++++++ > > > > 2 files changed, 25 insertions(+) > > > > > > Reviewed-by: Simon Glass <sjg@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 > > > > >
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); + 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. + * 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 + */ +int blk_probe_or_unbind(struct udevice *dev); + /** * blk_unbind_all() - Unbind all device of the given interface type *
This function will be commonly used in block device drivers in the succeeding patches. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- drivers/block/blk-uclass.c | 13 +++++++++++++ include/blk.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)