Re: [resent RFC 06/22] sata: call device_probe() after scanning

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

On Mon, Oct 04, 2021 at 09:45:35PM +0300, Ilias Apalodimas wrote:
> On Mon, Oct 04, 2021 at 12:44:14PM +0900, AKASHI Takahiro wrote:
> > Every time a sata 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/ata/dwc_ahsata.c | 10 ++++++++++
> >  drivers/ata/fsl_sata.c   | 11 +++++++++++
> >  drivers/ata/sata_mv.c    |  9 +++++++++
> >  drivers/ata/sata_sil.c   | 12 ++++++++++++
> >  4 files changed, 42 insertions(+)
> > 
> > diff --git a/drivers/ata/dwc_ahsata.c b/drivers/ata/dwc_ahsata.c
> > index 6d42548087b3..6a51c70d1170 100644
> > --- a/drivers/ata/dwc_ahsata.c
> > +++ b/drivers/ata/dwc_ahsata.c
> > @@ -1026,6 +1026,16 @@ int dwc_ahsata_scan(struct udevice *dev)
> >  		return ret;
> >  	}
> >  
> > +	ret = device_probe(bdev);
> > +	if (ret < 0) {
> > +		debug("Can't probe\n");
> > +		/* TODO: undo create */
> > +
> > +		device_unbind(bdev);
> > +
> > +		return ret;
> > +	}
> > +
> 
> Patches 2-6 seem to do the same thing for different subsystems.  I think
> creating a function for that would make it easier. 

Well, the reason why I put those changes in separate commits is
- first, different subsystems are owned by different maintainers, and
- more importantly, different subsystems may have different cleanup
  processing required.
  There are always extra setups after blk_create_device(), which should
  be reverted if device_probe() fails. For instance, sil_unbind_device()
  and fsl_unbind_device().
  So I would like to leave subsystem owners responsible for that. 

-Takahiro Akashi


> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/ata/fsl_sata.c b/drivers/ata/fsl_sata.c
> > index e44db0a37458..346e9298b4c5 100644
> > --- a/drivers/ata/fsl_sata.c
> > +++ b/drivers/ata/fsl_sata.c
> > @@ -982,6 +982,17 @@ static int fsl_ata_probe(struct udevice *dev)
> >  			failed_number++;
> >  			continue;
> >  		}
> > +
> > +		ret = device_probe(bdev);
> > +		if (ret < 0) {
> > +			debug("Can't probe\n");
> > +			ret = fsl_unbind_device(blk);
> 
> Apart from this exception I guess
> 
> > +			if (ret)
> > +				return ret;
> > +
> > +			failed_number++;
> > +			continue;
> > +		}
> >  	}
> >  
> >  	if (failed_number == nr_ports)
> > diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> > index 003222d47be6..09b735779ebf 100644
> > --- a/drivers/ata/sata_mv.c
> > +++ b/drivers/ata/sata_mv.c
> > @@ -1099,6 +1099,15 @@ static int sata_mv_probe(struct udevice *dev)
> >  			continue;
> >  		}
> >  
> > +		ret = device_probe(bdev);
> > +		if (ret < 0) {
> > +			debug("Can't probe\n");
> > +			/* TODO: undo create */
> > +
> > +			device_unbind(bdev);
> > +			continue;
> > +		}
> > +
> >  		/* If we got here, the current SATA port was probed
> >  		 * successfully, so set the probe status to successful.
> >  		 */
> > diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
> > index dda712f42cb2..295f7ca72303 100644
> > --- a/drivers/ata/sata_sil.c
> > +++ b/drivers/ata/sata_sil.c
> > @@ -864,6 +864,18 @@ static int sil_pci_probe(struct udevice *dev)
> >  			failed_number++;
> >  			continue;
> >  		}
> > +
> > +		ret = device_probe(bdev);
> > +		if (ret < 0) {
> > +			debug("Can't probe\n");
> > +			ret = sil_unbind_device(blk);
> > +			device_unbind(bdev);
> > +			if (ret)
> > +				return ret;
> > +
> > +			failed_number++;
> > +			continue;
> > +		}
> >  	}
> >  
> >  	if (failed_number == sata_info.maxport)
> > -- 
> > 2.33.0
> >