[V4,09/17] scsi: ufs: manually add well known logical units

Commit Message

Dolev Raviv Sept. 23, 2014, 7:31 a.m. UTC

From: Subhash Jadavani <[e-mail]>

According to SELECT REPORT field (of REPORT LUNS command) from SPC-4
(SCSI Primary Commands) specification, device should report all the LUNs
(which support any of the addressing method specified in the description).
Note that UFS W-LUs follow the extended logical unit addressing mothod
hence according to SPC specification all the UFS W-LUs should be reported
when SELECT REPORT field is to 00h.
But it seems UFS specification has modified the meaning of SELECT REPORT
field when its set to 00h. When SELECT REPORT field is cleared, device
should only report LUs which support peripheral device addressing method.
Even UFS Test specification seems to confirm this diversion from the spec.
Hence it's reasonable for the UFS devices adhering to the UFS device
specification to not report the W-LUs when SELECT REPORT is 00h.

scsi_scan_host() would only scan LUs with SELECT REPORT set to 00h hence
this patch manually add the UFS well known logical units.

Signed-off-by: Subhash Jadavani <[e-mail]>
Signed-off-by: Dolev Raviv <[e-mail]>

Comments

Christoph Hellwig Sept. 23, 2014, 8:35 a.m. UTC | #1

None of the REPORT LUNS language makes sense as we're not using it.

Can you respon the patch with a better description, and a little comment
in the code on why you're adding these wluns.

Also can I assume none of the later patches relies on their existance?
If you do you need to check the error return from scsi_add_device, if
not said comment should mention why it's fine to not actually find
any of these.

Also if you do use one of them from kernel space later it might make
more sense to use __scsi_add_device and store a pointer to the
scsi_device instead of looking it up later.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dolev Raviv Sept. 23, 2014, 11:48 a.m. UTC | #2

> None of the REPORT LUNS language makes sense as we're not using it.
>
> Can you respon the patch with a better description, and a little comment
> in the code on why you're adding these wluns.

ok.

>
> Also can I assume none of the later patches relies on their existance?

In UFS power management commands such as SSU, are sent to "device w-
lun". Leaving it out should only affect the ability to suspend the
device.
Other patches are not directly affected by it.

> If you do you need to check the error return from scsi_add_device, if
> not said comment should mention why it's fine to not actually find
> any of these.
>

Will discus it further with Subhash.

> Also if you do use one of them from kernel space later it might make
> more sense to use __scsi_add_device and store a pointer to the
> scsi_device instead of looking it up later.
>

I'll review all you suggestions with Subhash.

[e-mail] Sept. 23, 2014, 10:48 p.m. UTC | #3

Thanks Chris. Agreed with all your suggestion, next revision will address
your comments.

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Dolev Raviv
Sent: Tuesday, September 23, 2014 4:49 AM
To: Christoph Hellwig
Cc: Dolev Raviv; [e-mail]; [e-mail];
[e-mail]; [e-mail];
[e-mail]; [e-mail]; Subhash Jadavani
Subject: Re: [PATCH V4 09/17] scsi: ufs: manually add well known logical
units


> None of the REPORT LUNS language makes sense as we're not using it.
>
> Can you respon the patch with a better description, and a little 
> comment in the code on why you're adding these wluns.

ok.

>
> Also can I assume none of the later patches relies on their existance?

In UFS power management commands such as SSU, are sent to "device w- lun".
Leaving it out should only affect the ability to suspend the device.
Other patches are not directly affected by it.

> If you do you need to check the error return from scsi_add_device, if 
> not said comment should mention why it's fine to not actually find any 
> of these.
>

Will discus it further with Subhash.

> Also if you do use one of them from kernel space later it might make 
> more sense to use __scsi_add_device and store a pointer to the 
> scsi_device instead of looking it up later.
>

I'll review all you suggestions with Subhash.


--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dc89c48..1c5422b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -902,6 +902,17 @@  static int ufshcd_compose_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 }
 
 /**
+ * ufshcd_upiu_wlun_to_scsi_wlun - maps UPIU W-LUN id to SCSI W-LUN ID
+ * @scsi_lun: UPIU W-LUN id
+ *
+ * Returns SCSI W-LUN id
+ */
+static inline u16 ufshcd_upiu_wlun_to_scsi_wlun(u8 upiu_wlun_id)
+{
+	return (upiu_wlun_id & ~UFS_UPIU_WLUN_ID) | SCSI_W_LUN_BASE;
+}
+
+/**
  * ufshcd_queuecommand - main entry point for SCSI requests
  * @cmd: command from SCSI Midlayer
  * @done: call back function
@@ -3415,6 +3426,15 @@  static int ufshcd_probe_hba(struct ufs_hba *hba)
 		if (!hba->is_init_prefetch)
 			ufshcd_init_icc_levels(hba);
 
+		scsi_add_device(hba->host, 0, 0,
+			ufshcd_upiu_wlun_to_scsi_wlun(
+					UFS_UPIU_UFS_DEVICE_WLUN));
+		scsi_add_device(hba->host, 0, 0,
+			ufshcd_upiu_wlun_to_scsi_wlun(
+					UFS_UPIU_BOOT_WLUN));
+		scsi_add_device(hba->host, 0, 0,
+			ufshcd_upiu_wlun_to_scsi_wlun(
+					UFS_UPIU_RPMB_WLUN));
 		scsi_scan_host(hba->host);
 		pm_runtime_put_sync(hba->dev);
 	}