[U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices

[Michal Simek]

On 24.9.2016 19:26, Simon Glass wrote:
> Hi Michal,
> 
> On 8 September 2016 at 07:57, Michal Simek <michal.simek at xilinx.com> wrote:
>> All sata based drivers are bind and corresponding block
>> device is created. Based on this find_scsi_device() is able
>> to get back block device based on scsi_curr_dev pointer.
>>
>> intr_scsi() is commented now but it can be replaced by calling
>> find_scsi_device() and scsi_scan().
>>
>> scsi_dev_desc[] is commented out but common/scsi.c heavily depends on
>> it. That's why CONFIG_SYS_SCSI_MAX_DEVICE is hardcoded to 1 and symbol
>> is reassigned to a block description allocated by uclass.
>> There is only one block description by device now but it doesn't need to
>> be correct when more devices are present.
>>
>> scsi_bind() ensures corresponding block device creation.
>> uclass post_probe (scsi_post_probe()) is doing low level init.
>>
>> SCSI/SATA DM based drivers requires to have 64bit base address as
>> the first entry in platform data structure to setup mmio_base.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>>  cmd/scsi.c                  | 38 ++++++++++++++++++++++++++++++++++++++
>>  common/board_r.c            |  4 ++--
>>  common/scsi.c               | 17 ++++++++++++++++-
>>  drivers/block/ahci-uclass.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  drivers/block/ahci.c        | 30 ++++++++++++++++++++++--------
>>  include/ahci.h              |  2 +-
>>  include/sata.h              |  3 +++
>>  include/scsi.h              | 15 ++++++++++++++-
>>  8 files changed, 134 insertions(+), 13 deletions(-)
> 
> Thanks for looking at this. I've taken a look and have a few comments.
> 
> It's confusing that you are changing both scsi and sata. Do you need
> to add a DM_SCSI also? As far as I can see, they are separate
> subsystems.
> 
> I think you need a uclass which implements the scsi_scan() function.
> The existing code could be refactored so that the common parts are
> called from both scsi.c and your scsi-uclass.c. It should look for
> devices, and then create a block device for each. Since you don't know
> how many block devices to create, I don't think you can avoid creating
> them 'on the fly' in scsi_scan(). For an example, see
> usb_stor_probe_device().

Just a question about this. I have played with this and I haven't looked
at usb (because I have usb ports gone on my development platform) but
based on my understanding of our controller (ceva-sata) we support 2
ports where each of them can have 2 different IDs and I am not quite
sure about LUN but hardcoding it to 1 should be fine for now.

But all this depends on controller setup. Number of ports should be
possible to detect from every controller (0x0 offset - cap register 4:0
number of ports NP).

I have this code. What do you think about it?
(There is missing loop over number of ports which I will add but
unfortunately I don't have a HW with this setup here.

 void scsi_scan(int mode)
 {
 	unsigned char i, lun;
 	struct uclass *uc;
         struct udevice *dev; /* SCSI controller */
 	int ret;

 	if (mode == 1)
 		printf("scanning bus for devices...\n");

         ret = uclass_get(UCLASS_SCSI, &uc);
         if (ret)
                 return;

         uclass_foreach_dev(dev, uc) {
 		struct scsi_platdata *plat; /* scsi controller platdata */
 		struct blk_desc *bdesc; /* block device description */
 		struct udevice *bdev; /* block device */
 		struct udevice **devp;
 		int dev_num = 0;
 		int last_dev_num = -1;

 		/* probe SCSI controller driver */
 		ret = uclass_get_device_tail(dev, 0, devp);
 		if (ret)
 			return;

 		/* Get controller platdata */
 		plat = dev_get_platdata(dev);

 		for (i = 0; i < plat->max_id; i++) {
 			for (lun = 0; lun < plat->max_lun; lun++) {
 				/*
 				 * Create only one block device and do detection
 				 * to make sure that there won't be a lot of
 				 * block devices created
 				 */
 				if (last_dev_num != dev_num) {
 					char str[10];
 					snprintf(str, sizeof(str), "lun%d", lun);
 					ret = blk_create_devicef(dev, "scsi_blk", str, IF_TYPE_SCSI, -1, 512,
 								dev_num, &bdev);
 					if (ret) {
 						printf("Cannot create block device\n");
 						return;
 					}
 					last_dev_num = dev_num;
 					bdesc = dev_get_uclass_platdata(bdev);
 				}

 				scsi_init_dev_desc(bdesc, dev_num);
 				bdesc->lun = lun;
 				ret = scsi_detect_dev(i, bdesc);
 				if (ret)
 					continue;

 				if (mode == 1) {
 					printf("  Device %d: ", 0);
 					dev_print(bdesc);
 					dev_num++;
 				} /* if mode */
 			} /* next LUN */
 		}
 	}
 }


Thanks,
Michal