[U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
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@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(-)
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 <common.h> #include <command.h> +#include <dm.h> #include <scsi.h>
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, &bdev); + + 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_LUN 1 +#endif
/* almost the maximum amount of the scsi_ext command.. */ #define SCSI_MAX_READ_BLK 0xFFFF @@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev); + struct blk_desc *scsi_dev_desc = block_dev; #endif int device = block_dev->devnum; lbaint_t start, blks; @@ -235,6 +242,7 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev); + struct blk_desc *scsi_dev_desc = block_dev; #endif int device = block_dev->devnum; lbaint_t start, blks; @@ -462,13 +470,20 @@ void scsi_setup_test_unit_ready(ccb *pccb) * (re)-scan the scsi bus and reports scsi device info * to the user if mode = 1 */ +#ifdef CONFIG_DM_SATA +void scsi_scan(int mode, struct udevice *bdev) +#else void scsi_scan(int mode) +#endif { unsigned char i, perq, modi, lun; lbaint_t capacity; unsigned long blksz; ccb *pccb = (ccb *)&tempccb;
+#if defined(CONFIG_DM_SATA) + struct blk_desc *scsi_dev_desc = dev_get_uclass_platdata(bdev); +#endif if (mode == 1) printf("scanning bus for devices...\n"); for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { diff --git a/drivers/block/ahci-uclass.c b/drivers/block/ahci-uclass.c index 7b8c32699f53..f25b1bd336ae 100644 --- a/drivers/block/ahci-uclass.c +++ b/drivers/block/ahci-uclass.c @@ -1,14 +1,52 @@ /* * Copyright (c) 2015 Google, Inc * Written by Simon Glass sjg@chromium.org + * Copyright (c) 2016 Xilinx, Inc + * Written by Michal Simek * * SPDX-License-Identifier: GPL-2.0+ */
#include <common.h> #include <dm.h> +#include <scsi.h> + +#if defined(CONFIG_DM_SATA) +int scsi_bind(struct udevice *dev) +{ + struct udevice *bdev; + struct blk_desc *bdesc; + int ret; + + /* + * FIXME - maybe loop over CONFIG_SYS_SCSI_MAX_SCSI_ID + * here to support more ports + */ + ret = blk_create_devicef(dev, "scsi_blk", "blk", IF_TYPE_SCSI, -1, 512, + 0, &bdev); + if (ret) { + printf("Cannot create block device\n"); + return ret; + } + bdesc = dev_get_uclass_platdata(bdev); + debug("device %p, block device %p, block description %p\n", + dev, bdev, bdesc); + + return 0; +} + +static int scsi_post_probe(struct udevice *dev) +{ + debug("%s: device %p\n", __func__, dev); + scsi_low_level_init(0, dev); + return 0; +} +#endif
UCLASS_DRIVER(ahci) = { .id = UCLASS_AHCI, .name = "ahci", +#if defined(CONFIG_DM_SATA) + .post_probe = scsi_post_probe, +#endif }; diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e3e783a74cfd..03a95179eaa4 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,7 @@ int ahci_reset(void __iomem *base)
static int ahci_host_init(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI struct udevice *dev = probe_ent->dev; struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); @@ -198,7 +198,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(cap_save, mmio + HOST_CAP); writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
-#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI if (pplat->vendor == PCI_VENDOR_ID_INTEL) { u16 tmp16; @@ -327,6 +327,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL); tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); +#if !defined(CONFIG_DM_SATA) #ifndef CONFIG_SCSI_AHCI_PLAT # ifdef CONFIG_DM_PCI dm_pci_read_config16(dev, PCI_COMMAND, &tmp16); @@ -338,14 +339,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) pci_write_config_word(pdev, PCI_COMMAND, tmp16); # endif #endif +#endif return 0; }
static void ahci_print_info(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev = probe_ent->dev; # else pci_dev_t pdev = probe_ent->dev; @@ -372,7 +374,7 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) else speed_s = "?";
-#ifdef CONFIG_SCSI_AHCI_PLAT +#if defined(CONFIG_SCSI_AHCI_PLAT) || defined(CONFIG_DM_SATA) scc_s = "SATA"; #else # ifdef CONFIG_DM_PCI @@ -424,13 +426,15 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) static int ahci_init_one(struct udevice *dev) # else static int ahci_init_one(pci_dev_t dev) # endif { +#if defined(CONFIG_DM_PCI) u16 vendor; +#endif int rc;
probe_ent = malloc(sizeof(struct ahci_probe_ent)); @@ -450,6 +454,7 @@ static int ahci_init_one(pci_dev_t dev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
+#if !defined(CONFIG_DM_SATA) #ifdef CONFIG_DM_PCI probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM); @@ -473,6 +478,10 @@ static int ahci_init_one(pci_dev_t dev) if (vendor == 0x197b) pci_write_config_byte(dev, 0x41, 0xa1); #endif +#else + struct scsi_platdata *plat = dev_get_platdata(dev); + probe_ent->mmio_base = (void *)plat->base; +#endif
debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base); /* initialize adapter */ @@ -954,14 +963,17 @@ int scsi_exec(ccb *pccb)
}
- +#if defined(CONFIG_DM_SATA) +void scsi_low_level_init(int busdevfunc, struct udevice *dev) +#else void scsi_low_level_init(int busdevfunc) +#endif { int i; u32 linkmap;
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) struct udevice *dev; int ret;
@@ -969,6 +981,8 @@ void scsi_low_level_init(int busdevfunc) if (ret) return; ahci_init_one(dev); +# elif defined(CONFIG_DM_SATA) + ahci_init_one(dev); # else ahci_init_one(busdevfunc); # endif diff --git a/include/ahci.h b/include/ahci.h index a956c6ff5df7..e740273de804 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,7 @@ struct ahci_ioports { };
struct ahci_probe_ent { -#ifdef CONFIG_DM_PCI +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev; #else pci_dev_t dev; diff --git a/include/sata.h b/include/sata.h index b35359aa5a19..26c8de730399 100644 --- a/include/sata.h +++ b/include/sata.h @@ -2,6 +2,8 @@ #define __SATA_H__ #include <part.h>
+#if !defined(CONFIG_DM_SATA) + int init_sata(int dev); int reset_sata(int dev); int scan_sata(int dev); @@ -15,5 +17,6 @@ int __sata_stop(void); int sata_port_status(int dev, int port);
extern struct blk_desc sata_dev_desc[]; +#endif
#endif diff --git a/include/scsi.h b/include/scsi.h index 7e3759140b34..22d6bd0d02a7 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -166,14 +166,27 @@ typedef struct SCSI_cmd_block{ void scsi_print_error(ccb *pccb); int scsi_exec(ccb *pccb); void scsi_bus_reset(void); +#if !defined(CONFIG_DM_SATA) void scsi_low_level_init(int busdevfunc); - +#else +void scsi_low_level_init(int busdevfunc, struct udevice *dev); +#endif
/*************************************************************************** * functions residing inside cmd_scsi.c */ void scsi_init(void); +#if !defined(CONFIG_DM_SATA) void scsi_scan(int mode); +#else + +struct scsi_platdata { + unsigned long base; +}; + +void scsi_scan(int mode, struct udevice *bdev); +int scsi_bind(struct udevice *dev); +#endif
/** @return the number of scsi disks */ int scsi_get_disk_count(void);
This patch also includes ARM64 zynqmp changes: - Remove platform non DM initialization - Remove hardcoded sata base address
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
There are probably more things to test and to check but on my platform I can connect only one HDD. But IP itself have two ports which are not handled properly. I have tried to reuse as much infrastructure as is available. There need to be cleanup for SATA/SCSI/AHCI names.
There is also sata cmd and it is a question if make sense to keep it in the tree because it is subset of scsi commands.
scsi scan needs to be called first and maybe make sense to call it automatically as was done before.
Simon: Please check if I did it at least partially right.
TODO: CONFIG_DM_SATA should be moved to Kconfig
LOG:
ZynqMP> scsi scan SATA link 0 timeout. Target spinup took 0 ms. AHCI 0001.0301 32 slots 2 ports 6 Gbps 0x3 impl SATA mode flags: 64bit ncq pm clo only pmp fbss pio slum part ccc apst scanning bus for devices... Device 0: (1:0) Vendor: ATA Prod.: KINGSTON SVP200S Rev: 501A Type: Hard Disk Capacity: 57241.8 MB = 55.9 GB (117231408 x 512) Found 1 device(s). ZynqMP> ls sata 0 <DIR> 4096 . <DIR> 4096 .. <DIR> 4096 bin <DIR> 4096 boot <DIR> 4096 dev <DIR> 12288 etc <DIR> 4096 home <DIR> 4096 lib <DIR> 4096 lost+found <DIR> 4096 media <DIR> 4096 mnt <DIR> 4096 opt <DIR> 4096 proc <DIR> 4096 root <DIR> 4096 run
--- arch/arm/include/asm/arch-zynqmp/hardware.h | 2 -- board/xilinx/zynqmp/zynqmp.c | 11 ------- drivers/block/sata_ceva.c | 49 +++++++++++++++++++++++++++-- include/configs/xilinx_zynqmp.h | 7 +++-- 4 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/arch/arm/include/asm/arch-zynqmp/hardware.h b/arch/arm/include/asm/arch-zynqmp/hardware.h index 456c1b0902e5..3e311eed8765 100644 --- a/arch/arm/include/asm/arch-zynqmp/hardware.h +++ b/arch/arm/include/asm/arch-zynqmp/hardware.h @@ -18,8 +18,6 @@
#define ARASAN_NAND_BASEADDR 0xFF100000
-#define ZYNQMP_SATA_BASEADDR 0xFD0C0000 - #define ZYNQMP_USB0_XHCI_BASEADDR 0xFE200000 #define ZYNQMP_USB1_XHCI_BASEADDR 0xFE300000
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 566b5e8d2afa..f96d5804f56f 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -300,17 +300,6 @@ void reset_cpu(ulong addr) { }
-#ifdef CONFIG_SCSI_AHCI_PLAT -void scsi_init(void) -{ -#if defined(CONFIG_SATA_CEVA) - init_sata(0); -#endif - ahci_init((void __iomem *)ZYNQMP_SATA_BASEADDR); - scsi_scan(1); -} -#endif - int board_late_init(void) { u32 reg = 0; diff --git a/drivers/block/sata_ceva.c b/drivers/block/sata_ceva.c index dcc3b90b17f1..27b771d7f2a3 100644 --- a/drivers/block/sata_ceva.c +++ b/drivers/block/sata_ceva.c @@ -9,6 +9,7 @@ #include <ahci.h> #include <scsi.h> #include <asm/arch/hardware.h> +#include <dm.h>
#include <asm/io.h>
@@ -73,10 +74,9 @@ #define DRV_NAME "ahci-ceva" #define CEVA_FLAG_BROKEN_GEN2 1
-int init_sata(int dev) +int ceva_init_sata(ulong mmio) { ulong tmp; - ulong mmio = ZYNQMP_SATA_BASEADDR; int i;
/* @@ -111,3 +111,48 @@ int init_sata(int dev) } return 0; } + +static int sata_ceva_probe(struct udevice *dev) +{ + struct scsi_platdata *plat = dev_get_platdata(dev); + + ceva_init_sata(plat->base); + return 0; +} + +static int sata_ceva_bind(struct udevice *dev) +{ + int ret; + + ret = scsi_bind(dev); + if (ret) + return ret; + + return 0; +} + +static const struct udevice_id sata_ceva_ids[] = { + { .compatible = "ceva,ahci-1v84" }, + { } +}; + +static int sata_ceva_ofdata_to_platdata(struct udevice *dev) +{ + struct scsi_platdata *plat = dev_get_platdata(dev); + + plat->base = dev_get_addr(dev); + if (plat->base == FDT_ADDR_T_NONE) + return -EINVAL; + + return 0; +} + +U_BOOT_DRIVER(ceva_host_blk) = { + .name = "ceva_sata", + .id = UCLASS_AHCI, + .of_match = sata_ceva_ids, + .bind = sata_ceva_bind, + .probe = sata_ceva_probe, + .ofdata_to_platdata = sata_ceva_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct scsi_platdata), +}; diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index b9986bebe6e1..f7abb8e6c44c 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -189,15 +189,18 @@ # define CONFIG_SYS_EEPROM_SIZE (64 * 1024) #endif
-#ifdef CONFIG_SATA_CEVA + +#if defined(CONFIG_SATA_CEVA) && !defined(CONFIG_SPL_BUILD) +#define CONFIG_DM_SATA #define CONFIG_LIBATA #define CONFIG_SCSI_AHCI -#define CONFIG_SCSI_AHCI_PLAT #define CONFIG_SYS_SCSI_MAX_SCSI_ID 2 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * \ CONFIG_SYS_SCSI_MAX_LUN) #define CONFIG_SCSI +#else +#undef CONFIG_SATA_CEVA #endif
#define CONFIG_ARM_SMC
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@xilinx.com wrote:
This patch also includes ARM64 zynqmp changes:
- Remove platform non DM initialization
- Remove hardcoded sata base address
Signed-off-by: Michal Simek michal.simek@xilinx.com
There are probably more things to test and to check but on my platform I can connect only one HDD. But IP itself have two ports which are not handled properly. I have tried to reuse as much infrastructure as is available. There need to be cleanup for SATA/SCSI/AHCI names.
There is also sata cmd and it is a question if make sense to keep it in the tree because it is subset of scsi commands.
scsi scan needs to be called first and maybe make sense to call it automatically as was done before.
Simon: Please check if I did it at least partially right.
TODO: CONFIG_DM_SATA should be moved to Kconfig
LOG:
ZynqMP> scsi scan SATA link 0 timeout. Target spinup took 0 ms. AHCI 0001.0301 32 slots 2 ports 6 Gbps 0x3 impl SATA mode flags: 64bit ncq pm clo only pmp fbss pio slum part ccc apst scanning bus for devices... Device 0: (1:0) Vendor: ATA Prod.: KINGSTON SVP200S Rev: 501A Type: Hard Disk Capacity: 57241.8 MB = 55.9 GB (117231408 x 512) Found 1 device(s). ZynqMP> ls sata 0
<DIR> 4096 . <DIR> 4096 .. <DIR> 4096 bin <DIR> 4096 boot <DIR> 4096 dev <DIR> 12288 etc <DIR> 4096 home <DIR> 4096 lib <DIR> 4096 lost+found <DIR> 4096 media <DIR> 4096 mnt <DIR> 4096 opt <DIR> 4096 proc <DIR> 4096 root <DIR> 4096 run
arch/arm/include/asm/arch-zynqmp/hardware.h | 2 -- board/xilinx/zynqmp/zynqmp.c | 11 ------- drivers/block/sata_ceva.c | 49 +++++++++++++++++++++++++++-- include/configs/xilinx_zynqmp.h | 7 +++-- 4 files changed, 52 insertions(+), 17 deletions(-)
Looks good to me - this is how a driver should be organised.
Regards, Simon
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@xilinx.com wrote:
This patch also includes ARM64 zynqmp changes:
- Remove platform non DM initialization
- Remove hardcoded sata base address
Signed-off-by: Michal Simek michal.simek@xilinx.com
There are probably more things to test and to check but on my platform I can connect only one HDD. But IP itself have two ports which are not handled properly. I have tried to reuse as much infrastructure as is available. There need to be cleanup for SATA/SCSI/AHCI names.
There is also sata cmd and it is a question if make sense to keep it in the tree because it is subset of scsi commands.
scsi scan needs to be called first and maybe make sense to call it automatically as was done before.
Simon: Please check if I did it at least partially right.
TODO: CONFIG_DM_SATA should be moved to Kconfig
LOG:
ZynqMP> scsi scan SATA link 0 timeout. Target spinup took 0 ms. AHCI 0001.0301 32 slots 2 ports 6 Gbps 0x3 impl SATA mode flags: 64bit ncq pm clo only pmp fbss pio slum part ccc apst scanning bus for devices... Device 0: (1:0) Vendor: ATA Prod.: KINGSTON SVP200S Rev: 501A Type: Hard Disk Capacity: 57241.8 MB = 55.9 GB (117231408 x 512) Found 1 device(s). ZynqMP> ls sata 0
<DIR> 4096 . <DIR> 4096 .. <DIR> 4096 bin <DIR> 4096 boot <DIR> 4096 dev <DIR> 12288 etc <DIR> 4096 home <DIR> 4096 lib <DIR> 4096 lost+found <DIR> 4096 media <DIR> 4096 mnt <DIR> 4096 opt <DIR> 4096 proc <DIR> 4096 root <DIR> 4096 run
arch/arm/include/asm/arch-zynqmp/hardware.h | 2 -- board/xilinx/zynqmp/zynqmp.c | 11 ------- drivers/block/sata_ceva.c | 49 +++++++++++++++++++++++++++-- include/configs/xilinx_zynqmp.h | 7 +++-- 4 files changed, 52 insertions(+), 17 deletions(-)
Looks good to me - this is how a driver should be organised.
Great.
Thanks, Michal
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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().
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 <common.h> #include <command.h> +#include <dm.h> #include <scsi.h>
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, &bdev);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_LUN 1
Do we need these with driver model, or can we scan?
+#endif
/* almost the maximum amount of the scsi_ext command.. */ #define SCSI_MAX_READ_BLK 0xFFFF @@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;
Is this actually used?
#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -235,6 +242,7 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -462,13 +470,20 @@ void scsi_setup_test_unit_ready(ccb *pccb)
- (re)-scan the scsi bus and reports scsi device info
- to the user if mode = 1
*/ +#ifdef CONFIG_DM_SATA +void scsi_scan(int mode, struct udevice *bdev) +#else void scsi_scan(int mode) +#endif { unsigned char i, perq, modi, lun; lbaint_t capacity; unsigned long blksz; ccb *pccb = (ccb *)&tempccb;
+#if defined(CONFIG_DM_SATA)
struct blk_desc *scsi_dev_desc = dev_get_uclass_platdata(bdev);+#endif if (mode == 1) printf("scanning bus for devices...\n"); for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { diff --git a/drivers/block/ahci-uclass.c b/drivers/block/ahci-uclass.c index 7b8c32699f53..f25b1bd336ae 100644 --- a/drivers/block/ahci-uclass.c +++ b/drivers/block/ahci-uclass.c @@ -1,14 +1,52 @@ /*
- Copyright (c) 2015 Google, Inc
- Written by Simon Glass sjg@chromium.org
- Copyright (c) 2016 Xilinx, Inc
*/
- Written by Michal Simek
- SPDX-License-Identifier: GPL-2.0+
#include <common.h> #include <dm.h> +#include <scsi.h>
+#if defined(CONFIG_DM_SATA) +int scsi_bind(struct udevice *dev) +{
struct udevice *bdev;struct blk_desc *bdesc;int ret;/** FIXME - maybe loop over CONFIG_SYS_SCSI_MAX_SCSI_ID* here to support more ports*/ret = blk_create_devicef(dev, "scsi_blk", "blk", IF_TYPE_SCSI, -1, 512,0, &bdev);if (ret) {printf("Cannot create block device\n");return ret;}bdesc = dev_get_uclass_platdata(bdev);debug("device %p, block device %p, block description %p\n",dev, bdev, bdesc);return 0;+}
+static int scsi_post_probe(struct udevice *dev) +{
debug("%s: device %p\n", __func__, dev);scsi_low_level_init(0, dev);return 0;+} +#endif
UCLASS_DRIVER(ahci) = { .id = UCLASS_AHCI, .name = "ahci", +#if defined(CONFIG_DM_SATA)
.post_probe = scsi_post_probe,+#endif }; diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e3e783a74cfd..03a95179eaa4 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,7 @@ int ahci_reset(void __iomem *base)
static int ahci_host_init(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI struct udevice *dev = probe_ent->dev; struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); @@ -198,7 +198,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(cap_save, mmio + HOST_CAP); writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
-#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI if (pplat->vendor == PCI_VENDOR_ID_INTEL) { u16 tmp16; @@ -327,6 +327,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL); tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); +#if !defined(CONFIG_DM_SATA) #ifndef CONFIG_SCSI_AHCI_PLAT # ifdef CONFIG_DM_PCI dm_pci_read_config16(dev, PCI_COMMAND, &tmp16); @@ -338,14 +339,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) pci_write_config_word(pdev, PCI_COMMAND, tmp16); # endif #endif +#endif return 0; }
static void ahci_print_info(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev = probe_ent->dev; # else pci_dev_t pdev = probe_ent->dev; @@ -372,7 +374,7 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) else speed_s = "?";
-#ifdef CONFIG_SCSI_AHCI_PLAT +#if defined(CONFIG_SCSI_AHCI_PLAT) || defined(CONFIG_DM_SATA) scc_s = "SATA"; #else # ifdef CONFIG_DM_PCI @@ -424,13 +426,15 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) static int ahci_init_one(struct udevice *dev) # else static int ahci_init_one(pci_dev_t dev) # endif { +#if defined(CONFIG_DM_PCI) u16 vendor; +#endif int rc;
probe_ent = malloc(sizeof(struct ahci_probe_ent));@@ -450,6 +454,7 @@ static int ahci_init_one(pci_dev_t dev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
+#if !defined(CONFIG_DM_SATA) #ifdef CONFIG_DM_PCI probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM); @@ -473,6 +478,10 @@ static int ahci_init_one(pci_dev_t dev) if (vendor == 0x197b) pci_write_config_byte(dev, 0x41, 0xa1); #endif +#else
struct scsi_platdata *plat = dev_get_platdata(dev);probe_ent->mmio_base = (void *)plat->base;+#endif
debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base); /* initialize adapter */@@ -954,14 +963,17 @@ int scsi_exec(ccb *pccb)
}
+#if defined(CONFIG_DM_SATA) +void scsi_low_level_init(int busdevfunc, struct udevice *dev) +#else void scsi_low_level_init(int busdevfunc) +#endif { int i; u32 linkmap;
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) struct udevice *dev; int ret;
@@ -969,6 +981,8 @@ void scsi_low_level_init(int busdevfunc) if (ret) return; ahci_init_one(dev); +# elif defined(CONFIG_DM_SATA)
ahci_init_one(dev);# else ahci_init_one(busdevfunc); # endif diff --git a/include/ahci.h b/include/ahci.h index a956c6ff5df7..e740273de804 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,7 @@ struct ahci_ioports { };
struct ahci_probe_ent { -#ifdef CONFIG_DM_PCI +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev; #else pci_dev_t dev; diff --git a/include/sata.h b/include/sata.h index b35359aa5a19..26c8de730399 100644 --- a/include/sata.h +++ b/include/sata.h @@ -2,6 +2,8 @@ #define __SATA_H__ #include <part.h>
+#if !defined(CONFIG_DM_SATA)
int init_sata(int dev); int reset_sata(int dev); int scan_sata(int dev); @@ -15,5 +17,6 @@ int __sata_stop(void); int sata_port_status(int dev, int port);
extern struct blk_desc sata_dev_desc[]; +#endif
#endif diff --git a/include/scsi.h b/include/scsi.h index 7e3759140b34..22d6bd0d02a7 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -166,14 +166,27 @@ typedef struct SCSI_cmd_block{ void scsi_print_error(ccb *pccb); int scsi_exec(ccb *pccb); void scsi_bus_reset(void); +#if !defined(CONFIG_DM_SATA) void scsi_low_level_init(int busdevfunc);
What will happen to these functions?
+#else +void scsi_low_level_init(int busdevfunc, struct udevice *dev); +#endif
/***************************************************************************
- functions residing inside cmd_scsi.c
*/ void scsi_init(void); +#if !defined(CONFIG_DM_SATA) void scsi_scan(int mode); +#else
+struct scsi_platdata {
unsigned long base;+};
+void scsi_scan(int mode, struct udevice *bdev); +int scsi_bind(struct udevice *dev); +#endif
/** @return the number of scsi disks */ int scsi_get_disk_count(void); -- 1.9.1
Regards, Simon
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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.
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 <common.h> #include <command.h> +#include <dm.h> #include <scsi.h>
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, &bdev);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_LUN 1
Do we need these with driver model, or can we scan?
It is mostly because I didn't want to create another copy of the same functions. We are allocating all stuff on fly that's why we are working with one device at time.
+#endif
/* almost the maximum amount of the scsi_ext command.. */ #define SCSI_MAX_READ_BLK 0xFFFF @@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;Is this actually used?
yes a lot. It is pointer to device description. For non DM case it is statically described based on selected number of devices.
#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -235,6 +242,7 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -462,13 +470,20 @@ void scsi_setup_test_unit_ready(ccb *pccb)
- (re)-scan the scsi bus and reports scsi device info
- to the user if mode = 1
*/ +#ifdef CONFIG_DM_SATA +void scsi_scan(int mode, struct udevice *bdev) +#else void scsi_scan(int mode) +#endif { unsigned char i, perq, modi, lun; lbaint_t capacity; unsigned long blksz; ccb *pccb = (ccb *)&tempccb;
+#if defined(CONFIG_DM_SATA)
struct blk_desc *scsi_dev_desc = dev_get_uclass_platdata(bdev);+#endif if (mode == 1) printf("scanning bus for devices...\n"); for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { diff --git a/drivers/block/ahci-uclass.c b/drivers/block/ahci-uclass.c index 7b8c32699f53..f25b1bd336ae 100644 --- a/drivers/block/ahci-uclass.c +++ b/drivers/block/ahci-uclass.c @@ -1,14 +1,52 @@ /*
- Copyright (c) 2015 Google, Inc
- Written by Simon Glass sjg@chromium.org
- Copyright (c) 2016 Xilinx, Inc
*/
- Written by Michal Simek
- SPDX-License-Identifier: GPL-2.0+
#include <common.h> #include <dm.h> +#include <scsi.h>
+#if defined(CONFIG_DM_SATA) +int scsi_bind(struct udevice *dev) +{
struct udevice *bdev;struct blk_desc *bdesc;int ret;/** FIXME - maybe loop over CONFIG_SYS_SCSI_MAX_SCSI_ID* here to support more ports*/ret = blk_create_devicef(dev, "scsi_blk", "blk", IF_TYPE_SCSI, -1, 512,0, &bdev);if (ret) {printf("Cannot create block device\n");return ret;}bdesc = dev_get_uclass_platdata(bdev);debug("device %p, block device %p, block description %p\n",dev, bdev, bdesc);return 0;+}
+static int scsi_post_probe(struct udevice *dev) +{
debug("%s: device %p\n", __func__, dev);scsi_low_level_init(0, dev);return 0;+} +#endif
UCLASS_DRIVER(ahci) = { .id = UCLASS_AHCI, .name = "ahci", +#if defined(CONFIG_DM_SATA)
.post_probe = scsi_post_probe,+#endif }; diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e3e783a74cfd..03a95179eaa4 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,7 @@ int ahci_reset(void __iomem *base)
static int ahci_host_init(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI struct udevice *dev = probe_ent->dev; struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); @@ -198,7 +198,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(cap_save, mmio + HOST_CAP); writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
-#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI if (pplat->vendor == PCI_VENDOR_ID_INTEL) { u16 tmp16; @@ -327,6 +327,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL); tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); +#if !defined(CONFIG_DM_SATA) #ifndef CONFIG_SCSI_AHCI_PLAT # ifdef CONFIG_DM_PCI dm_pci_read_config16(dev, PCI_COMMAND, &tmp16); @@ -338,14 +339,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) pci_write_config_word(pdev, PCI_COMMAND, tmp16); # endif #endif +#endif return 0; }
static void ahci_print_info(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev = probe_ent->dev; # else pci_dev_t pdev = probe_ent->dev; @@ -372,7 +374,7 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) else speed_s = "?";
-#ifdef CONFIG_SCSI_AHCI_PLAT +#if defined(CONFIG_SCSI_AHCI_PLAT) || defined(CONFIG_DM_SATA) scc_s = "SATA"; #else # ifdef CONFIG_DM_PCI @@ -424,13 +426,15 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) static int ahci_init_one(struct udevice *dev) # else static int ahci_init_one(pci_dev_t dev) # endif { +#if defined(CONFIG_DM_PCI) u16 vendor; +#endif int rc;
probe_ent = malloc(sizeof(struct ahci_probe_ent));@@ -450,6 +454,7 @@ static int ahci_init_one(pci_dev_t dev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
+#if !defined(CONFIG_DM_SATA) #ifdef CONFIG_DM_PCI probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM); @@ -473,6 +478,10 @@ static int ahci_init_one(pci_dev_t dev) if (vendor == 0x197b) pci_write_config_byte(dev, 0x41, 0xa1); #endif +#else
struct scsi_platdata *plat = dev_get_platdata(dev);probe_ent->mmio_base = (void *)plat->base;+#endif
debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base); /* initialize adapter */@@ -954,14 +963,17 @@ int scsi_exec(ccb *pccb)
}
+#if defined(CONFIG_DM_SATA) +void scsi_low_level_init(int busdevfunc, struct udevice *dev) +#else void scsi_low_level_init(int busdevfunc) +#endif { int i; u32 linkmap;
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) struct udevice *dev; int ret;
@@ -969,6 +981,8 @@ void scsi_low_level_init(int busdevfunc) if (ret) return; ahci_init_one(dev); +# elif defined(CONFIG_DM_SATA)
ahci_init_one(dev);# else ahci_init_one(busdevfunc); # endif diff --git a/include/ahci.h b/include/ahci.h index a956c6ff5df7..e740273de804 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,7 @@ struct ahci_ioports { };
struct ahci_probe_ent { -#ifdef CONFIG_DM_PCI +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev; #else pci_dev_t dev; diff --git a/include/sata.h b/include/sata.h index b35359aa5a19..26c8de730399 100644 --- a/include/sata.h +++ b/include/sata.h @@ -2,6 +2,8 @@ #define __SATA_H__ #include <part.h>
+#if !defined(CONFIG_DM_SATA)
int init_sata(int dev); int reset_sata(int dev); int scan_sata(int dev); @@ -15,5 +17,6 @@ int __sata_stop(void); int sata_port_status(int dev, int port);
extern struct blk_desc sata_dev_desc[]; +#endif
#endif diff --git a/include/scsi.h b/include/scsi.h index 7e3759140b34..22d6bd0d02a7 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -166,14 +166,27 @@ typedef struct SCSI_cmd_block{ void scsi_print_error(ccb *pccb); int scsi_exec(ccb *pccb); void scsi_bus_reset(void); +#if !defined(CONFIG_DM_SATA) void scsi_low_level_init(int busdevfunc);
What will happen to these functions?
PCI is using dm_pci_bus_find_bdf() to find out udevice. I know it from post_probe function that's why I don't need to look for it. Not sure why PCI is using this function.
+#else +void scsi_low_level_init(int busdevfunc, struct udevice *dev); +#endif
/***************************************************************************
- functions residing inside cmd_scsi.c
*/ void scsi_init(void); +#if !defined(CONFIG_DM_SATA) void scsi_scan(int mode); +#else
+struct scsi_platdata {
unsigned long base;+};
+void scsi_scan(int mode, struct udevice *bdev); +int scsi_bind(struct udevice *dev); +#endif
/** @return the number of scsi disks */ int scsi_get_disk_count(void); -- 1.9.1
Regards, Simon
Thanks, Michal
Hi Michal,
On 26 September 2016 at 05:06, Michal Simek michal.simek@xilinx.com wrote:
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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.
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 <common.h> #include <command.h> +#include <dm.h> #include <scsi.h>
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, &bdev);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_LUN 1
Do we need these with driver model, or can we scan?
It is mostly because I didn't want to create another copy of the same functions. We are allocating all stuff on fly that's why we are working with one device at time.
I'd rather not use these options if they are not useful.
+#endif
/* almost the maximum amount of the scsi_ext command.. */ #define SCSI_MAX_READ_BLK 0xFFFF @@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;Is this actually used?
yes a lot. It is pointer to device description. For non DM case it is statically described based on selected number of devices.
But I can't see scsi_dev_desc in your function.
#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -235,6 +242,7 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -462,13 +470,20 @@ void scsi_setup_test_unit_ready(ccb *pccb)
- (re)-scan the scsi bus and reports scsi device info
- to the user if mode = 1
*/ +#ifdef CONFIG_DM_SATA +void scsi_scan(int mode, struct udevice *bdev) +#else void scsi_scan(int mode) +#endif { unsigned char i, perq, modi, lun; lbaint_t capacity; unsigned long blksz; ccb *pccb = (ccb *)&tempccb;
+#if defined(CONFIG_DM_SATA)
struct blk_desc *scsi_dev_desc = dev_get_uclass_platdata(bdev);+#endif if (mode == 1) printf("scanning bus for devices...\n"); for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { diff --git a/drivers/block/ahci-uclass.c b/drivers/block/ahci-uclass.c index 7b8c32699f53..f25b1bd336ae 100644 --- a/drivers/block/ahci-uclass.c +++ b/drivers/block/ahci-uclass.c @@ -1,14 +1,52 @@ /*
- Copyright (c) 2015 Google, Inc
- Written by Simon Glass sjg@chromium.org
- Copyright (c) 2016 Xilinx, Inc
*/
- Written by Michal Simek
- SPDX-License-Identifier: GPL-2.0+
#include <common.h> #include <dm.h> +#include <scsi.h>
+#if defined(CONFIG_DM_SATA) +int scsi_bind(struct udevice *dev) +{
struct udevice *bdev;struct blk_desc *bdesc;int ret;/** FIXME - maybe loop over CONFIG_SYS_SCSI_MAX_SCSI_ID* here to support more ports*/ret = blk_create_devicef(dev, "scsi_blk", "blk", IF_TYPE_SCSI, -1, 512,0, &bdev);if (ret) {printf("Cannot create block device\n");return ret;}bdesc = dev_get_uclass_platdata(bdev);debug("device %p, block device %p, block description %p\n",dev, bdev, bdesc);return 0;+}
+static int scsi_post_probe(struct udevice *dev) +{
debug("%s: device %p\n", __func__, dev);scsi_low_level_init(0, dev);return 0;+} +#endif
UCLASS_DRIVER(ahci) = { .id = UCLASS_AHCI, .name = "ahci", +#if defined(CONFIG_DM_SATA)
.post_probe = scsi_post_probe,+#endif }; diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e3e783a74cfd..03a95179eaa4 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,7 @@ int ahci_reset(void __iomem *base)
static int ahci_host_init(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI struct udevice *dev = probe_ent->dev; struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); @@ -198,7 +198,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(cap_save, mmio + HOST_CAP); writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
-#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI if (pplat->vendor == PCI_VENDOR_ID_INTEL) { u16 tmp16; @@ -327,6 +327,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL); tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); +#if !defined(CONFIG_DM_SATA) #ifndef CONFIG_SCSI_AHCI_PLAT # ifdef CONFIG_DM_PCI dm_pci_read_config16(dev, PCI_COMMAND, &tmp16); @@ -338,14 +339,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) pci_write_config_word(pdev, PCI_COMMAND, tmp16); # endif #endif +#endif return 0; }
static void ahci_print_info(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev = probe_ent->dev; # else pci_dev_t pdev = probe_ent->dev; @@ -372,7 +374,7 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) else speed_s = "?";
-#ifdef CONFIG_SCSI_AHCI_PLAT +#if defined(CONFIG_SCSI_AHCI_PLAT) || defined(CONFIG_DM_SATA) scc_s = "SATA"; #else # ifdef CONFIG_DM_PCI @@ -424,13 +426,15 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) static int ahci_init_one(struct udevice *dev) # else static int ahci_init_one(pci_dev_t dev) # endif { +#if defined(CONFIG_DM_PCI) u16 vendor; +#endif int rc;
probe_ent = malloc(sizeof(struct ahci_probe_ent));@@ -450,6 +454,7 @@ static int ahci_init_one(pci_dev_t dev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
+#if !defined(CONFIG_DM_SATA) #ifdef CONFIG_DM_PCI probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM); @@ -473,6 +478,10 @@ static int ahci_init_one(pci_dev_t dev) if (vendor == 0x197b) pci_write_config_byte(dev, 0x41, 0xa1); #endif +#else
struct scsi_platdata *plat = dev_get_platdata(dev);probe_ent->mmio_base = (void *)plat->base;+#endif
debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base); /* initialize adapter */@@ -954,14 +963,17 @@ int scsi_exec(ccb *pccb)
}
+#if defined(CONFIG_DM_SATA) +void scsi_low_level_init(int busdevfunc, struct udevice *dev) +#else void scsi_low_level_init(int busdevfunc) +#endif { int i; u32 linkmap;
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) struct udevice *dev; int ret;
@@ -969,6 +981,8 @@ void scsi_low_level_init(int busdevfunc) if (ret) return; ahci_init_one(dev); +# elif defined(CONFIG_DM_SATA)
ahci_init_one(dev);# else ahci_init_one(busdevfunc); # endif diff --git a/include/ahci.h b/include/ahci.h index a956c6ff5df7..e740273de804 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,7 @@ struct ahci_ioports { };
struct ahci_probe_ent { -#ifdef CONFIG_DM_PCI +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev; #else pci_dev_t dev; diff --git a/include/sata.h b/include/sata.h index b35359aa5a19..26c8de730399 100644 --- a/include/sata.h +++ b/include/sata.h @@ -2,6 +2,8 @@ #define __SATA_H__ #include <part.h>
+#if !defined(CONFIG_DM_SATA)
int init_sata(int dev); int reset_sata(int dev); int scan_sata(int dev); @@ -15,5 +17,6 @@ int __sata_stop(void); int sata_port_status(int dev, int port);
extern struct blk_desc sata_dev_desc[]; +#endif
#endif diff --git a/include/scsi.h b/include/scsi.h index 7e3759140b34..22d6bd0d02a7 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -166,14 +166,27 @@ typedef struct SCSI_cmd_block{ void scsi_print_error(ccb *pccb); int scsi_exec(ccb *pccb); void scsi_bus_reset(void); +#if !defined(CONFIG_DM_SATA) void scsi_low_level_init(int busdevfunc);
What will happen to these functions?
PCI is using dm_pci_bus_find_bdf() to find out udevice. I know it from post_probe function that's why I don't need to look for it. Not sure why PCI is using this function.
I think you might need to do DM_SCSI first to separate the SCSI code from the SCSI helper functions (the ones that deal with SCSI messages). Then DM_SATA after that?
+#else +void scsi_low_level_init(int busdevfunc, struct udevice *dev); +#endif
/***************************************************************************
- functions residing inside cmd_scsi.c
*/ void scsi_init(void); +#if !defined(CONFIG_DM_SATA) void scsi_scan(int mode); +#else
+struct scsi_platdata {
unsigned long base;+};
+void scsi_scan(int mode, struct udevice *bdev); +int scsi_bind(struct udevice *dev); +#endif
/** @return the number of scsi disks */ int scsi_get_disk_count(void); -- 1.9.1
Regards, Simon
Thanks, Michal
Regards, Simon
Hi Simon,
On 27.9.2016 02:35, Simon Glass wrote:
Hi Michal,
On 26 September 2016 at 05:06, Michal Simek michal.simek@xilinx.com wrote:
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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.
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 <common.h> #include <command.h> +#include <dm.h> #include <scsi.h>
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, &bdev);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_LUN 1
Do we need these with driver model, or can we scan?
It is mostly because I didn't want to create another copy of the same functions. We are allocating all stuff on fly that's why we are working with one device at time.
I'd rather not use these options if they are not useful.
Some new functions need to be created to extract code and don't copy it everywhere.
+#endif
/* almost the maximum amount of the scsi_ext command.. */ #define SCSI_MAX_READ_BLK 0xFFFF @@ -160,6 +166,7 @@ static ulong scsi_read(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;Is this actually used?
yes a lot. It is pointer to device description. For non DM case it is statically described based on selected number of devices.
But I can't see scsi_dev_desc in your function.
Not in this patch but in that functions it is there.
{ #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev); struct blk_desc *scsi_dev_desc = block_dev; #endif int device = block_dev->devnum; lbaint_t start, blks; uintptr_t buf_addr; unsigned short smallblks = 0; ccb *pccb = (ccb *)&tempccb; device &= 0xff;
/* Setup device */ pccb->target = scsi_dev_desc[device].target; pccb->lun = scsi_dev_desc[device].lun; buf_addr = (unsigned long)buffer; start = blknr;
#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -235,6 +242,7 @@ static ulong scsi_write(struct blk_desc *block_dev, lbaint_t blknr, { #ifdef CONFIG_BLK struct blk_desc *block_dev = dev_get_uclass_platdata(dev);
struct blk_desc *scsi_dev_desc = block_dev;#endif int device = block_dev->devnum; lbaint_t start, blks; @@ -462,13 +470,20 @@ void scsi_setup_test_unit_ready(ccb *pccb)
- (re)-scan the scsi bus and reports scsi device info
- to the user if mode = 1
*/ +#ifdef CONFIG_DM_SATA +void scsi_scan(int mode, struct udevice *bdev) +#else void scsi_scan(int mode) +#endif { unsigned char i, perq, modi, lun; lbaint_t capacity; unsigned long blksz; ccb *pccb = (ccb *)&tempccb;
+#if defined(CONFIG_DM_SATA)
struct blk_desc *scsi_dev_desc = dev_get_uclass_platdata(bdev);+#endif if (mode == 1) printf("scanning bus for devices...\n"); for (i = 0; i < CONFIG_SYS_SCSI_MAX_DEVICE; i++) { diff --git a/drivers/block/ahci-uclass.c b/drivers/block/ahci-uclass.c index 7b8c32699f53..f25b1bd336ae 100644 --- a/drivers/block/ahci-uclass.c +++ b/drivers/block/ahci-uclass.c @@ -1,14 +1,52 @@ /*
- Copyright (c) 2015 Google, Inc
- Written by Simon Glass sjg@chromium.org
- Copyright (c) 2016 Xilinx, Inc
*/
- Written by Michal Simek
- SPDX-License-Identifier: GPL-2.0+
#include <common.h> #include <dm.h> +#include <scsi.h>
+#if defined(CONFIG_DM_SATA) +int scsi_bind(struct udevice *dev) +{
struct udevice *bdev;struct blk_desc *bdesc;int ret;/** FIXME - maybe loop over CONFIG_SYS_SCSI_MAX_SCSI_ID* here to support more ports*/ret = blk_create_devicef(dev, "scsi_blk", "blk", IF_TYPE_SCSI, -1, 512,0, &bdev);if (ret) {printf("Cannot create block device\n");return ret;}bdesc = dev_get_uclass_platdata(bdev);debug("device %p, block device %p, block description %p\n",dev, bdev, bdesc);return 0;+}
+static int scsi_post_probe(struct udevice *dev) +{
debug("%s: device %p\n", __func__, dev);scsi_low_level_init(0, dev);return 0;+} +#endif
UCLASS_DRIVER(ahci) = { .id = UCLASS_AHCI, .name = "ahci", +#if defined(CONFIG_DM_SATA)
.post_probe = scsi_post_probe,+#endif }; diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e3e783a74cfd..03a95179eaa4 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -168,7 +168,7 @@ int ahci_reset(void __iomem *base)
static int ahci_host_init(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI struct udevice *dev = probe_ent->dev; struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); @@ -198,7 +198,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(cap_save, mmio + HOST_CAP); writel_with_flush(0xf, mmio + HOST_PORTS_IMPL);
-#ifndef CONFIG_SCSI_AHCI_PLAT +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) # ifdef CONFIG_DM_PCI if (pplat->vendor == PCI_VENDOR_ID_INTEL) { u16 tmp16; @@ -327,6 +327,7 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) writel(tmp | HOST_IRQ_EN, mmio + HOST_CTL); tmp = readl(mmio + HOST_CTL); debug("HOST_CTL 0x%x\n", tmp); +#if !defined(CONFIG_DM_SATA) #ifndef CONFIG_SCSI_AHCI_PLAT # ifdef CONFIG_DM_PCI dm_pci_read_config16(dev, PCI_COMMAND, &tmp16); @@ -338,14 +339,15 @@ static int ahci_host_init(struct ahci_probe_ent *probe_ent) pci_write_config_word(pdev, PCI_COMMAND, tmp16); # endif #endif +#endif return 0; }
static void ahci_print_info(struct ahci_probe_ent *probe_ent) { -#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SATA) +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev = probe_ent->dev; # else pci_dev_t pdev = probe_ent->dev; @@ -372,7 +374,7 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) else speed_s = "?";
-#ifdef CONFIG_SCSI_AHCI_PLAT +#if defined(CONFIG_SCSI_AHCI_PLAT) || defined(CONFIG_DM_SATA) scc_s = "SATA"; #else # ifdef CONFIG_DM_PCI @@ -424,13 +426,15 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) }
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) static int ahci_init_one(struct udevice *dev) # else static int ahci_init_one(pci_dev_t dev) # endif { +#if defined(CONFIG_DM_PCI) u16 vendor; +#endif int rc;
probe_ent = malloc(sizeof(struct ahci_probe_ent));@@ -450,6 +454,7 @@ static int ahci_init_one(pci_dev_t dev) probe_ent->pio_mask = 0x1f; probe_ent->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
+#if !defined(CONFIG_DM_SATA) #ifdef CONFIG_DM_PCI probe_ent->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, PCI_REGION_MEM); @@ -473,6 +478,10 @@ static int ahci_init_one(pci_dev_t dev) if (vendor == 0x197b) pci_write_config_byte(dev, 0x41, 0xa1); #endif +#else
struct scsi_platdata *plat = dev_get_platdata(dev);probe_ent->mmio_base = (void *)plat->base;+#endif
debug("ahci mmio_base=0x%p\n", probe_ent->mmio_base); /* initialize adapter */@@ -954,14 +963,17 @@ int scsi_exec(ccb *pccb)
}
+#if defined(CONFIG_DM_SATA) +void scsi_low_level_init(int busdevfunc, struct udevice *dev) +#else void scsi_low_level_init(int busdevfunc) +#endif { int i; u32 linkmap;
#ifndef CONFIG_SCSI_AHCI_PLAT -# ifdef CONFIG_DM_PCI +# if defined(CONFIG_DM_PCI) struct udevice *dev; int ret;
@@ -969,6 +981,8 @@ void scsi_low_level_init(int busdevfunc) if (ret) return; ahci_init_one(dev); +# elif defined(CONFIG_DM_SATA)
ahci_init_one(dev);# else ahci_init_one(busdevfunc); # endif diff --git a/include/ahci.h b/include/ahci.h index a956c6ff5df7..e740273de804 100644 --- a/include/ahci.h +++ b/include/ahci.h @@ -145,7 +145,7 @@ struct ahci_ioports { };
struct ahci_probe_ent { -#ifdef CONFIG_DM_PCI +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SATA) struct udevice *dev; #else pci_dev_t dev; diff --git a/include/sata.h b/include/sata.h index b35359aa5a19..26c8de730399 100644 --- a/include/sata.h +++ b/include/sata.h @@ -2,6 +2,8 @@ #define __SATA_H__ #include <part.h>
+#if !defined(CONFIG_DM_SATA)
int init_sata(int dev); int reset_sata(int dev); int scan_sata(int dev); @@ -15,5 +17,6 @@ int __sata_stop(void); int sata_port_status(int dev, int port);
extern struct blk_desc sata_dev_desc[]; +#endif
#endif diff --git a/include/scsi.h b/include/scsi.h index 7e3759140b34..22d6bd0d02a7 100644 --- a/include/scsi.h +++ b/include/scsi.h @@ -166,14 +166,27 @@ typedef struct SCSI_cmd_block{ void scsi_print_error(ccb *pccb); int scsi_exec(ccb *pccb); void scsi_bus_reset(void); +#if !defined(CONFIG_DM_SATA) void scsi_low_level_init(int busdevfunc);
What will happen to these functions?
PCI is using dm_pci_bus_find_bdf() to find out udevice. I know it from post_probe function that's why I don't need to look for it. Not sure why PCI is using this function.
I think you might need to do DM_SCSI first to separate the SCSI code from the SCSI helper functions (the ones that deal with SCSI messages). Then DM_SATA after that?
Maybe will see when I have a time to look at this.
+#else +void scsi_low_level_init(int busdevfunc, struct udevice *dev); +#endif
/***************************************************************************
- functions residing inside cmd_scsi.c
*/ void scsi_init(void); +#if !defined(CONFIG_DM_SATA) void scsi_scan(int mode); +#else
+struct scsi_platdata {
unsigned long base;+};
+void scsi_scan(int mode, struct udevice *bdev); +int scsi_bind(struct udevice *dev); +#endif
/** @return the number of scsi disks */ int scsi_get_disk_count(void); -- 1.9.1
Regards, Simon
Thanks, Michal
Regards, Simon
Thanks, Michal
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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
Hi Michal,
On 21 November 2016 at 12:36, Michal Simek michal.simek@xilinx.com wrote:
On 24.9.2016 19:26, Simon Glass wrote:
Hi Michal,
On 8 September 2016 at 07:57, Michal Simek michal.simek@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@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.
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, &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
Regards, Simon
participants (2)
-
Michal Simek -
Simon Glass