diff mbox

[V5,01/17] scsi: fixing the "type" for well known LUs

Message ID 1411571653-22729-2-git-send-email-draviv@codeaurora.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Dolev Raviv Sept. 24, 2014, 3:13 p.m. UTC
From: Subhash Jadavani <subhashj@codeaurora.org>

Some devices may respond with wrong type for well-known logical units.
This patch forces well-known type for devices which doesn't report it
correct.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Dolev Raviv <draviv@codeaurora.org>

Comments

Christoph Hellwig Sept. 24, 2014, 4:04 p.m. UTC | #1
On Wed, Sep 24, 2014 at 06:13:57PM +0300, Dolev Raviv wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> Some devices may respond with wrong type for well-known logical units.
> This patch forces well-known type for devices which doesn't report it
> correct.

This looks fine to me, as the well known LUN addresses seem to be nailed
down nicely in t10, but let's see if Martin or Robert disagree..

> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Dolev Raviv <draviv@codeaurora.org>
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 56675db..a34db9e 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -805,6 +805,14 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  	} else {
>  		sdev->type = (inq_result[0] & 0x1f);
>  		sdev->removable = (inq_result[1] & 0x80) >> 7;
> +
> +		/*
> +		 * some devices may respond with wrong type for
> +		 * well-known logical units. Force well-known type
> +		 * to enumerate them correctly.
> +		 */
> +		if (scsi_is_wlun(sdev->lun))
> +			sdev->type = TYPE_WLUN;
>  	}
>  
>  	if (sdev->type == TYPE_RBC || sdev->type == TYPE_ROM) {
> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index 261e708..d17178e 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -333,6 +333,7 @@ static inline int scsi_status_is_good(int status)
>  #define TYPE_RBC	    0x0e
>  #define TYPE_OSD            0x11
>  #define TYPE_ZBC            0x14
> +#define TYPE_WLUN           0x1e    /* well-known logical unit */
>  #define TYPE_NO_LUN         0x7f
>  
>  /* SCSI protocols; these are taken from SPC-3 section 7.5 */
> -- 
> 1.8.5.2
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Elliott, Robert (Server Storage) Sept. 24, 2014, 8:49 p.m. UTC | #2
> From: Christoph Hellwig [mailto:hch@infradead.org]
...
> On Wed, Sep 24, 2014 at 06:13:57PM +0300, Dolev Raviv wrote:
> > From: Subhash Jadavani <subhashj@codeaurora.org>
> >
> > Some devices may respond with wrong type for well-known logical units.
> > This patch forces well-known type for devices which doesn't report it
> > correct.
> 
> This looks fine to me, as the well known LUN addresses seem to be nailed
> down nicely in t10, but let's see if Martin or Robert disagree..
> 
...
> > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> > index 56675db..a34db9e 100644
> > --- a/drivers/scsi/scsi_scan.c
> > +++ b/drivers/scsi/scsi_scan.c
> > @@ -805,6 +805,14 @@ static int scsi_add_lun(struct scsi_device *sdev,
> unsigned char *inq_result,
> >  	} else {
> >  		sdev->type = (inq_result[0] & 0x1f);
> >  		sdev->removable = (inq_result[1] & 0x80) >> 7;
> > +
> > +		/*
> > +		 * some devices may respond with wrong type for
> > +		 * well-known logical units. Force well-known type
> > +		 * to enumerate them correctly.
> > +		 */
> > +		if (scsi_is_wlun(sdev->lun))
> > +			sdev->type = TYPE_WLUN;
> >  	}
...

My only concern is that the peripheral device type was included
at the outset in spc3r01 in 2001, so a design that can't get this
right might have other problems.  A print might be justified
to report something is amiss:

	if (scsi_is_wlun(sdev->lun) && sdev->type != TYPE_WLUN) {
		sdev_printk(KERN_WARNING, sdev,
			"%s: correcting incorrect peripheral device type 0x%x for W-LUN 0x%16phN\n",
			__func__, sdev->type, sdev->lun);
		sdev->type = TYPE_WLUN;
	}


---
Rob Elliott    HP Server Storage





--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
subhashj@codeaurora.org Sept. 25, 2014, 6:18 a.m. UTC | #3
>> > +		 * some devices may respond with wrong type for
>> > +		 * well-known logical units. Force well-known type
>> > +		 * to enumerate them correctly.
>> > +		 */
>> > +		if (scsi_is_wlun(sdev->lun))
>> > +			sdev->type = TYPE_WLUN;
>> >  	}
> ...
>
> My only concern is that the peripheral device type was included at the
outset in spc3r01 in 2001, so a design that can't get this right might have
other problems.  A print might be justified to report something is amiss:
>
>	if (scsi_is_wlun(sdev->lun) && sdev->type != TYPE_WLUN) {
>		sdev_printk(KERN_WARNING, sdev,
>			"%s: correcting incorrect peripheral device type
0x%x for W-LUN 0x%16phN\n",
>			__func__, sdev->type, sdev->lun);
>		sdev->type = TYPE_WLUN;
>	}

