[U-Boot] [PATCH v3 1/2] dm: Add support for scsi/sata based devices
Simon Glass
sjg at chromium.org
Thu Dec 1 03:20:19 CET 2016
Hi Michal,
On 30 November 2016 at 13:48, Michal Simek <michal.simek at xilinx.com> wrote:
> All sata based drivers are bind and corresponding block
> device is created. Based on this find_scsi_device() is able
> to get back block device based on scsi_curr_dev pointer.
>
> intr_scsi() is commented now but it can be replaced by calling
> find_scsi_device() and scsi_scan().
>
> scsi_dev_desc[] is commented out but common/scsi.c heavily depends on
> it. That's why CONFIG_SYS_SCSI_MAX_DEVICE is hardcoded to 1 and symbol
> is reassigned to a block description allocated by uclass.
> There is only one block description by device now but it doesn't need to
> be correct when more devices are present.
>
> scsi_bind() ensures corresponding block device creation.
> uclass post_probe (scsi_post_probe()) is doing low level init.
>
> SCSI/SATA DM based drivers requires to have 64bit base address as
> the first entry in platform data structure to setup mmio_base.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
> Changes in v3:
> - Fix scsi_scan return path
> - Fix header location uclass-internal.h
> - Add scsi_max_devs under !DM_SCSI
> - Add new header device-internal because of device_probe()
> - Redesign block device creation algorithm
> - Use device_unbind in error path
> - Create block device with id and lun numbers (lun was there in v2)
> - Cleanup dev_num initialization in block device description
> with fixing parameters in blk_create_devicef
> - Create new Kconfig menu for SATA/SCSI drivers
> - Extend description for DM_SCSI
> - Fix Kconfig dependencies
> - Fix kernel doc format in scsi_platdata
> - Fix ahci_init_one - vendor variable
>
> Changes in v2:
> - Use CONFIG_DM_SCSI instead of mix of DM_SCSI and DM_SATA
> Ceva sata has never used sata commands that's why keep it in
> SCSI part only.
> - Separate scsi_scan() for DM_SCSI and do not change cmd/scsi.c
> - Extend platdata
>
> common/board_r.c | 4 +--
> common/scsi.c | 76 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/block/Kconfig | 12 +++++++
> drivers/block/Makefile | 1 +
> drivers/block/ahci.c | 30 +++++++++++++-----
> drivers/block/blk-uclass.c | 2 +-
> drivers/block/scsi-uclass.c | 29 +++++++++++++++++
> include/ahci.h | 2 +-
> include/dm/uclass-id.h | 1 +
> include/sata.h | 2 ++
> include/scsi.h | 20 +++++++++++-
> 11 files changed, 166 insertions(+), 13 deletions(-)
> create mode 100644 drivers/block/scsi-uclass.c
Reviewed-by: Simon Glass <sjg at chromium.org>
With a few nits below.
Also, as mentioned we do need a test (in subsequent patches). For an
example that is fairly close see:
test/dm/mmc.c
drivers/mmc/sandbox_mmc.c
There is a sandbox_scsi.c but it is empty because there is no
interface to the SCSI driver. From what I can tell your SCSI uclass
needs to have an operations struct (perhaps with just an 'exec'
method). The SCSI uclass can then implement the scsi_exec() call by
calling this operation. Does that sound right?
>
> diff --git a/common/board_r.c b/common/board_r.c
> index d959ad3c6f90..108cabbd023b 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_SCSI)
> 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_SCSI)
> INIT_FUNC_WATCHDOG_RESET
> initr_scsi,
> #endif
> diff --git a/common/scsi.c b/common/scsi.c
> index 04add624958f..e7efa5ae797c 100644
> --- a/common/scsi.c
> +++ b/common/scsi.c
> @@ -10,7 +10,10 @@
> #include <inttypes.h>
> #include <pci.h>
> #include <scsi.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
>
> +#if !defined(CONFIG_DM_SCSI)
> #ifdef CONFIG_SCSI_DEV_LIST
> #define SCSI_DEV_LIST CONFIG_SCSI_DEV_LIST
> #else
> @@ -31,6 +34,7 @@
> #endif
> #define SCSI_DEV_LIST {SCSI_VEND_ID, SCSI_DEV_ID}
> #endif
> +#endif
>
> #if defined(CONFIG_PCI) && !defined(CONFIG_SCSI_AHCI_PLAT)
> const struct pci_device_id scsi_device_list[] = { SCSI_DEV_LIST };
> @@ -39,11 +43,13 @@ static ccb tempccb; /* temporary scsi command buffer */
>
> static unsigned char tempbuff[512]; /* temporary data buffer */
>
> +#if !defined(CONFIG_DM_SCSI)
> static int scsi_max_devs; /* number of highest available scsi device */
>
> static int scsi_curr_dev; /* current device */
>
> static struct blk_desc scsi_dev_desc[CONFIG_SYS_SCSI_MAX_DEVICE];
> +#endif
>
> /* almost the maximum amount of the scsi_ext command.. */
> #define SCSI_MAX_READ_BLK 0xFFFF
> @@ -444,6 +450,7 @@ static void scsi_init_dev_desc_priv(struct blk_desc *dev_desc)
> #endif
> }
>
> +#if !defined(CONFIG_DM_SCSI)
> /**
> * scsi_init_dev_desc - initialize all SCSI specific blk_desc properties
> *
> @@ -460,6 +467,7 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum)
>
> scsi_init_dev_desc_priv(dev_desc);
> }
> +#endif
>
> /**
> * scsi_detect_dev - Detect scsi device
> @@ -540,6 +548,73 @@ removable:
> * (re)-scan the scsi bus and reports scsi device info
> * to the user if mode = 1
> */
> +#if defined(CONFIG_DM_SCSI)
> +int 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 ret;
> +
> + uclass_foreach_dev(dev, uc) {
> + struct scsi_platdata *plat; /* scsi controller platdata */
> +
> + /* probe SCSI controller driver */
> + ret = device_probe(dev);
> + if (ret)
> + return ret;
> +
> + /* Get controller platdata */
> + plat = dev_get_platdata(dev);
> +
> + for (i = 0; i < plat->max_id; i++) {
> + for (lun = 0; lun < plat->max_lun; lun++) {
> + struct udevice *bdev; /* block device */
> + /* block device description */
> + struct blk_desc *bdesc;
> + char str[10];
> +
> + /*
> + * Create only one block device and do detection
> + * to make sure that there won't be a lot of
> + * block devices created
> + */
> + snprintf(str, sizeof(str), "id%dlun%d", i, lun);
> + ret = blk_create_devicef(dev, "scsi_blk",
> + str, IF_TYPE_SCSI,
> + -1, 0, 0, &bdev);
> + if (ret) {
> + debug("Can't create device\n");
> + return ret;
> + }
> + bdesc = dev_get_uclass_platdata(bdev);
> +
> + scsi_init_dev_desc_priv(bdesc);
> + bdesc->lun = lun;
> + ret = scsi_detect_dev(i, bdesc);
> + if (ret) {
> + device_unbind(bdev);
> + continue;
> + }
> +
> + if (mode == 1) {
> + printf(" Device %d: ", 0);
> + dev_print(bdesc);
One question here - how come only mode 1 devices are printed?
> + } /* if mode */
> + } /* next LUN */
> + }
> + }
> +
> + return 0;
> +}
> +#else
> int scsi_scan(int mode)
> {
> unsigned char i, lun;
> @@ -576,6 +651,7 @@ int scsi_scan(int mode)
> #endif
> return 0;
> }
> +#endif
>
> #ifdef CONFIG_BLK
> static const struct blk_ops scsi_blk_ops = {
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index fe5aa07f921a..ecca1ef44636 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -19,6 +19,14 @@ config AHCI
> operations at present. The block device interface has not been converted
> to driver model.
>
> +config DM_SCSI
> + bool "Support SCSI controllers with driver model"
> + depends on BLK
> + help
> + This option enables the SCSI uclass which supports SCSI and SATA HDDs.
> + For every device configuration (IDs/LUNs) is created block device
For every device configuration (IDs/LUNs) a block device is created
> + with RAW read/write and filesystem support.
Also while you are are here can you please write out 'Small Computer
System Interface' in full somewhere so it is clear what it stands for?
> +
> config BLOCK_CACHE
> bool "Use block device cache"
> default n
> @@ -27,3 +35,7 @@ config BLOCK_CACHE
> This is most useful when accessing filesystems under U-Boot since
> it will prevent repeated reads from directory structures and other
> filesystem data structures.
> +
> +menu "SATA/SCSI device support"
> +
> +endmenu
> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index 436b79f98165..a72feecd5456 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -12,6 +12,7 @@ obj-y += blk_legacy.o
> endif
>
> obj-$(CONFIG_AHCI) += ahci-uclass.o
> +obj-$(CONFIG_DM_SCSI) += scsi-uclass.o
> obj-$(CONFIG_SCSI_AHCI) += ahci.o
> obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o
> obj-$(CONFIG_FSL_SATA) += fsl_sata.o
> diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c
> index 5139989d0b5f..d419855bd845 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_SCSI)
> # 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_SCSI)
> # 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_SCSI)
> #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_SCSI)
> +# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SCSI)
> 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_SCSI)
> 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_SCSI)
This diff is a bit confusing, but what I see in practice is this:
static void ahci_print_info(struct ahci_probe_ent *probe_ent)
{
#if !defined(CONFIG_SCSI_AHCI_PLAT) && !defined(CONFIG_DM_SCSI)
# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SCSI)
struct udevice *dev = probe_ent->dev;
# else
pci_dev_t pdev = probe_ent->dev;
# endif
u16 cc;
#endif
But if CONFIG_DM_SCSI is defined it will never reach the second #if. So I think:
# if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SCSI)
should just be left as:
-# ifdef CONFIG_DM_PCI
> static int ahci_init_one(struct udevice *dev)
> # else
> static int ahci_init_one(pci_dev_t dev)
> # endif
> {
> +#if !defined(CONFIG_DM_SCSI)
> 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_SCSI)
> #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_SCSI)
> +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_SCSI)
> + ahci_init_one(dev);
> # else
> ahci_init_one(busdevfunc);
> # endif
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 2e041c2b3dc7..38cb9388da6f 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -26,7 +26,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = {
>
> static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
> [IF_TYPE_IDE] = UCLASS_INVALID,
> - [IF_TYPE_SCSI] = UCLASS_INVALID,
> + [IF_TYPE_SCSI] = UCLASS_SCSI,
> [IF_TYPE_ATAPI] = UCLASS_INVALID,
> [IF_TYPE_USB] = UCLASS_MASS_STORAGE,
> [IF_TYPE_DOC] = UCLASS_INVALID,
> diff --git a/drivers/block/scsi-uclass.c b/drivers/block/scsi-uclass.c
> new file mode 100644
> index 000000000000..123b25c406a4
> --- /dev/null
> +++ b/drivers/block/scsi-uclass.c
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright (c) 2015 Google, Inc
> + * Written by Simon Glass <sjg at chromium.org>
> + * Copyright (c) 2016 Xilinx, Inc
> + * Written by Michal Simek
> + *
> + * Based on ahci-uclass.c
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <scsi.h>
> +
> +#if !defined(CONFIG_SPL_BUILD)
> +static int scsi_post_probe(struct udevice *dev)
> +{
> + debug("%s: device %p\n", __func__, dev);
> + scsi_low_level_init(0, dev);
> + return 0;
> +}
> +
> +UCLASS_DRIVER(scsi) = {
> + .id = UCLASS_SCSI,
> + .name = "scsi",
> + .post_probe = scsi_post_probe,
> +};
> +#endif
> diff --git a/include/ahci.h b/include/ahci.h
> index a956c6ff5df7..4876b41e9010 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_SCSI)
> struct udevice *dev;
> #else
> pci_dev_t dev;
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index eb78c4dac485..8c92d0b03088 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -66,6 +66,7 @@ enum uclass_id {
> UCLASS_REMOTEPROC, /* Remote Processor device */
> UCLASS_RESET, /* Reset controller device */
> UCLASS_RTC, /* Real time clock device */
> + UCLASS_SCSI, /* SCSI device */
> UCLASS_SERIAL, /* Serial UART */
> UCLASS_SPI, /* SPI bus */
> UCLASS_SPMI, /* System Power Management Interface bus */
> diff --git a/include/sata.h b/include/sata.h
> index b35359aa5a19..d18cc9aa8754 100644
> --- a/include/sata.h
> +++ b/include/sata.h
> @@ -2,6 +2,7 @@
> #define __SATA_H__
> #include <part.h>
>
> +#if !defined(CONFIG_DM_SCSI)
> int init_sata(int dev);
> int reset_sata(int dev);
> int scan_sata(int dev);
> @@ -15,5 +16,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 c8796785a465..190dacd0f2ec 100644
> --- a/include/scsi.h
> +++ b/include/scsi.h
> @@ -166,8 +166,11 @@ 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_SCSI)
> 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
> @@ -175,6 +178,21 @@ void scsi_low_level_init(int busdevfunc);
> void scsi_init(void);
> int scsi_scan(int mode);
>
> +#if defined(CONFIG_DM_SCSI)
> +/**
> + * struct scsi_platdata - stores information about SCSI controller
> + *
> + * @base: Controller base address
> + * @max_lun: Maximum number of logical units
> + * @max_id: Maximum number of target ids
> + */
> +struct scsi_platdata {
> + unsigned long base;
> + unsigned long max_lun;
> + unsigned long max_id;
> +};
> +#endif
> +
> #define SCSI_IDENTIFY 0xC0 /* not used */
>
> /* Hardware errors */
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list