On 9 April 2017 at 13:27, Simon Glass <s...@chromium.org> wrote:
> Hi,
>
> On 7 April 2017 at 05:42, Jean-Jacques Hiblot <jjhib...@ti.com> wrote:
>> With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading
>> to a divide-by-0 exception.
>> scsi_detect_dev() can be used to get the required parameters (block size
>> and number of blocks) from the drive before calling blk_create_devicef().
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhib...@ti.com>
>> ---
>> common/scsi.c | 35 ++++++++++++++++++++++++++---------
>> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Please see below.
>
>>
>> diff --git a/common/scsi.c b/common/scsi.c
>> index 972ef338..d37222c 100644
>> --- a/common/scsi.c
>> +++ b/common/scsi.c
>> @@ -580,9 +580,19 @@ 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);
>> + ret = scsi_detect_dev(i, lun, &_bd);
>> + 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,20 +600,27 @@ 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);
>> if (ret) {
>> debug("Can't create device\n");
>> return ret;
>> }
>> - bdesc = dev_get_uclass_platdata(bdev);
>>
>> - scsi_init_dev_desc_priv(bdesc);
>> - ret = scsi_detect_dev(i, lun, bdesc);
>> - if (ret) {
>> - device_unbind(bdev);
>> - continue;
>> - }
>> + bdesc = dev_get_uclass_platdata(bdev);
>> + bdesc->target = i;
>> + bdesc->lun = lun;
>> + bdesc->removable = _bd.removable;
>> + bdesc->type = _bd.type;
>> + memcpy(&bdesc->vendor, &_bd.vendor,
>> + sizeof(_bd.vendor));
>> + memcpy(&bdesc->product, &_bd.product,
>> + sizeof(_bd.product));
>> + memcpy(&bdesc->revision, &_bd.revision,
>> + sizeof(_bd.revision));
>
> Can you please move this block (inside the double for loops) into a
> separate function? It is getting too long. A follow-up patch is fine
> since you have already done this.
>
>> part_init(bdesc);
>>
>> if (mode == 1) {
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
Applied to u-boot-dm, thanks!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
