Currently the SES driver reports bogus errors when plugging
in a drive that exposes only a single, simple subenclosure:
[ 432.713731] scsi 3:0:0:0: Direct-Access WD Elements 2667 2007 PQ: 0 ANSI: 6
[ 432.714127] scsi 3:0:0:1: Enclosure WD SES Device 2007 PQ: 0 ANSI: 6
...
[ 432.716508] scsi 3:0:0:1: Attached scsi generic sg2 type 13
...
[ 439.897020] scsi 3:0:0:1: Wrong diagnostic page; asked for 1 got 8
[ 439.897023] scsi 3:0:0:1: Failed to get diagnostic page 0x1
[ 439.897025] scsi 3:0:0:1: Failed to bind enclosure -19
According to the SES specification, simple subenclosures always
return diagnostic page 8 no matter what page was requested, so
the device is permitted to page 8 here and nothing is wrong.
To avoid polluting the kernel logs with bogus errors, the first
diagnostic page read bypasses the usual page code check. If it
returns page 8 the device is assumed to be a simple subenclosure
and no enclosure device is created. Simple subenclosures only
return a vendor specific status byte in page 8 and don't support
any other pages, so they can't support the enclosure interface.
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx>
---
drivers/scsi/ses.c | 53 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index d7d0c35c58b8..3457d8bc1ddf 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -73,9 +73,12 @@ static void init_device_slot_control(unsigned char *dest_desc,
dest_desc[3] &= 0x3c;
}
-
-static int ses_recv_diag(struct scsi_device *sdev, int page_code,
- void *buf, int bufflen)
+/*
+ * Raw RECEIVE_DIAGNOSTIC page command. Does not check the returned
+ * page code, which may differ from the requested page code!
+ */
+static int __ses_recv_diag(struct scsi_device *sdev, int page_code,
+ void *buf, int bufflen)
{
int ret;
unsigned char cmd[] = {
@@ -86,7 +89,6 @@ static int ses_recv_diag(struct scsi_device *sdev, int page_code,
bufflen & 0xff,
0
};
- unsigned char recv_page_code;
unsigned int retries = SES_RETRIES;
struct scsi_sense_hdr sshdr;
const struct scsi_exec_args exec_args = {
@@ -100,6 +102,15 @@ static int ses_recv_diag(struct scsi_device *sdev, int page_code,
(sshdr.sense_key == NOT_READY ||
(sshdr.sense_key == UNIT_ATTENTION && sshdr.asc == 0x29)));
+ return ret;
+}
+
+static int ses_recv_diag(struct scsi_device *sdev, int page_code,
+ void *buf, int bufflen)
+{
+ unsigned char recv_page_code;
+ int ret = __ses_recv_diag(sdev, page_code, buf, bufflen);
+
if (unlikely(ret))
return ret;
@@ -108,8 +119,11 @@ static int ses_recv_diag(struct scsi_device *sdev, int page_code,
if (likely(recv_page_code == page_code))
return ret;
- /* successful diagnostic but wrong page code. This happens to some
- * USB devices, just print a message and pretend there was an error */
+ /*
+ * Successful diagnostic but wrong page code. Shouldn't happen
+ * except for simple subenclosures, which should already have
+ * been detected by this point.
+ */
sdev_printk(KERN_ERR, sdev,
"Wrong diagnostic page; asked for %d got %u\n",
@@ -695,11 +709,33 @@ static int ses_intf_add(struct device *cdev)
if (!hdr_buf || !ses_dev)
goto err_init_free;
+ /*
+ * Read without checking page code, getting a different page
+ * is not necessarily an error for devices with only a simple
+ * subenclosure (eg. some USB drives).
+ */
page = 1;
- result = ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
+ result = __ses_recv_diag(sdev, page, hdr_buf, INIT_ALLOC_SIZE);
if (result)
goto recv_failed;
+ /*
+ * A simple subenclosure only supports page 8 and will return
+ * it after any diagnostic page request. Simple subenclosures
+ * are not supported by this driver -- there is simply no data
+ * to report besides a vendor specific byte -- but they will
+ * not be treated as an error.
+ */
+ if (hdr_buf[0] == 8) {
+ err = 0;
+ goto err_init_free;
+ }
+
+ /*
+ * All diagnostic pages will include a length field so even
+ * if the page code is incorrect at this point, that'll get
+ * detected when re-reading the page.
+ */
len = (hdr_buf[2] << 8) + hdr_buf[3] + 4;
buf = kzalloc(len, GFP_KERNEL);
if (!buf)
@@ -817,7 +853,8 @@ static int ses_intf_add(struct device *cdev)
err_init_free:
kfree(ses_dev);
kfree(hdr_buf);
- sdev_printk(KERN_ERR, sdev, "Failed to bind enclosure %d\n", err);
+ if (err)
+ sdev_printk(KERN_ERR, sdev, "Failed to bind enclosure %d\n", err);
return err;
}
--
2.39.2
