[U-Boot] [RFC PATCH 1/5] scsi: Extract block device initialization
Extract block device initialization to specific function.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/scsi.c | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index dbbf4043b22a..0bce91dfa099 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -458,6 +458,28 @@ void scsi_setup_test_unit_ready(ccb *pccb) pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */ }
+static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) +{ + dev_desc->target = 0xff; + dev_desc->lun = 0xff; + dev_desc->lba = 0; + dev_desc->blksz = 0; + dev_desc->log2blksz = + LOG2_INVALID(typeof(dev_desc->log2blksz)); + dev_desc->type = DEV_TYPE_UNKNOWN; + dev_desc->vendor[0] = 0; + dev_desc->product[0] = 0; + dev_desc->revision[0] = 0; + dev_desc->removable = false; + dev_desc->if_type = IF_TYPE_SCSI; + dev_desc->devnum = devnum; + dev_desc->part_type = PART_TYPE_UNKNOWN; +#ifndef CONFIG_BLK + dev_desc->block_read = scsi_read; + dev_desc->block_write = scsi_write; +#endif +} + /* * (re)-scan the scsi bus and reports scsi device info * to the user if mode = 1 @@ -471,26 +493,9 @@ void scsi_scan(int mode)
if (mode == 1) printf("scanning bus for devices...\n"); - for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { - scsi_dev_desc[i].target = 0xff; - scsi_dev_desc[i].lun = 0xff; - scsi_dev_desc[i].lba = 0; - scsi_dev_desc[i].blksz = 0; - scsi_dev_desc[i].log2blksz = - LOG2_INVALID(typeof(scsi_dev_desc[i].log2blksz)); - scsi_dev_desc[i].type = DEV_TYPE_UNKNOWN; - scsi_dev_desc[i].vendor[0] = 0; - scsi_dev_desc[i].product[0] = 0; - scsi_dev_desc[i].revision[0] = 0; - scsi_dev_desc[i].removable = false; - scsi_dev_desc[i].if_type = IF_TYPE_SCSI; - scsi_dev_desc[i].devnum = i; - scsi_dev_desc[i].part_type = PART_TYPE_UNKNOWN; -#ifndef CONFIG_BLK - scsi_dev_desc[i].block_read = scsi_read; - scsi_dev_desc[i].block_write = scsi_write; -#endif - } + for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) + scsi_init_dev_desc(&scsi_dev_desc[i], i); + scsi_max_devs = 0; for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { pccb->target = i;
The patch enables running detection algorithm on block device description structure.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/scsi.c | 134 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 73 insertions(+), 61 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index 0bce91dfa099..a02c7a505639 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,15 +480,79 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) #endif }
+static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc, int lun) +{ + unsigned char perq, modi; + lbaint_t capacity; + unsigned long blksz; + + pccb->lun = lun; + pccb->pdata = (unsigned char *)&tempbuff; + pccb->datalen = 512; + scsi_setup_inquiry(pccb); + if (scsi_exec(pccb) != true) { + if (pccb->contr_stat == SCSI_SEL_TIME_OUT) { + /* + * selection timeout => assuming no + * device present + */ + debug("Selection timeout ID %d\n", + pccb->target); + return -ETIMEDOUT; + } + scsi_print_error(pccb); + return -ENODEV; + } + perq = tempbuff[0]; + modi = tempbuff[1]; + if ((perq & 0x1f) == 0x1f) + return -ENODEV; /* skip unknown devices */ + if ((modi & 0x80) == 0x80) /* drive is removable */ + dev_desc->removable = true; + /* get info for this device */ + scsi_ident_cpy((unsigned char *)&dev_desc + [0].vendor[0], + &tempbuff[8], 8); + scsi_ident_cpy((unsigned char *)&dev_desc + [0].product[0], + &tempbuff[16], 16); + scsi_ident_cpy((unsigned char *)&dev_desc + [0].revision[0], + &tempbuff[32], 4); + dev_desc->target = pccb->target; + dev_desc->lun = pccb->lun; + + pccb->datalen = 0; + scsi_setup_test_unit_ready(pccb); + if (scsi_exec(pccb) != true) { + if (dev_desc->removable) { + dev_desc->type = perq; + goto removable; + } + scsi_print_error(pccb); + return -EINVAL; + } + if (scsi_read_capacity(pccb, &capacity, &blksz)) { + scsi_print_error(pccb); + return -EINVAL; + } + dev_desc->lba = capacity; + dev_desc->blksz = blksz; + dev_desc->log2blksz = LOG2(dev_desc->blksz); + dev_desc->type = perq; + part_init(&dev_desc[0]); +removable: + return 0; +} + /* * (re)-scan the scsi bus and reports scsi device info * to the user if mode = 1 */ void scsi_scan(int mode) { - unsigned char i, perq, modi, lun; - lbaint_t capacity; - unsigned long blksz; + unsigned char i, lun; + int ret; ccb *pccb = (ccb *)&tempccb;
if (mode == 1) @@ -500,66 +564,14 @@ void scsi_scan(int mode) for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { pccb->target = i; for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { - pccb->lun = lun; - pccb->pdata = (unsigned char *)&tempbuff; - pccb->datalen = 512; - scsi_setup_inquiry(pccb); - if (scsi_exec(pccb) != true) { - if (pccb->contr_stat == SCSI_SEL_TIME_OUT) { - /* - * selection timeout => assuming no - * device present - */ - debug("Selection timeout ID %d\n", - pccb->target); - continue; - } - scsi_print_error(pccb); + ret = scsi_detect_dev(pccb, + &scsi_dev_desc[scsi_max_devs], + lun); + if (ret) continue; - } - perq = tempbuff[0]; - modi = tempbuff[1]; - if ((perq & 0x1f) == 0x1f) - continue; /* skip unknown devices */ - if ((modi & 0x80) == 0x80) /* drive is removable */ - scsi_dev_desc[scsi_max_devs].removable = true; - /* get info for this device */ - scsi_ident_cpy((unsigned char *)&scsi_dev_desc - [scsi_max_devs].vendor[0], - &tempbuff[8], 8); - scsi_ident_cpy((unsigned char *)&scsi_dev_desc - [scsi_max_devs].product[0], - &tempbuff[16], 16); - scsi_ident_cpy((unsigned char *)&scsi_dev_desc - [scsi_max_devs].revision[0], - &tempbuff[32], 4); - scsi_dev_desc[scsi_max_devs].target = pccb->target; - scsi_dev_desc[scsi_max_devs].lun = pccb->lun; - - pccb->datalen = 0; - scsi_setup_test_unit_ready(pccb); - if (scsi_exec(pccb) != true) { - if (scsi_dev_desc[scsi_max_devs].removable) { - scsi_dev_desc[scsi_max_devs].type = - perq; - goto removable; - } - scsi_print_error(pccb); - continue; - } - if (scsi_read_capacity(pccb, &capacity, &blksz)) { - scsi_print_error(pccb); - continue; - } - scsi_dev_desc[scsi_max_devs].lba = capacity; - scsi_dev_desc[scsi_max_devs].blksz = blksz; - scsi_dev_desc[scsi_max_devs].log2blksz = - LOG2(scsi_dev_desc[scsi_max_devs].blksz); - scsi_dev_desc[scsi_max_devs].type = perq; - part_init(&scsi_dev_desc[scsi_max_devs]); -removable: + if (mode == 1) { - printf(" Device %d: ", scsi_max_devs); + printf(" Device %d: ", 0); dev_print(&scsi_dev_desc[scsi_max_devs]); } /* if mode */ scsi_max_devs++;
Hi Michal,
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
The patch enables running detection algorithm on block device description structure.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 134 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 73 insertions(+), 61 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/common/scsi.c b/common/scsi.c index 0bce91dfa099..a02c7a505639 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,15 +480,79 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) #endif }
+static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc, int lun)
Can you comment this function?
+{
unsigned char perq, modi;lbaint_t capacity;unsigned long blksz;pccb->lun = lun;pccb->pdata = (unsigned char *)&tempbuff;pccb->datalen = 512;scsi_setup_inquiry(pccb);if (scsi_exec(pccb) != true) {if (pccb->contr_stat == SCSI_SEL_TIME_OUT) {/** selection timeout => assuming no* device present*/debug("Selection timeout ID %d\n",pccb->target);return -ETIMEDOUT;}scsi_print_error(pccb);return -ENODEV;}perq = tempbuff[0];modi = tempbuff[1];if ((perq & 0x1f) == 0x1f)return -ENODEV; /* skip unknown devices */if ((modi & 0x80) == 0x80) /* drive is removable */dev_desc->removable = true;/* get info for this device */scsi_ident_cpy((unsigned char *)&dev_desc[0].vendor[0],&tempbuff[8], 8);scsi_ident_cpy((unsigned char *)&dev_desc[0].product[0],
Can you use more of the line here? You have 80 columns!
&tempbuff[16], 16);scsi_ident_cpy((unsigned char *)&dev_desc[0].revision[0],&tempbuff[32], 4);dev_desc->target = pccb->target;dev_desc->lun = pccb->lun;pccb->datalen = 0;scsi_setup_test_unit_ready(pccb);if (scsi_exec(pccb) != true) {if (dev_desc->removable) {dev_desc->type = perq;goto removable;}scsi_print_error(pccb);return -EINVAL;}if (scsi_read_capacity(pccb, &capacity, &blksz)) {scsi_print_error(pccb);return -EINVAL;
Should you not return the error from scsi_read_capacity()?
}dev_desc->lba = capacity;dev_desc->blksz = blksz;dev_desc->log2blksz = LOG2(dev_desc->blksz);dev_desc->type = perq;part_init(&dev_desc[0]);+removable:
return 0;+}
[...]
Regards, Simon
On 19.11.2016 14:48, Simon Glass wrote:
Hi Michal,
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
The patch enables running detection algorithm on block device description structure.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 134 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 73 insertions(+), 61 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/common/scsi.c b/common/scsi.c index 0bce91dfa099..a02c7a505639 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,15 +480,79 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) #endif }
+static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc, int lun)
Can you comment this function?
will do.
+{
unsigned char perq, modi;lbaint_t capacity;unsigned long blksz;pccb->lun = lun;pccb->pdata = (unsigned char *)&tempbuff;pccb->datalen = 512;scsi_setup_inquiry(pccb);if (scsi_exec(pccb) != true) {if (pccb->contr_stat == SCSI_SEL_TIME_OUT) {/** selection timeout => assuming no* device present*/debug("Selection timeout ID %d\n",pccb->target);return -ETIMEDOUT;}scsi_print_error(pccb);return -ENODEV;}perq = tempbuff[0];modi = tempbuff[1];if ((perq & 0x1f) == 0x1f)return -ENODEV; /* skip unknown devices */if ((modi & 0x80) == 0x80) /* drive is removable */dev_desc->removable = true;/* get info for this device */scsi_ident_cpy((unsigned char *)&dev_desc[0].vendor[0],&tempbuff[8], 8);scsi_ident_cpy((unsigned char *)&dev_desc[0].product[0],Can you use more of the line here? You have 80 columns!
will fix.
&tempbuff[16], 16);scsi_ident_cpy((unsigned char *)&dev_desc[0].revision[0],&tempbuff[32], 4);dev_desc->target = pccb->target;dev_desc->lun = pccb->lun;pccb->datalen = 0;scsi_setup_test_unit_ready(pccb);if (scsi_exec(pccb) != true) {if (dev_desc->removable) {dev_desc->type = perq;goto removable;}scsi_print_error(pccb);return -EINVAL;}if (scsi_read_capacity(pccb, &capacity, &blksz)) {scsi_print_error(pccb);return -EINVAL;Should you not return the error from scsi_read_capacity()?
scsi_read_capacity returns 1 or 0 error value which is just useless.
Thanks, Michal
Prepare LUN(Logical unit number) directly in block description structure and reuse it.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/scsi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index a02c7a505639..c8d43c5b188d 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,13 +480,13 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) #endif }
-static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc, int lun) +static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc) { unsigned char perq, modi; lbaint_t capacity; unsigned long blksz;
- pccb->lun = lun; + pccb->lun = dev_desc->lun; pccb->pdata = (unsigned char *)&tempbuff; pccb->datalen = 512; scsi_setup_inquiry(pccb); @@ -564,9 +564,9 @@ void scsi_scan(int mode) for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { pccb->target = i; for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { + scsi_dev_desc[scsi_max_devs].lun = lun; ret = scsi_detect_dev(pccb, - &scsi_dev_desc[scsi_max_devs], - lun); + &scsi_dev_desc[scsi_max_devs]); if (ret) continue;
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
Prepare LUN(Logical unit number) directly in block description structure and reuse it.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
pccb is pointer to temporary buffer which is used only for sending command. Make it local as is done in scsi_read/scsi_write.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/scsi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index c8d43c5b188d..4ac31fe8118b 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,12 +480,14 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) #endif }
-static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc) +static int scsi_detect_dev(int target, struct blk_desc *dev_desc) { unsigned char perq, modi; lbaint_t capacity; unsigned long blksz; + ccb *pccb = (ccb *)&tempccb;
+ pccb->target = target; pccb->lun = dev_desc->lun; pccb->pdata = (unsigned char *)&tempbuff; pccb->datalen = 512; @@ -553,7 +555,6 @@ void scsi_scan(int mode) { unsigned char i, lun; int ret; - ccb *pccb = (ccb *)&tempccb;
if (mode == 1) printf("scanning bus for devices...\n"); @@ -562,11 +563,9 @@ void scsi_scan(int mode)
scsi_max_devs = 0; for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { - pccb->target = i; for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { scsi_dev_desc[scsi_max_devs].lun = lun; - ret = scsi_detect_dev(pccb, - &scsi_dev_desc[scsi_max_devs]); + ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs]); if (ret) continue;
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
pccb is pointer to temporary buffer which is used only for sending command. Make it local as is done in scsi_read/scsi_write.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
There is no reason to directly point to static allocated array when we have proper block_dev pointer available via parameter in !CONFIG_BLK. For CONFIG_BLK this is read directly from uclass platdata.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
common/scsi.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index 4ac31fe8118b..be0efc1c3ae5 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -161,43 +161,41 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev); #endif - int device = block_dev->devnum; lbaint_t start, blks; uintptr_t buf_addr; unsigned short smallblks = 0; ccb *pccb = (ccb *)&tempccb; - device &= 0xff;
/* Setup device */ - pccb->target = scsi_dev_desc[device].target; - pccb->lun = scsi_dev_desc[device].lun; + pccb->target = block_dev->target; + pccb->lun = block_dev->lun; buf_addr = (unsigned long)buffer; start = blknr; blks = blkcnt; debug("\nscsi_read: dev %d startblk " LBAF ", blccnt " LBAF " buffer %lx\n", - device, start, blks, (unsigned long)buffer); + block_dev->devnum, start, blks, (unsigned long)buffer); do { pccb->pdata = (unsigned char *)buf_addr; #ifdef CONFIG_SYS_64BIT_LBA if (start > SCSI_LBA48_READ) { unsigned long blocks; blocks = min_t(lbaint_t, blks, SCSI_MAX_READ_BLK); - pccb->datalen = scsi_dev_desc[device].blksz * blocks; + pccb->datalen = block_dev->blksz * blocks; scsi_setup_read16(pccb, start, blocks); start += blocks; blks -= blocks; } else #endif if (blks > SCSI_MAX_READ_BLK) { - pccb->datalen = scsi_dev_desc[device].blksz * + pccb->datalen = block_dev->blksz * SCSI_MAX_READ_BLK; smallblks = SCSI_MAX_READ_BLK; scsi_setup_read_ext(pccb, start, smallblks); start += SCSI_MAX_READ_BLK; blks -= SCSI_MAX_READ_BLK; } else { - pccb->datalen = scsi_dev_desc[device].blksz * blks; + pccb->datalen = block_dev->blksz * blks; smallblks = (unsigned short)blks; scsi_setup_read_ext(pccb, start, smallblks); start += blks; @@ -236,33 +234,30 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev); #endif - int device = block_dev->devnum; lbaint_t start, blks; uintptr_t buf_addr; unsigned short smallblks; ccb *pccb = (ccb *)&tempccb;
- device &= 0xff; - /* Setup device */ - pccb->target = scsi_dev_desc[device].target; - pccb->lun = scsi_dev_desc[device].lun; + pccb->target = block_dev->target; + pccb->lun = block_dev->lun; buf_addr = (unsigned long)buffer; start = blknr; blks = blkcnt; debug("\n%s: dev %d startblk " LBAF ", blccnt " LBAF " buffer %lx\n", - __func__, device, start, blks, (unsigned long)buffer); + __func__, block_dev->devnum, start, blks, (unsigned long)buffer); do { pccb->pdata = (unsigned char *)buf_addr; if (blks > SCSI_MAX_WRITE_BLK) { - pccb->datalen = (scsi_dev_desc[device].blksz * + pccb->datalen = (block_dev->blksz * SCSI_MAX_WRITE_BLK); smallblks = SCSI_MAX_WRITE_BLK; scsi_setup_write_ext(pccb, start, smallblks); start += SCSI_MAX_WRITE_BLK; blks -= SCSI_MAX_WRITE_BLK; } else { - pccb->datalen = scsi_dev_desc[device].blksz * blks; + pccb->datalen = block_dev->blksz * blks; smallblks = (unsigned short)blks; scsi_setup_write_ext(pccb, start, smallblks); start += blks;
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
There is no reason to directly point to static allocated array when we have proper block_dev pointer available via parameter in !CONFIG_BLK. For CONFIG_BLK this is read directly from uclass platdata.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On 18 November 2016 at 08:44, Michal Simek michal.simek@xilinx.com wrote:
Extract block device initialization to specific function.
Signed-off-by: Michal Simek michal.simek@xilinx.com
common/scsi.c | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (2)
-
Michal Simek -
Simon Glass