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.
> 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?
> 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
