From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Sat, 24 Sep 2016 11:26:30 -0600 Subject: [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices In-Reply-To: References: Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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, &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 > + * Copyright (c) 2016 Xilinx, Inc > + * Written by Michal Simek > * > * SPDX-License-Identifier: GPL-2.0+ > */ > > #include > #include > +#include > + > +#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 > > +#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