On 01/04/2017 06:22, Simon Glass wrote:
Hi Jean-Jacques,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot <jjhib...@ti.com> wrote:
With DM_SCSI enabled, scsi scan suffers 2 problems:
* blk_create_devicef is called with blkz = 0, leading to a divide-by-0
exception
* new blk devices are created at each scan.
To fix this we start by removing all known SCSI devices at the beginning
of the scan. Then a detection process is started for each possible
device only to get the blksz and lba of the device (no call to part_init() yet).
If a device is found, we can call blk_create_devicef() with the right
parameters and continue the process.
Signed-off-by: Jean-Jacques Hiblot <jjhib...@ti.com>
---
common/scsi.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c
index fb5b407..3385cc2 100644
--- a/common/scsi.c
+++ b/common/scsi.c
@@ -480,7 +480,7 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc,
int devnum)
*
* Return: 0 on success, error value otherwise
*/
-static int scsi_detect_dev(int target, struct blk_desc *dev_desc)
+static int scsi_detect_dev(int target, struct blk_desc *dev_desc, int
init_part)
{
unsigned char perq, modi;
lbaint_t capacity;
@@ -539,7 +539,8 @@ static int scsi_detect_dev(int target, struct blk_desc
*dev_desc)
dev_desc->blksz = blksz;
dev_desc->log2blksz = LOG2(dev_desc->blksz);
dev_desc->type = perq;
- part_init(&dev_desc[0]);
+ if (init_part)
+ part_init(&dev_desc[0]);
Do you need this in here? The caller could just do it and avoid the
extra parameter.
I simply tried as much as possible to avoid code duplication. But agreed
it's not a very elegant solution. I'll rework this. Thanks again for the
comments
removable:
return 0;
}
@@ -565,6 +566,9 @@ int scsi_scan(int mode)
if (ret)
return ret;
+ /* remove all SCSI interfaces since we're going to (re)create them */
+ blk_unbind_all(IF_TYPE_SCSI);
+
This seems to already be done a few lines up...is it needed?
No. It's just a rebase issue. I started the work on v2017.01 and didn't
see the introduction of the unbind.
uclass_foreach_dev(dev, uc) {
struct scsi_platdata *plat; /* scsi controller platdata */
@@ -580,9 +584,20 @@ int scsi_scan(int mode)
for (lun = 0; lun < plat->max_lun; lun++) {
struct udevice *bdev; /* block device */
/* block device description */
+ struct blk_desc _bd;
struct blk_desc *bdesc;
char str[10];
+ scsi_init_dev_desc_priv(&_bd);
+ _bd.lun = lun;
+ ret = scsi_detect_dev(i, &_bd, 0);
+ if (ret)
+ /*
+ * no device detected?
+ * check the next lun.
+ */
+ continue;
+
/*
* Create only one block device and do
detection
* to make sure that there won't be a lot of
@@ -590,17 +605,25 @@ int scsi_scan(int mode)
*/
snprintf(str, sizeof(str), "id%dlun%d", i,
lun);
ret = blk_create_devicef(dev, "scsi_blk",
- str, IF_TYPE_SCSI,
- -1, 0, 0, &bdev);
+ str, IF_TYPE_SCSI,
+ -1,
+ _bd.blksz,
+ _bd.blksz * _bd.lba,
+ &bdev);
But why put these in here when...
if (ret) {
debug("Can't create device\n");
return ret;
}
bdesc = dev_get_uclass_platdata(bdev);
+ /*
+ * perform the detection once more but this
+ * time, let's initialize do the initialization
+ * of the partitions
+ */
scsi_init_dev_desc_priv(bdesc);
...they are overwritten here?
bdesc->lun = lun;
- ret = scsi_detect_dev(i, bdesc);
+ ret = scsi_detect_dev(i, bdesc, 1);
if (ret) {
device_unbind(bdev);
continue;
@@ -631,7 +654,7 @@ int scsi_scan(int mode)
for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) {
for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) {
scsi_dev_desc[scsi_max_devs].lun = lun;
- ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs]);
+ ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs],
1);
if (ret)
continue;
Call part_init() here?
--
1.9.1
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
