Hi Mugunthan and Bin, On 14 February 2016 at 20:03, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Sun, Feb 7, 2016 at 4:29 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 3 February 2016 at 04:59, Mugunthan V N <mugunthan...@ti.com> wrote: >>> >>> Implement scsi_init() api to probe driver model based sata >>> devices. >>> >>> Signed-off-by: Mugunthan V N <mugunthan...@ti.com> >>> --- >>> drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 39 insertions(+) >> >> This patch seems odd to me. I would hope that scsi_init() would go >> away with driver model and it would happen as part of the controller >> probe. But of course we cannot probe SCSI from UCLASS_DISK. I think >> the uclass 'disk' is too broad to be useful. >> > > I agree. I raised similar comment before that this just looks a place > holder for the SCSI stuff. > >> So I am wondering whether the decision to use UCLASS_DISK instead of >> UCLASS_AHCI was a mistake. >> >> Perhaps instead we should have: >> >> UCLASS_AHCI >> UCLASS_SCSI >> UCLASS_MMC >> etc... >> > > I still think UCLASS_AHCI is not a good name. Maybe UCLASS_ATA as we > are talking about protocols here (SCSI, MMC).
UCLASS_ATA seems confusing. How would we distinguish IDE and SATA? BTW since your email I have sent a patch to implement block devices. >From what I can tell the above approach will work well. I think our uclasses should mirror the current IF_TYPE values: /* Interface types: */ #define IF_TYPE_UNKNOWN 0 #define IF_TYPE_IDE 1 #define IF_TYPE_SCSI 2 #define IF_TYPE_ATAPI 3 #define IF_TYPE_USB 4 #define IF_TYPE_DOC 5 #define IF_TYPE_MMC 6 #define IF_TYPE_SD 7 #define IF_TYPE_SATA 8 #define IF_TYPE_HOST 9 > >> and each of these devices can have a UCLASS_BLK under them (the block >> device). >> >> Possibly we could even have a dummy UCLASS_DISK device under each of >> the above, but I'm not sure that is useful. >> >> What do you think? >> > > [snip] > > Regards, > Bin Since this patch is presumably destined for the current release and we don't intend to change UCLASS_DISK for that release, I think this patch can go in as is. We can fix it up later. Also note that scsi_get_device() is not the same as scsi_get_dev(). It would be better to use uclass_get_device() though. Reviewed-by: Simon Glass <s...@chromium.org> Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