Agreed, will take care of this in next patch revision.


-----Original Message-----
From: linux-scsi-owner@vger.kernel.org
[mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Elliott, Robert
(Server Storage)
Sent: Wednesday, September 24, 2014 1:50 PM
To: Christoph Hellwig; Dolev Raviv
Cc: James.Bottomley@HansenPartnership.com; linux-scsi@vger.kernel.org;
linux-scsi-owner@vger.kernel.org; linux-arm-msm@vger.kernel.org;
santoshsy@gmail.com; Subhash Jadavani; Sujit Reddy Thumma; Martin K.
Petersen
Subject: RE: [PATCH V5 01/17] scsi: fixing the "type" for well known LUs


> From: Christoph Hellwig [mailto:hch@infradead.org]
...
> On Wed, Sep 24, 2014 at 06:13:57PM +0300, Dolev Raviv wrote:
> > From: Subhash Jadavani <subhashj@codeaurora.org>
> >
> > Some devices may respond with wrong type for well-known logical units.
> > This patch forces well-known type for devices which doesn't report 
> > it correct.
> 
> This looks fine to me, as the well known LUN addresses seem to be 
> nailed down nicely in t10, but let's see if Martin or Robert disagree..
> 
...
> > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c 
> > index 56675db..a34db9e 100644
> > --- a/drivers/scsi/scsi_scan.c
> > +++ b/drivers/scsi/scsi_scan.c
> > @@ -805,6 +805,14 @@ static int scsi_add_lun(struct scsi_device 
> > *sdev,
> unsigned char *inq_result,
> >  	} else {
> >  		sdev->type = (inq_result[0] & 0x1f);
> >  		sdev->removable = (inq_result[1] & 0x80) >> 7;
> > +
> > +		/*
> > +		 * some devices may respond with wrong type for
> > +		 * well-known logical units. Force well-known type
> > +		 * to enumerate them correctly.
> > +		 */
> > +		if (scsi_is_wlun(sdev->lun))
> > +			sdev->type = TYPE_WLUN;
> >  	}
...

My only concern is that the peripheral device type was included at the
outset in spc3r01 in 2001, so a design that can't get this right might have
other problems.  A print might be justified to report something is amiss:

	if (scsi_is_wlun(sdev->lun) && sdev->type != TYPE_WLUN) {
		sdev_printk(KERN_WARNING, sdev,
			"%s: correcting incorrect peripheral device type
0x%x for W-LUN 0x%16phN\n",
			__func__, sdev->type, sdev->lun);
		sdev->type = TYPE_WLUN;
	}


---
Rob Elliott    HP Server Storage





--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the
body of a message to majordomo@vger.kernel.org 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56675db..a34db9e 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -805,6 +805,14 @@  static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	} else {
 		sdev->type = (inq_result[0] & 0x1f);
 		sdev->removable = (inq_result[1] & 0x80) >> 7;
+
+		/*
+		 * some devices may respond with wrong type for
+		 * well-known logical units. Force well-known type
+		 * to enumerate them correctly.
+		 */
+		if (scsi_is_wlun(sdev->lun))
+			sdev->type = TYPE_WLUN;
 	}
 
 	if (sdev->type == TYPE_RBC || sdev->type == TYPE_ROM) {
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 261e708..d17178e 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -333,6 +333,7 @@  static inline int scsi_status_is_good(int status)
 #define TYPE_RBC	    0x0e
 #define TYPE_OSD            0x11
 #define TYPE_ZBC            0x14
+#define TYPE_WLUN           0x1e    /* well-known logical unit */
 #define TYPE_NO_LUN         0x7f
 
 /* SCSI protocols; these are taken from SPC-3 section 7.5 */