Re: [RFC 07/22] block: ide: call device_probe() after scanning

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Tue, Oct 12, 2021 at 08:53:54AM +0300, Ilias Apalodimas wrote:
> On Mon, Oct 11, 2021 at 08:54:13AM -0600, Simon Glass wrote:
> > Hi Takahiro,
> > 
> > On Sun, 10 Oct 2021 at 19:43, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Sun, Oct 10, 2021 at 08:14:13AM -0600, Simon Glass wrote:
> > > > On Thu, 30 Sept 2021 at 23:03, AKASHI Takahiro
> > > > <takahiro.akashi@linaro.org> wrote:
> > > > >
> > > > > Every time an ide bus/port is scanned and a new device is detected,
> > > > > we want to call device_probe() as it will give us a chance to run additional
> > > > > post-processings for some purposes.
> > > > >
> > > > > In particular, support for creating partitions on a device will be added.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > > ---
> > > > >  drivers/block/ide.c | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > >
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > I'm starting to wonder if you can create a function that does the
> > > > probe and unbind? Something in the blk interface, perhaps? It would
> > > > reduce the duplicated code and provide a standard way of bringing up a
> > > > new device.
> > >
> > > That is exactly what Ilias suggested but I'm a bit declined to do :)
> > >
> > > Common 'scanning' code looks like:
> > >   blk_create_devicef(... , &dev);
> > >   desc = dev_get_uclass_data(dev);
> > >   initialize some members in desc as well as device-specific info --- (A)
> > >     (now dev can be accessible.)
> > >   ret = device_probe(dev);
> > >   if (ret) {
> > >      de-initialize *dev*  --- (B)
> > >      device_unbind()
> > >   }
> > >
> > > Basically (B) is supposed to undo (A) which may or may not exist,
> > > depending on types of block devices.
> > >
> > > So I'm not 100% sure that a combination of device_probe() and device_unbind()
> > > will fit to all the device types.
> > > (The only cases that I have noticed are fsl_sata.c and sata_sil.c. Both
> > > have their own xxx_unbind_device(), but they simply call device_remove() and
> > > device_unbind(), though. So no worry?)
> > 
> > Yes I agree it would be a very strange function. But at least it would
> > have the benefit of grouping the code together under a particular
> > name, something like blk_back_out_bind(), but that's not a good
> > name....it just feels like this might get refactored in the future and
> > having the code in one place might be handy.
> 
> naming is hard! try_device_probe() maybe?

Indeed. A name should come later.
So I will temporarily use blk_probe_or_unbind() :-)

-Takahiro Akashi


> Cheers
> /Ilias
> > 
> > Regards,
> > Simon