* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
@ 2016-09-08 13:57 Michal Simek
2016-09-08 13:57 ` [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM Michal Simek
2016-09-24 17:26 ` [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Simon Glass
0 siblings, 2 replies; 10+ messages in thread
From: Michal Simek @ 2016-09-08 13:57 UTC (permalink / raw)
To: u-boot
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM
2016-09-08 13:57 [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Michal Simek
@ 2016-09-08 13:57 ` Michal Simek
2016-09-24 17:26 ` Simon Glass
2016-09-24 17:26 ` [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Simon Glass
1 sibling, 1 reply; 10+ messages in thread
From: Michal Simek @ 2016-09-08 13:57 UTC (permalink / raw)
To: u-boot
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
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-09-08 13:57 [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Michal Simek
2016-09-08 13:57 ` [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM Michal Simek
@ 2016-09-24 17:26 ` Simon Glass
2016-09-26 11:06 ` Michal Simek
2016-11-21 19:36 ` Michal Simek
1 sibling, 2 replies; 10+ messages in thread
From: Simon Glass @ 2016-09-24 17:26 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM
2016-09-08 13:57 ` [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM Michal Simek
@ 2016-09-24 17:26 ` Simon Glass
2016-09-26 6:43 ` Michal Simek
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2016-09-24 17:26 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM
2016-09-24 17:26 ` Simon Glass
@ 2016-09-26 6:43 ` Michal Simek
0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2016-09-26 6:43 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-09-24 17:26 ` [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Simon Glass
@ 2016-09-26 11:06 ` Michal Simek
2016-09-27 0:35 ` Simon Glass
2016-11-21 19:36 ` Michal Simek
1 sibling, 1 reply; 10+ messages in thread
From: Michal Simek @ 2016-09-26 11:06 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-09-26 11:06 ` Michal Simek
@ 2016-09-27 0:35 ` Simon Glass
2016-09-27 5:29 ` Michal Simek
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2016-09-27 0:35 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-09-27 0:35 ` Simon Glass
@ 2016-09-27 5:29 ` Michal Simek
0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2016-09-27 5:29 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-09-24 17:26 ` [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Simon Glass
2016-09-26 11:06 ` Michal Simek
@ 2016-11-21 19:36 ` Michal Simek
2016-11-24 2:21 ` Simon Glass
1 sibling, 1 reply; 10+ messages in thread
From: Michal Simek @ 2016-11-21 19:36 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices
2016-11-21 19:36 ` Michal Simek
@ 2016-11-24 2:21 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2016-11-24 2:21 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-24 2:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-08 13:57 [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Michal Simek
2016-09-08 13:57 ` [U-Boot] [RFC PATCH 2/2] block: Move ceva driver to DM Michal Simek
2016-09-24 17:26 ` Simon Glass
2016-09-26 6:43 ` Michal Simek
2016-09-24 17:26 ` [U-Boot] [RFC PATCH 1/2] dm: Add support for scsi/sata based devices Simon Glass
2016-09-26 11:06 ` Michal Simek
2016-09-27 0:35 ` Simon Glass
2016-09-27 5:29 ` Michal Simek
2016-11-21 19:36 ` Michal Simek
2016-11-24 2:21 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.