On Thu, Aug 22, 2024 at 02:36:07PM -0700, Bart Van Assche wrote: > Move the ufshcd_device_init(hba, true) call from ufshcd_async_scan() > into ufshcd_init(). This patch prepares for moving both scsi_add_host() > calls into ufshcd_add_scsi_host(). Calling ufshcd_device_init() from > ufshcd_init() without holding hba->host_sem is safe because > hba->host_sem serializes core code with sysfs callback code and because > the ufshcd_device_init() is moved before the scsi_add_host() call. I think this last sentence is not complete. > > Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx> One nitpick below. Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > --- > drivers/ufs/core/ufshcd.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 5c8751672bc7..0fdf19889191 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -8933,10 +8933,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie) > int ret; > > down(&hba->host_sem); > - /* Initialize hba, detect and initialize UFS device */ > - ret = ufshcd_device_init(hba, /*init_dev_params=*/true); > - if (ret == 0) > - ret = ufshcd_probe_hba(hba); > + ret = ufshcd_probe_hba(hba); > up(&hba->host_sem); > if (ret) > goto out; > @@ -10632,6 +10629,11 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) > */ > ufshcd_set_ufs_dev_active(hba); > > + /* Initialize hba, detect and initialize UFS device */ > + err = ufshcd_device_init(hba, /*init_dev_params=*/true); I think this inline comment is not really needed. It is rather decreasing code readability. - Mani > + if (err) > + goto out_disable; > + > err = ufshcd_add_scsi_host(hba); > if (err) > goto out_disable; -- மணிவண்ணன் சதாசிவம்