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

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

2016-11-23 Thread Simon Glass

Hi Michal,

On 21 November 2016 at 12:36, Michal Simek  wrote:
> On 24.9.2016 19:26, Simon Glass wrote:
>> Hi Michal,
>>
>> On 8 September 2016 at 07:57, Michal Simek  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 
>>> ---
>>>
>>>  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.

I think this is reasonable. Ideally you could avoid creating the block
device until you know it is OK, but failing that, make sure to unbind
it.

>
>  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, );
>  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,
>

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

2016-11-21 Thread Michal Simek

On 24.9.2016 19:26, Simon Glass wrote:
> Hi Michal,
> 
> On 8 September 2016 at 07:57, Michal Simek  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 
>> ---
>>
>>  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, );
 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, );
if (ret) {
printf("Cannot create block 
device\n");
return;
}
last_dev_num = dev_num;

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

2016-09-26 Thread Michal Simek

Hi Simon,

On 27.9.2016 02:35, Simon Glass wrote:
> Hi Michal,
> 
> On 26 September 2016 at 05:06, Michal Simek  wrote:
>> On 24.9.2016 19:26, Simon Glass wrote:
>>> Hi Michal,
>>>
>>> On 8 September 2016 at 07:57, Michal Simek  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 
 ---

  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.
>>
>> TBH I am confused with that too. This is ceva sata driver
>> but we use scsi subsystem to work with it.
>> From my look sata is mostly copied from scsi but I don't know history of
>> it.
>> I will look at using just one interface - sata or scsi to see how this
>> will look like.
> 
> Here is my understanding. I may have it wrong.
> 
> SCSI should be a uclass
> SATA should be a uclass (called AHCI)
> 
> SCSI provide a library of functions which can be called by SCSI or
> SATA code. This is distinct from the uclass so really should be in
> some sort of common file.

will look at it.

> 
>>
>>
>>> 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().
>>
>> Will look.
>>
>>>
>>> Also we will need a sandbox device at some point so we can run tests.
>>
>> This can be added later.
>>
>>>
>>> Minor point - please put #idef CONFIG_DM_SATA first and the legacy
>>> path in the #else cause. Mostly you do this but in a few cases it is
>>> not consistent.
>>
>> ok. Will look at it.
>>
>>>
>>> A few more notes below.
>>>

 diff --git a/cmd/scsi.c b/cmd/scsi.c
 index 387ca1a262ab..dc1176610672 100644
 --- a/cmd/scsi.c
 +++ b/cmd/scsi.c
 @@ -10,6 +10,7 @@
   */
  #include 
  #include 
 +#include 
  #include 

  static int scsi_curr_dev; /* current device */
 @@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc, 
 char *const argv[])
  /*
   * scsi command intepreter
   */
 +#ifdef CONFIG_DM_SATA
 +struct udevice *find_scsi_device(int dev_num)
 +{
 +   struct udevice *bdev;
 +   int ret;
 +
 +   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
 +
 +   if (ret) {
 +   printf("SCSI Device %d not found\n", dev_num);
 +   return NULL;
 +   }
 +
 +   return bdev;
 +}
 +#endif
 +
  int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
  {
 switch (argc) {
 @@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
 *const argv[])
 if (strncmp(argv[1], "res", 3) == 0) {
 printf("\nReset SCSI\n");
 scsi_bus_reset();
 +
 +#if defined(CONFIG_DM_SATA)
 +   struct udevice *bdev;
 +
 +   bdev = find_scsi_device(scsi_curr_dev);
 +   if (!bdev)
 +   return CMD_RET_FAILURE;
 +

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

2016-09-26 Thread Simon Glass

Hi Michal,

On 26 September 2016 at 05:06, Michal Simek  wrote:
> On 24.9.2016 19:26, Simon Glass wrote:
>> Hi Michal,
>>
>> On 8 September 2016 at 07:57, Michal Simek  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 
>>> ---
>>>
>>>  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.
>
> TBH I am confused with that too. This is ceva sata driver
> but we use scsi subsystem to work with it.
> From my look sata is mostly copied from scsi but I don't know history of
> it.
> I will look at using just one interface - sata or scsi to see how this
> will look like.

Here is my understanding. I may have it wrong.

SCSI should be a uclass
SATA should be a uclass (called AHCI)

SCSI provide a library of functions which can be called by SCSI or
SATA code. This is distinct from the uclass so really should be in
some sort of common file.

>
>
>> 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().
>
> Will look.
>
>>
>> Also we will need a sandbox device at some point so we can run tests.
>
> This can be added later.
>
>>
>> Minor point - please put #idef CONFIG_DM_SATA first and the legacy
>> path in the #else cause. Mostly you do this but in a few cases it is
>> not consistent.
>
> ok. Will look at it.
>
>>
>> A few more notes below.
>>
>>>
>>> diff --git a/cmd/scsi.c b/cmd/scsi.c
>>> index 387ca1a262ab..dc1176610672 100644
>>> --- a/cmd/scsi.c
>>> +++ b/cmd/scsi.c
>>> @@ -10,6 +10,7 @@
>>>   */
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  static int scsi_curr_dev; /* current device */
>>> @@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc, 
>>> char *const argv[])
>>>  /*
>>>   * scsi command intepreter
>>>   */
>>> +#ifdef CONFIG_DM_SATA
>>> +struct udevice *find_scsi_device(int dev_num)
>>> +{
>>> +   struct udevice *bdev;
>>> +   int ret;
>>> +
>>> +   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
>>> +
>>> +   if (ret) {
>>> +   printf("SCSI Device %d not found\n", dev_num);
>>> +   return NULL;
>>> +   }
>>> +
>>> +   return bdev;
>>> +}
>>> +#endif
>>> +
>>>  int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>>  {
>>> switch (argc) {
>>> @@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
>>> *const argv[])
>>> if (strncmp(argv[1], "res", 3) == 0) {
>>> printf("\nReset SCSI\n");
>>> scsi_bus_reset();
>>> +
>>> +#if defined(CONFIG_DM_SATA)
>>> +   struct udevice *bdev;
>>> +
>>> +   bdev = find_scsi_device(scsi_curr_dev);
>>> +   if (!bdev)
>>> +   return CMD_RET_FAILURE;
>>> +
>>> +   scsi_scan(1, bdev);
>>> +#else
>>> scsi_scan(1);
>>> +#endif
>>> return 0;
>>> }
>>> if (strncmp(argv[1], "inf", 3) ==

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

2016-09-26 Thread Michal Simek

On 24.9.2016 19:26, Simon Glass wrote:
> Hi Michal,
> 
> On 8 September 2016 at 07:57, Michal Simek  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 
>> ---
>>
>>  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.

TBH I am confused with that too. This is ceva sata driver
but we use scsi subsystem to work with it.
>From my look sata is mostly copied from scsi but I don't know history of
it.
I will look at using just one interface - sata or scsi to see how this
will look like.


> 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().

Will look.

> 
> Also we will need a sandbox device at some point so we can run tests.

This can be added later.

> 
> Minor point - please put #idef CONFIG_DM_SATA first and the legacy
> path in the #else cause. Mostly you do this but in a few cases it is
> not consistent.

ok. Will look at it.

> 
> A few more notes below.
> 
>>
>> diff --git a/cmd/scsi.c b/cmd/scsi.c
>> index 387ca1a262ab..dc1176610672 100644
>> --- a/cmd/scsi.c
>> +++ b/cmd/scsi.c
>> @@ -10,6 +10,7 @@
>>   */
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  static int scsi_curr_dev; /* current device */
>> @@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc, 
>> char *const argv[])
>>  /*
>>   * scsi command intepreter
>>   */
>> +#ifdef CONFIG_DM_SATA
>> +struct udevice *find_scsi_device(int dev_num)
>> +{
>> +   struct udevice *bdev;
>> +   int ret;
>> +
>> +   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
>> +
>> +   if (ret) {
>> +   printf("SCSI Device %d not found\n", dev_num);
>> +   return NULL;
>> +   }
>> +
>> +   return bdev;
>> +}
>> +#endif
>> +
>>  int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>>  {
>> switch (argc) {
>> @@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
>> *const argv[])
>> if (strncmp(argv[1], "res", 3) == 0) {
>> printf("\nReset SCSI\n");
>> scsi_bus_reset();
>> +
>> +#if defined(CONFIG_DM_SATA)
>> +   struct udevice *bdev;
>> +
>> +   bdev = find_scsi_device(scsi_curr_dev);
>> +   if (!bdev)
>> +   return CMD_RET_FAILURE;
>> +
>> +   scsi_scan(1, bdev);
>> +#else
>> scsi_scan(1);
>> +#endif
>> return 0;
>> }
>> if (strncmp(argv[1], "inf", 3) == 0) {
>> @@ -51,7 +80,16 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
>> *const argv[])
>> return 0;
>> }
>> if (strncmp(argv[1], "scan", 4) == 0) {
>> +#if defined(CONFIG_DM_SATA)
>> +   struct udevice *bdev;
>> +
>> +   bdev = find_scsi_device(scsi_curr_dev);
>> +   if (!bdev)
>> +   return CMD_RET_FAILURE;
>> +

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

2016-09-24 Thread Simon Glass

Hi Michal,

On 8 September 2016 at 07:57, Michal Simek  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 
> ---
>
>  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().

Also we will need a sandbox device at some point so we can run tests.

Minor point - please put #idef CONFIG_DM_SATA first and the legacy
path in the #else cause. Mostly you do this but in a few cases it is
not consistent.

A few more notes below.

>
> diff --git a/cmd/scsi.c b/cmd/scsi.c
> index 387ca1a262ab..dc1176610672 100644
> --- a/cmd/scsi.c
> +++ b/cmd/scsi.c
> @@ -10,6 +10,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>
>  static int scsi_curr_dev; /* current device */
> @@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc, char 
> *const argv[])
>  /*
>   * scsi command intepreter
>   */
> +#ifdef CONFIG_DM_SATA
> +struct udevice *find_scsi_device(int dev_num)
> +{
> +   struct udevice *bdev;
> +   int ret;
> +
> +   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
> +
> +   if (ret) {
> +   printf("SCSI Device %d not found\n", dev_num);
> +   return NULL;
> +   }
> +
> +   return bdev;
> +}
> +#endif
> +
>  int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>  {
> switch (argc) {
> @@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
> *const argv[])
> if (strncmp(argv[1], "res", 3) == 0) {
> printf("\nReset SCSI\n");
> scsi_bus_reset();
> +
> +#if defined(CONFIG_DM_SATA)
> +   struct udevice *bdev;
> +
> +   bdev = find_scsi_device(scsi_curr_dev);
> +   if (!bdev)
> +   return CMD_RET_FAILURE;
> +
> +   scsi_scan(1, bdev);
> +#else
> scsi_scan(1);
> +#endif
> return 0;
> }
> if (strncmp(argv[1], "inf", 3) == 0) {
> @@ -51,7 +80,16 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
> *const argv[])
> return 0;
> }
> if (strncmp(argv[1], "scan", 4) == 0) {
> +#if defined(CONFIG_DM_SATA)
> +   struct udevice *bdev;
> +
> +   bdev = find_scsi_device(scsi_curr_dev);
> +   if (!bdev)
> +   return CMD_RET_FAILURE;
> +   scsi_scan(1, bdev);
> +#else
> scsi_scan(1);
> +#endif
> return 0;
> }
> if (strncmp(argv[1], "part", 4) == 0) {
> diff --git a/common/board_r.c b/common/board_r.c
> index d959ad3c6f90..f3ea457507de 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -620,7 +620,7 @@ static int initr_ambapp_print(void)
>  }
>  #endif
>
> -#if defined(CONFIG_SCSI)
> +#if defined(CONFIG_SCSI) && !defined(CONFIG_DM_SATA)
>  static int

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

2016-09-23 Thread Michal Simek

Hi Simon,

2016-09-08 15:57 GMT+02:00 Michal Simek :

> 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 
> ---
>
>  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(-)
>
> diff --git a/cmd/scsi.c b/cmd/scsi.c
> index 387ca1a262ab..dc1176610672 100644
> --- a/cmd/scsi.c
> +++ b/cmd/scsi.c
> @@ -10,6 +10,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>
>  static int scsi_curr_dev; /* current device */
> @@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc,
> char *const argv[])
>  /*
>   * scsi command intepreter
>   */
> +#ifdef CONFIG_DM_SATA
> +struct udevice *find_scsi_device(int dev_num)
> +{
> +   struct udevice *bdev;
> +   int ret;
> +
> +   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
> +
> +   if (ret) {
> +   printf("SCSI Device %d not found\n", dev_num);
> +   return NULL;
> +   }
> +
> +   return bdev;
> +}
> +#endif
> +
>  int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>  {
> switch (argc) {
> @@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char
> *const argv[])
> if (strncmp(argv[1], "res", 3) == 0) {
> printf("\nReset SCSI\n");
> scsi_bus_reset();
> +
> +#if defined(CONFIG_DM_SATA)
> +   struct udevice *bdev;
> +
> +   bdev = find_scsi_device(scsi_curr_dev);
> +   if (!bdev)
> +   return CMD_RET_FAILURE;
> +
> +   scsi_scan(1, bdev);
> +#else
> scsi_scan(1);
> +#endif
> return 0;
> }
> if (strncmp(argv[1], "inf", 3) == 0) {
> @@ -51,7 +80,16 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char
> *const argv[])
> return 0;
> }
> if (strncmp(argv[1], "scan", 4) == 0) {
> +#if defined(CONFIG_DM_SATA)
> +   struct udevice *bdev;
> +
> +   bdev = find_scsi_device(scsi_curr_dev);
> +   if (!bdev)
> +   return CMD_RET_FAILURE;
> +   scsi_scan(1, bdev);
> +#else
> scsi_scan(1);
> +#endif
> return 0;
> }
> if (strncmp(argv[1], "part", 4) == 0) {
> diff --git a/common/board_r.c b/common/board_r.c
> index d959ad3c6f90..f3ea457507de 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -620,7 +620,7 @@ static int initr_ambapp_print(void)
>  }
>  #endif
>
> -#if defined(CONFIG_SCSI)
> +#if defined(CONFIG_SCSI) && !defined(CONFIG_DM_SATA)
>  static int initr_scsi(void)
>  {
> puts("SCSI:  ");
> @@ -923,7 +923,7 @@ init_fnc_t init_sequence_r[] = {
> initr_ambapp_print,
>  #endif
>  #endif
> -#ifdef CONFIG_SCSI
> +#if defined(CONFIG_SCSI) && !defined(CONFIG_DM_SATA)
> INIT_FUNC_WATCHDOG_RESET
> initr_scsi,
>  #endif
> diff --git a/common/scsi.c b/common/scsi.c
> index dbbf4043b22a..1726898b06e2 100644
> --- a/common/scsi.c
> +++ b/common/scsi.c
> @@ -26,7 +26,7 @@
>  #define SCSI_VEND_ID 0x10b9
>  #define SCSI_DEV_ID  0x5288
>
> -#elif !defined(CONFIG_SCSI_AHCI_PLAT)
> +#elif !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA)
>  #error no scsi device defined
>  #endif
>  #define SCSI_DEV_LIST {SCSI_VEND_ID, SCSI_DEV_ID}
> @@ -43,7 +43,13 @@ static int scsi_max_devs; /* number of highest
> available scsi device */
>
>  static int scsi_curr_dev; /* current device */
>
> +#if !defined(CONFIG_DM_SATA)
>  static

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

2016-09-08 Thread Michal Simek

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

 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(-)

diff --git a/cmd/scsi.c b/cmd/scsi.c
index 387ca1a262ab..dc1176610672 100644
--- a/cmd/scsi.c
+++ b/cmd/scsi.c
@@ -10,6 +10,7 @@
  */
 #include 
 #include 
+#include 
 #include 
 
 static int scsi_curr_dev; /* current device */
@@ -25,6 +26,23 @@ int do_scsiboot(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
 /*
  * scsi command intepreter
  */
+#ifdef CONFIG_DM_SATA
+struct udevice *find_scsi_device(int dev_num)
+{
+   struct udevice *bdev;
+   int ret;
+
+   ret = blk_get_device(IF_TYPE_SCSI, dev_num, );
+
+   if (ret) {
+   printf("SCSI Device %d not found\n", dev_num);
+   return NULL;
+   }
+
+   return bdev;
+}
+#endif
+
 int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
switch (argc) {
@@ -35,7 +53,18 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
if (strncmp(argv[1], "res", 3) == 0) {
printf("\nReset SCSI\n");
scsi_bus_reset();
+
+#if defined(CONFIG_DM_SATA)
+   struct udevice *bdev;
+
+   bdev = find_scsi_device(scsi_curr_dev);
+   if (!bdev)
+   return CMD_RET_FAILURE;
+
+   scsi_scan(1, bdev);
+#else
scsi_scan(1);
+#endif
return 0;
}
if (strncmp(argv[1], "inf", 3) == 0) {
@@ -51,7 +80,16 @@ int do_scsi(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
return 0;
}
if (strncmp(argv[1], "scan", 4) == 0) {
+#if defined(CONFIG_DM_SATA)
+   struct udevice *bdev;
+
+   bdev = find_scsi_device(scsi_curr_dev);
+   if (!bdev)
+   return CMD_RET_FAILURE;
+   scsi_scan(1, bdev);
+#else
scsi_scan(1);
+#endif
return 0;
}
if (strncmp(argv[1], "part", 4) == 0) {
diff --git a/common/board_r.c b/common/board_r.c
index d959ad3c6f90..f3ea457507de 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -620,7 +620,7 @@ static int initr_ambapp_print(void)
 }
 #endif
 
-#if defined(CONFIG_SCSI)
+#if defined(CONFIG_SCSI) && !defined(CONFIG_DM_SATA)
 static int initr_scsi(void)
 {
puts("SCSI:  ");
@@ -923,7 +923,7 @@ init_fnc_t init_sequence_r[] = {
initr_ambapp_print,
 #endif
 #endif
-#ifdef CONFIG_SCSI
+#if defined(CONFIG_SCSI) && !defined(CONFIG_DM_SATA)
INIT_FUNC_WATCHDOG_RESET
initr_scsi,
 #endif
diff --git a/common/scsi.c b/common/scsi.c
index dbbf4043b22a..1726898b06e2 100644
--- a/common/scsi.c
+++ b/common/scsi.c
@@ -26,7 +26,7 @@
 #define SCSI_VEND_ID 0x10b9
 #define SCSI_DEV_ID  0x5288
 
-#elif !defined(CONFIG_SCSI_AHCI_PLAT)
+#elif !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA)
 #error no scsi device defined
 #endif
 #define SCSI_DEV_LIST {SCSI_VEND_ID, SCSI_DEV_ID}
@@ -43,7 +43,13 @@ static int scsi_max_devs; /* number of highest available 
scsi device */
 
 static int scsi_curr_dev; /* current device */
 
+#if !defined(CONFIG_DM_SATA)
 static struct blk_desc scsi_dev_desc[CONFIG_SYS_SCSI_MAX_DEVICE];
+#else
+#undef CONFIG_SYS_SCSI_MAX_DEVICE
+#define CONFIG_SYS_SCSI_MAX_DEVICE 1
+#define CONFIG_SYS_SCSI_MAX_LUN1
+#endif
 
 /* almost the maximum amount of the scsi_ext command.. */
 #define SCSI_MAX_READ_BLK 0x
@@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev,

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

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

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

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

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

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

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

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

8 matches

Site Navigation

Mail list logo

Footer information