On 9/10/2024 2:50 PM, Bart Van Assche wrote:
ufshcd_async_scan() is called (asynchronously) only by ufshcd_init().
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. This is safe because
hba->host_sem serializes core code and sysfs callbacks. The
ufshcd_device_init() call is moved before the scsi_add_host() call and
hence happens before any SCSI sysfs attributes are created.
Since ufshcd_add_scsi_host() checks whether or not the MCQ mode is
enabled and since ufshcd_device_init() may modify hba->mcq_enabled,
only call scsi_add_host() from ufshcd_device_init() if the SCSI host
has not yet been added by ufshcd_device_init().
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
drivers/ufs/core/ufshcd.c | 16 +++++++++-------
include/ufs/ufshcd.h | 3 +++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index f62d257a92da..a3c5493ccc8f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8907,16 +8907,12 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
static void ufshcd_async_scan(void *data, async_cookie_t cookie)
{
struct ufs_hba *hba = (struct ufs_hba *)data;
- ktime_t device_init_start;
int ret;
down(&hba->host_sem);
/* Initialize hba, detect and initialize UFS device */
- device_init_start = ktime_get();
- ret = ufshcd_device_init(hba, /*init_dev_params=*/true);
- if (ret == 0)
- ret = ufshcd_probe_hba(hba, true);
- ufshcd_process_device_init_result(hba, device_init_start, ret);
+ ret = ufshcd_probe_hba(hba, true);
+ ufshcd_process_device_init_result(hba, hba->device_init_start, ret);
This patch probably changed the original trace print. In the original
code, the trace only prints the time spent in probe_hba().
In this patch however the trace print includes the time spent in
ufshcd_add_scsi_host() plus the time wait for
async_schedule(ufshcd_async_scan, hba).
up(&hba->host_sem);
if (ret)
goto out;
@@ -10387,7 +10383,7 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
{
int err;
- if (!is_mcq_supported(hba)) {
+ if (!hba->scsi_host_added) {
err = scsi_add_host(hba->host, hba->dev);
if (err) {
dev_err(hba->dev, "scsi_add_host failed\n");
@@ -10610,6 +10606,12 @@ 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 */
+ hba->device_init_start = ktime_get();
+ err = ufshcd_device_init(hba, /*init_dev_params=*/true);
+ if (err)
+ goto out_disable;
+
err = ufshcd_add_scsi_host(hba);
if (err)
goto out_disable;
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index a43b14276bc3..55ec89fa16af 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -839,6 +839,7 @@ enum ufshcd_mcq_opr {
* @dev: device handle
* @ufs_device_wlun: WLUN that controls the entire UFS device.
* @hwmon_device: device instance registered with the hwmon core.
+ * @device_init_start: start time of first ufshcd_device_init() call.
* @curr_dev_pwr_mode: active UFS device power mode.
* @uic_link_state: active state of the link to the UFS device.
* @rpm_lvl: desired UFS power management level during runtime PM.
@@ -972,6 +973,8 @@ struct ufs_hba {
struct device *hwmon_device;
#endif
+ ktime_t device_init_start;
IMO, it's kinda overkill to add this device_init_start member for the
purpose of trace print the time spent in the first probe_hba() only.
How about print the time spent in probe_hba() outside of the
ufshcd_process_device_init_result() function so that you don't need to
pass device_init_start into ufshcd_process_device_init_result()?
+
enum ufs_dev_pwr_mode curr_dev_pwr_mode;
enum uic_link_state uic_link_state;
/* Desired UFS power management level during runtime PM */
