[U-Boot] [PATCH 0/7] OMAP: Move SATA to use block driver model
This series adds support for SATA using the driver model on omap platforms. It is based on the work of Mugunthan V N mugunthanvnm@ti.com in Feb 2016
The first 3 patches are preparatory work. The 4th patch adds the actual driver. The 5th and 6th patches are fixes related to the scsi_scan logic when DM is used. The last patch enables the DM sata by default for the dra7 platforms.
This series depends on the series "mmc: omap_hsmmc: add support for CONFIG_BLK" posted a few days ago to add support for CONFIG_BLK in the omap_hsmmc driver.
Tested on DRA72 evm. buildman has been run on am33, dra7, am43, omap5 and am57
Jean-Jacques Hiblot (3): dm: scsi: ahci: fill max_lun and max_id members of scsi_platdata dm: scsi: fix scan defconfig: dra7xx_evm: enable CONFIG_BLK and disk driver model for SCSI
Mugunthan V N (4): arm: omap: sata: move enable sata clocks to enable_basic_clocks() arm: omap: sata: compile out sata init apis when CONFIG_DM_SCSI is defined arm: omap-common: sata: prepare driver for DM conversion drivers: block: dwc_ahci: Implement a driver for Synopsys DWC sata device
arch/arm/mach-omap2/omap5/hw_data.c | 12 ++++++ arch/arm/mach-omap2/sata.c | 38 ++++++------------ common/scsi.c | 35 +++++++++++++--- configs/dra7xx_evm_defconfig | 4 ++ configs/dra7xx_hs_evm_defconfig | 4 ++ drivers/block/Kconfig | 9 +++++ drivers/block/Makefile | 1 + drivers/block/ahci.c | 12 +++++- drivers/block/dwc_ahci.c | 79 +++++++++++++++++++++++++++++++++++++ include/sata.h | 2 + 10 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 drivers/block/dwc_ahci.c
From: Mugunthan V N mugunthanvnm@ti.com
All the clocks which has to be enabled has to be done in enable_basic_clocks(), so moving enable sata clock to common clocks enable function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- arch/arm/mach-omap2/omap5/hw_data.c | 12 ++++++++++++ arch/arm/mach-omap2/sata.c | 23 ----------------------- 2 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/arch/arm/mach-omap2/omap5/hw_data.c b/arch/arm/mach-omap2/omap5/hw_data.c index 5d956b5..a8a6b8a 100644 --- a/arch/arm/mach-omap2/omap5/hw_data.c +++ b/arch/arm/mach-omap2/omap5/hw_data.c @@ -361,6 +361,9 @@ void enable_basic_clocks(void) (*prcm)->cm_l4per_gpio6_clkctrl, (*prcm)->cm_l4per_gpio7_clkctrl, (*prcm)->cm_l4per_gpio8_clkctrl, +#ifdef CONFIG_SCSI_AHCI_PLAT + (*prcm)->cm_l3init_ocp2scp3_clkctrl, +#endif 0 };
@@ -379,6 +382,9 @@ void enable_basic_clocks(void) #ifdef CONFIG_TI_QSPI (*prcm)->cm_l4per_qspi_clkctrl, #endif +#ifdef CONFIG_SCSI_AHCI_PLAT + (*prcm)->cm_l3init_sata_clkctrl, +#endif 0 };
@@ -411,6 +417,12 @@ void enable_basic_clocks(void) setbits_le32((*prcm)->cm_l4per_qspi_clkctrl, (1<<24)); #endif
+#ifdef CONFIG_SCSI_AHCI_PLAT + /* Enable optional functional clock for SATA */ + setbits_le32((*prcm)->cm_l3init_sata_clkctrl, + SATA_CLKCTRL_OPTFCLKEN_MASK); +#endif + /* Enable SCRM OPT clocks for PER and CORE dpll */ setbits_le32((*prcm)->cm_wkupaon_scrm_clkctrl, OPTFCLKEN_SCRM_PER_MASK); diff --git a/arch/arm/mach-omap2/sata.c b/arch/arm/mach-omap2/sata.c index 2c2d1bc..0c82689 100644 --- a/arch/arm/mach-omap2/sata.c +++ b/arch/arm/mach-omap2/sata.c @@ -37,29 +37,6 @@ int init_sata(int dev) int ret; u32 val;
- u32 const clk_domains_sata[] = { - 0 - }; - - u32 const clk_modules_hw_auto_sata[] = { - (*prcm)->cm_l3init_ocp2scp3_clkctrl, - 0 - }; - - u32 const clk_modules_explicit_en_sata[] = { - (*prcm)->cm_l3init_sata_clkctrl, - 0 - }; - - do_enable_clocks(clk_domains_sata, - clk_modules_hw_auto_sata, - clk_modules_explicit_en_sata, - 0); - - /* Enable optional functional clock for SATA */ - setbits_le32((*prcm)->cm_l3init_sata_clkctrl, - SATA_CLKCTRL_OPTFCLKEN_MASK); - sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;
/* Power up the PHY */
On Fri, Mar 24, 2017 at 01:24:41PM +0100, Jean-Jacques Hiblot wrote:
From: Mugunthan V N mugunthanvnm@ti.com
All the clocks which has to be enabled has to be done in enable_basic_clocks(), so moving enable sata clock to common clocks enable function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
From: Mugunthan V N mugunthanvnm@ti.com
Compile out sata init/reset apis as this will be implemented in scsi-uclass driver to initialize sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- arch/arm/mach-omap2/sata.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-omap2/sata.c b/arch/arm/mach-omap2/sata.c index 0c82689..217f1c9 100644 --- a/arch/arm/mach-omap2/sata.c +++ b/arch/arm/mach-omap2/sata.c @@ -32,6 +32,7 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+#ifndef CONFIG_DM_SCSI int init_sata(int dev) { int ret; @@ -68,3 +69,4 @@ void scsi_bus_reset(void) ahci_reset((void __iomem *)DWC_AHSATA_BASE); ahci_init((void __iomem *)DWC_AHSATA_BASE); } +#endif
On Fri, Mar 24, 2017 at 01:24:42PM +0100, Jean-Jacques Hiblot wrote:
From: Mugunthan V N mugunthanvnm@ti.com
Compile out sata init/reset apis as this will be implemented in scsi-uclass driver to initialize sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
From: Mugunthan V N mugunthanvnm@ti.com
Prepare sata driver for DM conversion by abstracting sata phy init to seperate function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- arch/arm/mach-omap2/sata.c | 13 +++++++++---- include/sata.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/sata.c b/arch/arm/mach-omap2/sata.c index 217f1c9..dce432b 100644 --- a/arch/arm/mach-omap2/sata.c +++ b/arch/arm/mach-omap2/sata.c @@ -32,16 +32,21 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+int enable_sata_phy(void) +{ + sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata; + + /* Power up the PHY */ + return phy_pipe3_power_on(&sata_phy); +} + #ifndef CONFIG_DM_SCSI int init_sata(int dev) { int ret; u32 val;
- sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata; - - /* Power up the PHY */ - phy_pipe3_power_on(&sata_phy); + enable_sata_phy();
/* Enable SATA module, No Idle, No Standby */ val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO; diff --git a/include/sata.h b/include/sata.h index d18cc9a..583b72d 100644 --- a/include/sata.h +++ b/include/sata.h @@ -18,4 +18,6 @@ int sata_port_status(int dev, int port); extern struct blk_desc sata_dev_desc[]; #endif
+int enable_sata_phy(void); + #endif
Hi,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
From: Mugunthan V N mugunthanvnm@ti.com
Prepare sata driver for DM conversion by abstracting sata phy init to seperate function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/mach-omap2/sata.c | 13 +++++++++---- include/sata.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/sata.c b/arch/arm/mach-omap2/sata.c index 217f1c9..dce432b 100644 --- a/arch/arm/mach-omap2/sata.c +++ b/arch/arm/mach-omap2/sata.c @@ -32,16 +32,21 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+int enable_sata_phy(void)
We should not be calling board functions from the driver. Can you instead add a SATA PHY uclass / driver?
+{
sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;/* Power up the PHY */return phy_pipe3_power_on(&sata_phy);+}
#ifndef CONFIG_DM_SCSI int init_sata(int dev) { int ret; u32 val;
sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;/* Power up the PHY */phy_pipe3_power_on(&sata_phy);
enable_sata_phy(); /* Enable SATA module, No Idle, No Standby */ val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO;diff --git a/include/sata.h b/include/sata.h index d18cc9a..583b72d 100644 --- a/include/sata.h +++ b/include/sata.h @@ -18,4 +18,6 @@ int sata_port_status(int dev, int port); extern struct blk_desc sata_dev_desc[]; #endif
+int enable_sata_phy(void);
#endif
1.9.1
Regards, Simon
On 01/04/2017 06:21, Simon Glass wrote:
Hi,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
From: Mugunthan V N mugunthanvnm@ti.com
Prepare sata driver for DM conversion by abstracting sata phy init to seperate function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/mach-omap2/sata.c | 13 +++++++++---- include/sata.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/sata.c b/arch/arm/mach-omap2/sata.c index 217f1c9..dce432b 100644 --- a/arch/arm/mach-omap2/sata.c +++ b/arch/arm/mach-omap2/sata.c @@ -32,16 +32,21 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+int enable_sata_phy(void)
We should not be calling board functions from the driver. Can you instead add a SATA PHY uclass / driver?
Simon,
thanks for the review. I'll re-work the series in that direction. We already have all the required entries in the dts for this.
Jean-Jacques
+{
sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;/* Power up the PHY */return phy_pipe3_power_on(&sata_phy);+}
- #ifndef CONFIG_DM_SCSI int init_sata(int dev) { int ret; u32 val;
sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;/* Power up the PHY */phy_pipe3_power_on(&sata_phy);
enable_sata_phy(); /* Enable SATA module, No Idle, No Standby */ val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO;diff --git a/include/sata.h b/include/sata.h index d18cc9a..583b72d 100644 --- a/include/sata.h +++ b/include/sata.h @@ -18,4 +18,6 @@ int sata_port_status(int dev, int port); extern struct blk_desc sata_dev_desc[]; #endif
+int enable_sata_phy(void);
- #endif
-- 1.9.1
Regards, Simon
From: Mugunthan V N mugunthanvnm@ti.com
Implement a sata driver for Synopsys DWC sata device based on U-boot driver model.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- drivers/block/Kconfig | 9 ++++++ drivers/block/Makefile | 1 + drivers/block/dwc_ahci.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 drivers/block/dwc_ahci.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 88e66e2..0aecb6b 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -48,4 +48,13 @@ config SATA_CEVA ZynqMP. Support up to 2 external devices. Complient with SATA 3.1 and AHCI 1.3 specifications with hot-plug detect feature.
+ +config DWC_AHCI + bool "Enable Synopsys DWC AHCI driver support" + select SCSI_AHCI + depends on DM_SCSI + help + Enable this driver to support Sata devices through + Synopsys DWC AHCI module. + endmenu diff --git a/drivers/block/Makefile b/drivers/block/Makefile index a72feec..cffe498 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -11,6 +11,7 @@ ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+obj-$(CONFIG_DWC_AHCI) += dwc_ahci.o obj-$(CONFIG_AHCI) += ahci-uclass.o obj-$(CONFIG_DM_SCSI) += scsi-uclass.o obj-$(CONFIG_SCSI_AHCI) += ahci.o diff --git a/drivers/block/dwc_ahci.c b/drivers/block/dwc_ahci.c new file mode 100644 index 0000000..939e57d --- /dev/null +++ b/drivers/block/dwc_ahci.c @@ -0,0 +1,79 @@ +/* + * DWC SATA platform driver + * + * (C) Copyright 2016 + * Texas Instruments Incorporated, <www.ti.com> + * + * Author: Mugunthan V N mugunthanvnm@ti.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <ahci.h> +#include <scsi.h> +#include <sata.h> +#include <asm/arch/sata.h> +#include <asm/io.h> + +struct dwc_ahci_priv { + void *base; + void *wrapper_base; +}; + +static int dwc_ahci_ofdata_to_platdata(struct udevice *dev) +{ + struct dwc_ahci_priv *priv = dev_get_priv(dev); + fdt_addr_t addr; + + priv->base = map_physmem(dev_get_addr(dev), sizeof(void *), + MAP_NOCACHE); + + addr = dev_get_addr_index(dev, 1); + if (addr != FDT_ADDR_T_NONE) { + priv->wrapper_base = map_physmem(addr, sizeof(void *), + MAP_NOCACHE); + } else { + priv->wrapper_base = NULL; + } + + return 0; +} + +static int dwc_ahci_probe(struct udevice *dev) +{ + struct dwc_ahci_priv *priv = dev_get_priv(dev); + int ret; + + if (priv->wrapper_base) { + u32 val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO; + + /* Enable SATA module, No Idle, No Standby */ + writel(val, priv->wrapper_base + TI_SATA_SYSCONFIG); + } + + ret = enable_sata_phy(); + if (ret) { + error("Sata phy enable failed\n"); + return ret; + } + + return ahci_init(priv->base); +} + +static const struct udevice_id dwc_ahci_ids[] = { + { .compatible = "snps,dwc-ahci" }, + { } +}; + +U_BOOT_DRIVER(dwc_ahci) = { + .name = "dwc_ahci", + .id = UCLASS_SCSI, + .of_match = dwc_ahci_ids, + .ofdata_to_platdata = dwc_ahci_ofdata_to_platdata, + .probe = dwc_ahci_probe, + .priv_auto_alloc_size = sizeof(struct dwc_ahci_priv), + .platdata_auto_alloc_size = sizeof(struct scsi_platdata), + .flags = DM_FLAG_ALLOC_PRIV_DMA, +};
Those 2 values are required for proper operation of the DM_SCSI version of scsi_scan().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- drivers/block/ahci.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index 3fa14a7..3c5359f 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -971,7 +971,9 @@ void scsi_low_level_init(int busdevfunc) { int i; u32 linkmap; - +#ifdef CONFIG_DM_SCSI + struct scsi_platdata *plat = dev_get_platdata(dev); +#endif #ifndef CONFIG_SCSI_AHCI_PLAT # if defined(CONFIG_DM_PCI) struct udevice *dev; @@ -990,6 +992,13 @@ void scsi_low_level_init(int busdevfunc)
linkmap = probe_ent->link_port_map;
+#ifdef CONFIG_DM_SCSI + if (plat) { + plat->max_lun = 1; + plat->max_id = ffs(linkmap); + } +#endif + for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { if (((linkmap >> i) & 0x01)) { if (ahci_port_start((u8) i)) { @@ -1047,6 +1056,7 @@ err_out:
void __weak scsi_init(void) { + printf("AHCI\n"); }
#endif
Hi,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Those 2 values are required for proper operation of the DM_SCSI version of scsi_scan().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/block/ahci.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index 3fa14a7..3c5359f 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -971,7 +971,9 @@ void scsi_low_level_init(int busdevfunc) { int i; u32 linkmap;
+#ifdef CONFIG_DM_SCSI
struct scsi_platdata *plat = dev_get_platdata(dev);+#endif #ifndef CONFIG_SCSI_AHCI_PLAT # if defined(CONFIG_DM_PCI) struct udevice *dev; @@ -990,6 +992,13 @@ void scsi_low_level_init(int busdevfunc)
linkmap = probe_ent->link_port_map;+#ifdef CONFIG_DM_SCSI
We should not be touching or using scsi_low_level_init() with DM. There is a patch I just reviewed which seems to do a similar thing:
scsi: move base, max_lun and max_id to uclass plat data
if (plat) {plat->max_lun = 1;plat->max_id = ffs(linkmap);}+#endif
for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { if (((linkmap >> i) & 0x01)) { if (ahci_port_start((u8) i)) {@@ -1047,6 +1056,7 @@ err_out:
void __weak scsi_init(void) {
printf("AHCI\n");}
#endif
1.9.1
Regards, Simon
With DM_SCSI enabled, scsi scan suffers 2 problems: * blk_create_devicef is called with blkz = 0, leading to a divide-by-0 exception * new blk devices are created at each scan.
To fix this we start by removing all known SCSI devices at the beginning of the scan. Then a detection process is started for each possible device only to get the blksz and lba of the device (no call to part_init() yet). If a device is found, we can call blk_create_devicef() with the right parameters and continue the process.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- common/scsi.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index fb5b407..3385cc2 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,7 +480,7 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) * * Return: 0 on success, error value otherwise */ -static int scsi_detect_dev(int target, struct blk_desc *dev_desc) +static int scsi_detect_dev(int target, struct blk_desc *dev_desc, int init_part) { unsigned char perq, modi; lbaint_t capacity; @@ -539,7 +539,8 @@ static int scsi_detect_dev(int target, struct blk_desc *dev_desc) dev_desc->blksz = blksz; dev_desc->log2blksz = LOG2(dev_desc->blksz); dev_desc->type = perq; - part_init(&dev_desc[0]); + if (init_part) + part_init(&dev_desc[0]); removable: return 0; } @@ -565,6 +566,9 @@ int scsi_scan(int mode) if (ret) return ret;
+ /* remove all SCSI interfaces since we're going to (re)create them */ + blk_unbind_all(IF_TYPE_SCSI); + uclass_foreach_dev(dev, uc) { struct scsi_platdata *plat; /* scsi controller platdata */
@@ -580,9 +584,20 @@ int scsi_scan(int mode) for (lun = 0; lun < plat->max_lun; lun++) { struct udevice *bdev; /* block device */ /* block device description */ + struct blk_desc _bd; struct blk_desc *bdesc; char str[10];
+ scsi_init_dev_desc_priv(&_bd); + _bd.lun = lun; + ret = scsi_detect_dev(i, &_bd, 0); + if (ret) + /* + * no device detected? + * check the next lun. + */ + continue; + /* * Create only one block device and do detection * to make sure that there won't be a lot of @@ -590,17 +605,25 @@ int scsi_scan(int mode) */ snprintf(str, sizeof(str), "id%dlun%d", i, lun); ret = blk_create_devicef(dev, "scsi_blk", - str, IF_TYPE_SCSI, - -1, 0, 0, &bdev); + str, IF_TYPE_SCSI, + -1, + _bd.blksz, + _bd.blksz * _bd.lba, + &bdev); if (ret) { debug("Can't create device\n"); return ret; } bdesc = dev_get_uclass_platdata(bdev);
+ /* + * perform the detection once more but this + * time, let's initialize do the initialization + * of the partitions + */ scsi_init_dev_desc_priv(bdesc); bdesc->lun = lun; - ret = scsi_detect_dev(i, bdesc); + ret = scsi_detect_dev(i, bdesc, 1); if (ret) { device_unbind(bdev); continue; @@ -631,7 +654,7 @@ int scsi_scan(int mode) for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { scsi_dev_desc[scsi_max_devs].lun = lun; - ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs]); + ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs], 1); if (ret) continue;
Hi Jean-Jacques,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
With DM_SCSI enabled, scsi scan suffers 2 problems:
- blk_create_devicef is called with blkz = 0, leading to a divide-by-0 exception
- new blk devices are created at each scan.
To fix this we start by removing all known SCSI devices at the beginning of the scan. Then a detection process is started for each possible device only to get the blksz and lba of the device (no call to part_init() yet). If a device is found, we can call blk_create_devicef() with the right parameters and continue the process.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index fb5b407..3385cc2 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,7 +480,7 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum)
- Return: 0 on success, error value otherwise
*/ -static int scsi_detect_dev(int target, struct blk_desc *dev_desc) +static int scsi_detect_dev(int target, struct blk_desc *dev_desc, int init_part) { unsigned char perq, modi; lbaint_t capacity; @@ -539,7 +539,8 @@ static int scsi_detect_dev(int target, struct blk_desc *dev_desc) dev_desc->blksz = blksz; dev_desc->log2blksz = LOG2(dev_desc->blksz); dev_desc->type = perq;
part_init(&dev_desc[0]);
if (init_part)part_init(&dev_desc[0]);
Do you need this in here? The caller could just do it and avoid the extra parameter.
removable: return 0; } @@ -565,6 +566,9 @@ int scsi_scan(int mode) if (ret) return ret;
/* remove all SCSI interfaces since we're going to (re)create them */blk_unbind_all(IF_TYPE_SCSI);
This seems to already be done a few lines up...is it needed?
uclass_foreach_dev(dev, uc) { struct scsi_platdata *plat; /* scsi controller platdata */@@ -580,9 +584,20 @@ int scsi_scan(int mode) for (lun = 0; lun < plat->max_lun; lun++) { struct udevice *bdev; /* block device */ /* block device description */
struct blk_desc _bd; struct blk_desc *bdesc; char str[10];scsi_init_dev_desc_priv(&_bd);_bd.lun = lun;ret = scsi_detect_dev(i, &_bd, 0);if (ret)/** no device detected?* check the next lun.*/continue;/* * Create only one block device and do detection * to make sure that there won't be a lot of@@ -590,17 +605,25 @@ int scsi_scan(int mode) */ snprintf(str, sizeof(str), "id%dlun%d", i, lun); ret = blk_create_devicef(dev, "scsi_blk",
str, IF_TYPE_SCSI,-1, 0, 0, &bdev);
str, IF_TYPE_SCSI,-1,_bd.blksz,_bd.blksz * _bd.lba,&bdev);
But why put these in here when...
if (ret) { debug("Can't create device\n"); return ret; } bdesc = dev_get_uclass_platdata(bdev);
/** perform the detection once more but this* time, let's initialize do the initialization* of the partitions*/ scsi_init_dev_desc_priv(bdesc);
...they are overwritten here?
bdesc->lun = lun;
ret = scsi_detect_dev(i, bdesc);
ret = scsi_detect_dev(i, bdesc, 1); if (ret) { device_unbind(bdev); continue;@@ -631,7 +654,7 @@ int scsi_scan(int mode) for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { scsi_dev_desc[scsi_max_devs].lun = lun;
ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs]);
ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs], 1); if (ret) continue;
Call part_init() here?
-- 1.9.1
Regards, Simon
On 01/04/2017 06:22, Simon Glass wrote:
Hi Jean-Jacques,
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
With DM_SCSI enabled, scsi scan suffers 2 problems:
- blk_create_devicef is called with blkz = 0, leading to a divide-by-0 exception
- new blk devices are created at each scan.
To fix this we start by removing all known SCSI devices at the beginning of the scan. Then a detection process is started for each possible device only to get the blksz and lba of the device (no call to part_init() yet). If a device is found, we can call blk_create_devicef() with the right parameters and continue the process.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index fb5b407..3385cc2 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -480,7 +480,7 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum)
- Return: 0 on success, error value otherwise
*/ -static int scsi_detect_dev(int target, struct blk_desc *dev_desc) +static int scsi_detect_dev(int target, struct blk_desc *dev_desc, int init_part) { unsigned char perq, modi; lbaint_t capacity; @@ -539,7 +539,8 @@ static int scsi_detect_dev(int target, struct blk_desc *dev_desc) dev_desc->blksz = blksz; dev_desc->log2blksz = LOG2(dev_desc->blksz); dev_desc->type = perq;
part_init(&dev_desc[0]);
if (init_part)part_init(&dev_desc[0]);Do you need this in here? The caller could just do it and avoid the extra parameter.
I simply tried as much as possible to avoid code duplication. But agreed it's not a very elegant solution. I'll rework this. Thanks again for the comments
removable: return 0; } @@ -565,6 +566,9 @@ int scsi_scan(int mode) if (ret) return ret;
/* remove all SCSI interfaces since we're going to (re)create them */blk_unbind_all(IF_TYPE_SCSI);This seems to already be done a few lines up...is it needed?
No. It's just a rebase issue. I started the work on v2017.01 and didn't see the introduction of the unbind.
uclass_foreach_dev(dev, uc) { struct scsi_platdata *plat; /* scsi controller platdata */@@ -580,9 +584,20 @@ int scsi_scan(int mode) for (lun = 0; lun < plat->max_lun; lun++) { struct udevice *bdev; /* block device */ /* block device description */
struct blk_desc _bd; struct blk_desc *bdesc; char str[10];scsi_init_dev_desc_priv(&_bd);_bd.lun = lun;ret = scsi_detect_dev(i, &_bd, 0);if (ret)/** no device detected?* check the next lun.*/continue;/* * Create only one block device and do detection * to make sure that there won't be a lot of@@ -590,17 +605,25 @@ int scsi_scan(int mode) */ snprintf(str, sizeof(str), "id%dlun%d", i, lun); ret = blk_create_devicef(dev, "scsi_blk",
str, IF_TYPE_SCSI,-1, 0, 0, &bdev);
str, IF_TYPE_SCSI,-1,_bd.blksz,_bd.blksz * _bd.lba,&bdev);But why put these in here when...
if (ret) { debug("Can't create device\n"); return ret; } bdesc = dev_get_uclass_platdata(bdev);
/** perform the detection once more but this* time, let's initialize do the initialization* of the partitions*/ scsi_init_dev_desc_priv(bdesc);...they are overwritten here?
bdesc->lun = lun;
ret = scsi_detect_dev(i, bdesc);
ret = scsi_detect_dev(i, bdesc, 1); if (ret) { device_unbind(bdev); continue;@@ -631,7 +654,7 @@ int scsi_scan(int mode) for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { for (lun = 0; lun < CONFIG_SYS_SCSI_MAX_LUN; lun++) { scsi_dev_desc[scsi_max_devs].lun = lun;
ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs]);
ret = scsi_detect_dev(i, &scsi_dev_desc[scsi_max_devs], 1); if (ret) continue;Call part_init() here?
-- 1.9.1
Regards, Simon
Enable disk driver model for dra7xx_evm as dwc_ahci supports driver model. As a consequence we must also enable CONFIG_BLK and CONFIG_DM_USB.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- configs/dra7xx_evm_defconfig | 4 ++++ configs/dra7xx_hs_evm_defconfig | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig index 42f87b3..1d45622 100644 --- a/configs/dra7xx_evm_defconfig +++ b/configs/dra7xx_evm_defconfig @@ -89,6 +89,7 @@ CONFIG_TI_QSPI=y CONFIG_TIMER=y CONFIG_OMAP_TIMER=y CONFIG_USB=y +CONFIG_DM_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_DWC3=y @@ -101,3 +102,6 @@ CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_G_DNL_MANUFACTURER="Texas Instruments" CONFIG_G_DNL_VENDOR_NUM=0x0451 CONFIG_G_DNL_PRODUCT_NUM=0xd022 +CONFIG_BLK=y +CONFIG_DM_SCSI=y +CONFIG_DWC_AHCI=y diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig index f3a9c68..e31ad28 100644 --- a/configs/dra7xx_hs_evm_defconfig +++ b/configs/dra7xx_hs_evm_defconfig @@ -93,6 +93,7 @@ CONFIG_TI_QSPI=y CONFIG_TIMER=y CONFIG_OMAP_TIMER=y CONFIG_USB=y +CONFIG_DM_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y CONFIG_USB_DWC3=y @@ -105,3 +106,6 @@ CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_G_DNL_MANUFACTURER="Texas Instruments" CONFIG_G_DNL_VENDOR_NUM=0x0451 CONFIG_G_DNL_PRODUCT_NUM=0xd022 +CONFIG_BLK=y +CONFIG_DM_SCSI=y +CONFIG_DWC_AHCI=y
On 24 March 2017 at 06:24, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Enable disk driver model for dra7xx_evm as dwc_ahci supports driver model. As a consequence we must also enable CONFIG_BLK and CONFIG_DM_USB.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
configs/dra7xx_evm_defconfig | 4 ++++ configs/dra7xx_hs_evm_defconfig | 4 ++++ 2 files changed, 8 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
participants (3)
-
Jean-Jacques Hiblot -
Simon Glass -
Tom Rini