[U-Boot] [PATCH v2 00/10] 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 2 patches are preparatory work. The 3rd patch adds a new framework to handle PHYs in a generic manner. The idea is to move the phy initialization out of the board-specific code into a proper driver. The link between the phy device and the controller device is done in by device-tree as it's done in linux. The API to control a phy has been copied from linux, excpet that the functions are prefixed by 'generic_phy_' instead of just 'phy_' because phy_reset() is already used to handle the Ethernet phys. The 4th patch adds a phy driver for the pipe3 sata phy found in the omaps/am5x/dra7x SOCs. The 5th patch allows the device under the node ocp2scp@4a090000 to be probed. The 6th patch implements a driver for the SATA controller found in the omaps/am5x/dra7x SOCs. The 7th patch is cosmetic and changes the interface of scsi_detect_dev() The 8th patch moves the call to part_init() out of scsi_detect_dev(). This is a preparatory work path #9. The 9th patches fix a divide-by-0 error that happens in scsi_scan() when DM is used. The last patch enables the DM sata by default for the dra7 platforms.
changes since v1: * changed the way the 'old' sata code is compiled out when DM_SCSI is enabled. * added a new framework for PHY management. * added a new driver for the PIPE3 phy and use it in the dwc_ahci driver. * changed the interface of scsi_detect_dev() and moved the call part_init() out of it. * modified the fix to scsi_scan() in order to call scsi_detect_dev() only once. * the max_lun and max_id parameters are now taken from the device-tree. * Updated the defconfig changes to include the PHY driver and its dependencies.
Jean-Jacques Hiblot (8): arm: omap: sata: compile out board-level sata code when CONFIG_DM_SCSI is defined drivers: phy: add generic PHY framework drivers: phy: add PIPE3 phy driver dra7: dtsi: mark ocp2scp bus compatible with "simple-bus" scsi: make the LUN a parameter of scsi_detect_dev() scsi: move the partition initialization out of the scsi detection dm: scsi: fix divide-by-0 error in scsi_scan() defconfig: dra7xx_evm: enable CONFIG_BLK and disk driver model for SCSI
Mugunthan V N (2): arm: omap: sata: move enable sata clocks to enable_basic_clocks() drivers: block: dwc_ahci: Implement a driver for Synopsys DWC sata device
Makefile | 1 + arch/arm/dts/dra7.dtsi | 2 +- arch/arm/mach-omap2/Makefile | 2 + arch/arm/mach-omap2/omap5/hw_data.c | 12 ++ arch/arm/mach-omap2/sata.c | 23 --- common/scsi.c | 48 +++-- configs/dra7xx_evm_defconfig | 12 +- configs/dra7xx_hs_evm_defconfig | 11 +- drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/block/Kconfig | 10 + drivers/block/Makefile | 1 + drivers/block/dwc_ahci.c | 100 ++++++++++ drivers/phy/Kconfig | 34 ++++ drivers/phy/Makefile | 6 + drivers/phy/phy-uclass.c | 77 ++++++++ drivers/phy/ti-pipe3-phy.c | 368 ++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++ 19 files changed, 708 insertions(+), 41 deletions(-) create mode 100644 drivers/block/dwc_ahci.c create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 drivers/phy/ti-pipe3-phy.c create mode 100644 include/generic-phy.h

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 ---
the patch hasn't changed since v1
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 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com 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
the patch hasn't changed since v1
arch/arm/mach-omap2/omap5/hw_data.c | 12 ++++++++++++ arch/arm/mach-omap2/sata.c | 23 ----------------------- 2 files changed, 12 insertions(+), 23 deletions(-)
Applied to u-boot-dm, thanks!

When CONFIG_DM_SCSI is defined, the SATA initialization will be implemented in the scsi-uclass driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- Instead of compiling out the code inside the C file, let's just not compile the files at all.
arch/arm/mach-omap2/Makefile | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index e814eb0..aa3986d 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -29,9 +29,11 @@ obj-y += abb.o endif
ifneq ($(CONFIG_OMAP54XX),) +ifeq ($(CONFIG_DM_SCSI),) obj-y += pipe3-phy.o obj-$(CONFIG_SCSI_AHCI_PLAT) += sata.o endif +endif
ifeq ($(CONFIG_SYS_DCACHE_OFF),) obj-y += omap-cache.o

On Fri, Apr 07, 2017 at 01:42:01PM +0200, Jean-Jacques Hiblot wrote:
When CONFIG_DM_SCSI is defined, the SATA initialization will be implemented in the scsi-uclass driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

On 8 April 2017 at 19:13, Tom Rini trini@konsulko.com wrote:
On Fri, Apr 07, 2017 at 01:42:01PM +0200, Jean-Jacques Hiblot wrote:
When CONFIG_DM_SCSI is defined, the SATA initialization will be implemented in the scsi-uclass driver.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com
-- Tom
Applied to u-boot-dm, thanks!

The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- Makefile | 1 + drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/phy/Kconfig | 22 ++++++++++++++ drivers/phy/Makefile | 5 ++++ drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++++++++++++++++++++++ 8 files changed, 147 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 include/generic-phy.h
diff --git a/Makefile b/Makefile index 2638acf..06454ce 100644 --- a/Makefile +++ b/Makefile @@ -650,6 +650,7 @@ libs-y += fs/ libs-y += net/ libs-y += disk/ libs-y += drivers/ +libs-y += drivers/phy/ libs-y += drivers/dma/ libs-y += drivers/gpio/ libs-y += drivers/i2c/ diff --git a/drivers/Kconfig b/drivers/Kconfig index 0e5d97d..a90ceca 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -88,6 +88,8 @@ source "drivers/video/Kconfig"
source "drivers/watchdog/Kconfig"
+source "drivers/phy/Kconfig" + config PHYS_TO_BUS bool "Custom physical to bus address mapping" help diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..4656509 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ endif
ifdef CONFIG_TPL_BUILD diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..b6fed9e --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,22 @@ + +menu "PHY Subsystem" + +config GENERIC_PHY + bool "PHY Core" + depends on DM + help + Generic PHY support. + + This framework is designed to provide a generic interface for PHY + devices. + +config SPL_GENERIC_PHY + bool "PHY Core in SPL" + depends on DM + help + Generic PHY support in SPL. + + This framework is designed to provide a generic interface for PHY + devices. + +endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 0000000..ccd15ed --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o + +ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) +obj-y += marvell/ +endif diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c new file mode 100644 index 0000000..4d1584d --- /dev/null +++ b/drivers/phy/phy-uclass.c @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ + * Written by Jean-Jacques Hiblot jjhiblot@ti.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <common.h> +#include <dm.h> +#include <generic-phy.h> + +#define get_ops(dev) ((struct generic_phy_ops *)(dev)->driver->ops) + +#define generic_phy_to_dev(x) ((struct udevice *)(x)) +#define dev_to_generic_phy(x) ((struct generic_phy *)(x)) + +struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string) +{ + struct udevice *generic_phy_dev; + + int rc = uclass_get_device_by_phandle(UCLASS_PHY, dev, + string, &generic_phy_dev); + if (rc) { + error("unable to find generic_phy device %d\n", rc); + return ERR_PTR(rc); + } + return dev_to_generic_phy(generic_phy_dev); +} + +int generic_phy_init(struct generic_phy *generic_phy) +{ + struct udevice *dev = generic_phy_to_dev(generic_phy); + struct generic_phy_ops *ops = get_ops(dev); + + return (ops && ops->init) ? ops->init(dev) : 0; +} + +int generic_phy_reset(struct generic_phy *generic_phy) +{ + struct udevice *dev = generic_phy_to_dev(generic_phy); + struct generic_phy_ops *ops = get_ops(dev); + + return (ops && ops->reset) ? ops->reset(dev) : 0; +} + +int generic_phy_exit(struct generic_phy *generic_phy) +{ + struct udevice *dev = generic_phy_to_dev(generic_phy); + struct generic_phy_ops *ops = get_ops(dev); + + return (ops && ops->exit) ? ops->exit(dev) : 0; +} + +int generic_phy_power_on(struct generic_phy *generic_phy) +{ + struct udevice *dev = generic_phy_to_dev(generic_phy); + struct generic_phy_ops *ops = get_ops(dev); + + return (ops && ops->power_on) ? ops->power_on(dev) : 0; +} + +int generic_phy_power_off(struct generic_phy *generic_phy) +{ + struct udevice *dev = generic_phy_to_dev(generic_phy); + struct generic_phy_ops *ops = get_ops(dev); + + return (ops && ops->power_off) ? ops->power_off(dev) : 0; +} + + + +UCLASS_DRIVER(simple_generic_phy) = { + .id = UCLASS_PHY, + .name = "generic_phy", +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 8c92d0b..9d34a32 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -83,6 +83,7 @@ enum uclass_id { UCLASS_VIDEO, /* Video or LCD device */ UCLASS_VIDEO_BRIDGE, /* Video bridge, e.g. DisplayPort to LVDS */ UCLASS_VIDEO_CONSOLE, /* Text console driver for video device */ + UCLASS_PHY, /* generic PHY device */
UCLASS_COUNT, UCLASS_INVALID = -1, diff --git a/include/generic-phy.h b/include/generic-phy.h new file mode 100644 index 0000000..f02e9ce --- /dev/null +++ b/include/generic-phy.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ + * Written by Jean-Jacques Hiblot jjhiblot@ti.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __GENERIC_PHY_H +#define __GENERIC_PHY_H + +struct generic_phy; +/* + * struct generic_phy_ops - set of function pointers for phy operations + * @init: operation to be performed for initializing phy + * @exit: operation to be performed while exiting + * @power_on: powering on the phy + * @power_off: powering off the phy + */ +struct generic_phy_ops { + int (*init)(struct udevice *phy); + int (*exit)(struct udevice *phy); + int (*reset)(struct udevice *phy); + int (*power_on)(struct udevice *phy); + int (*power_off)(struct udevice *phy); +}; + + +int generic_phy_init(struct generic_phy *phy); +int generic_phy_reset(struct generic_phy *phy); +int generic_phy_exit(struct generic_phy *phy); +int generic_phy_power_on(struct generic_phy *phy); +int generic_phy_power_off(struct generic_phy *phy); + +struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string); + +#endif /*__GENERIC_PHY_H */

On Fri, Apr 07, 2017 at 01:42:02PM +0200, Jean-Jacques Hiblot wrote:
The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Makefile | 1 + drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/phy/Kconfig | 22 ++++++++++++++ drivers/phy/Makefile | 5 ++++ drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++++++++++++++++++++++ 8 files changed, 147 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 include/generic-phy.h
Can you please create a sandbox driver and a test?
diff --git a/Makefile b/Makefile index 2638acf..06454ce 100644 --- a/Makefile +++ b/Makefile @@ -650,6 +650,7 @@ libs-y += fs/ libs-y += net/ libs-y += disk/ libs-y += drivers/ +libs-y += drivers/phy/
Could this go in drivers/Makefile?
libs-y += drivers/dma/ libs-y += drivers/gpio/ libs-y += drivers/i2c/ diff --git a/drivers/Kconfig b/drivers/Kconfig index 0e5d97d..a90ceca 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -88,6 +88,8 @@ source "drivers/video/Kconfig"
source "drivers/watchdog/Kconfig"
+source "drivers/phy/Kconfig"
config PHYS_TO_BUS bool "Custom physical to bus address mapping" help diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..4656509 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ endif
ifdef CONFIG_TPL_BUILD diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..b6fed9e --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,22 @@
+menu "PHY Subsystem"
+config GENERIC_PHY
Just a question: do you think we need the word GENERIC in this config? I'm OK with it, but wonder if CONFIG_PHY would be enough?
bool "PHY Core"
depends on DM
help
Generic PHY support.
This framework is designed to provide a generic interface for PHY
devices.
Could you given a few examples of PHY devices and the types of operations you can perform on them.
+config SPL_GENERIC_PHY
bool "PHY Core in SPL"
depends on DM
help
Generic PHY support in SPL.
This framework is designed to provide a generic interface for PHY
devices.
+endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 0000000..ccd15ed --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) +obj-y += marvell/ +endif diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c new file mode 100644 index 0000000..4d1584d --- /dev/null +++ b/drivers/phy/phy-uclass.c @@ -0,0 +1,77 @@ +/*
- Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
- Written by Jean-Jacques Hiblot jjhiblot@ti.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
SPDX?
- */
+#include <common.h> +#include <dm.h> +#include <generic-phy.h>
+#define get_ops(dev) ((struct generic_phy_ops *)(dev)->driver->ops)
+#define generic_phy_to_dev(x) ((struct udevice *)(x)) +#define dev_to_generic_phy(x) ((struct generic_phy *)(x))
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string) +{
struct udevice *generic_phy_dev;
dev is a shorter name :-)
int rc = uclass_get_device_by_phandle(UCLASS_PHY, dev,
string, &generic_phy_dev);
if (rc) {
error("unable to find generic_phy device %d\n", rc);
return ERR_PTR(rc);
}
return dev_to_generic_phy(generic_phy_dev);
+}
+int generic_phy_init(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->init) ? ops->init(dev) : 0;
+}
+int generic_phy_reset(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->reset) ? ops->reset(dev) : 0;
+}
+int generic_phy_exit(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->exit) ? ops->exit(dev) : 0;
+}
+int generic_phy_power_on(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->power_on) ? ops->power_on(dev) : 0;
+}
+int generic_phy_power_off(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->power_off) ? ops->power_off(dev) : 0;
+}
Drop 2 extra blank ilnes
+UCLASS_DRIVER(simple_generic_phy) = {
remove the word 'simple' ?
.id = UCLASS_PHY,
.name = "generic_phy",
+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 8c92d0b..9d34a32 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -83,6 +83,7 @@ enum uclass_id { UCLASS_VIDEO, /* Video or LCD device */ UCLASS_VIDEO_BRIDGE, /* Video bridge, e.g. DisplayPort to LVDS */ UCLASS_VIDEO_CONSOLE, /* Text console driver for video device */
UCLASS_PHY, /* generic PHY device */ UCLASS_COUNT, UCLASS_INVALID = -1,
diff --git a/include/generic-phy.h b/include/generic-phy.h new file mode 100644 index 0000000..f02e9ce --- /dev/null +++ b/include/generic-phy.h @@ -0,0 +1,38 @@ +/*
- Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
- Written by Jean-Jacques Hiblot jjhiblot@ti.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#ifndef __GENERIC_PHY_H +#define __GENERIC_PHY_H
+struct generic_phy; +/*
- struct generic_phy_ops - set of function pointers for phy operations
- @init: operation to be performed for initializing phy
- @exit: operation to be performed while exiting
- @power_on: powering on the phy
- @power_off: powering off the phy
Need to mention that these are all optional (from what I can tell above).
- */
+struct generic_phy_ops {
int (*init)(struct udevice *phy);
int (*exit)(struct udevice *phy);
int (*reset)(struct udevice *phy);
int (*power_on)(struct udevice *phy);
int (*power_off)(struct udevice *phy);
+};
+int generic_phy_init(struct generic_phy *phy);
Why do these not use struct udevice?
+int generic_phy_reset(struct generic_phy *phy); +int generic_phy_exit(struct generic_phy *phy); +int generic_phy_power_on(struct generic_phy *phy); +int generic_phy_power_off(struct generic_phy *phy);
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string);
+#endif /*__GENERIC_PHY_H */
1.9.1
Regards, Simon

On 09/04/2017 21:27, Simon Glass wrote:
Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Makefile | 1 + drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/phy/Kconfig | 22 ++++++++++++++ drivers/phy/Makefile | 5 ++++ drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++++++++++++++++++++++ 8 files changed, 147 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 include/generic-phy.h
Can you please create a sandbox driver and a test?
Sure. I'll add something. It'll be pretty basic though
diff --git a/Makefile b/Makefile index 2638acf..06454ce 100644 --- a/Makefile +++ b/Makefile @@ -650,6 +650,7 @@ libs-y += fs/ libs-y += net/ libs-y += disk/ libs-y += drivers/ +libs-y += drivers/phy/
Could this go in drivers/Makefile?
OK
libs-y += drivers/dma/ libs-y += drivers/gpio/ libs-y += drivers/i2c/ diff --git a/drivers/Kconfig b/drivers/Kconfig index 0e5d97d..a90ceca 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -88,6 +88,8 @@ source "drivers/video/Kconfig"
source "drivers/watchdog/Kconfig"
+source "drivers/phy/Kconfig"
- config PHYS_TO_BUS bool "Custom physical to bus address mapping" help
diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..4656509 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ endif
ifdef CONFIG_TPL_BUILD diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..b6fed9e --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,22 @@
+menu "PHY Subsystem"
+config GENERIC_PHY
Just a question: do you think we need the word GENERIC in this config? I'm OK with it, but wonder if CONFIG_PHY would be enough?
GENERIC_PHY is the name of the config option in the kernel and the functions are also prefixed with generic_phy_. BTW the functions in linux are not prefixed with generic_phy_ but only phy_, but in the case of u-boot a lot of phy_xxx() functions already exist and are not necessarily static (like phy_reset() for ther ethernet phy).
bool "PHY Core"
depends on DM
help
Generic PHY support.
This framework is designed to provide a generic interface for PHY
devices.
Could you given a few examples of PHY devices and the types of operations you can perform on them.
OK
+config SPL_GENERIC_PHY
bool "PHY Core in SPL"
depends on DM
help
Generic PHY support in SPL.
This framework is designed to provide a generic interface for PHY
devices.
+endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile new file mode 100644 index 0000000..ccd15ed --- /dev/null +++ b/drivers/phy/Makefile @@ -0,0 +1,5 @@ +obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) +obj-y += marvell/ +endif diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c new file mode 100644 index 0000000..4d1584d --- /dev/null +++ b/drivers/phy/phy-uclass.c @@ -0,0 +1,77 @@ +/*
- Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
- Written by Jean-Jacques Hiblot jjhiblot@ti.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
SPDX?
OK
- */
+#include <common.h> +#include <dm.h> +#include <generic-phy.h>
+#define get_ops(dev) ((struct generic_phy_ops *)(dev)->driver->ops)
+#define generic_phy_to_dev(x) ((struct udevice *)(x)) +#define dev_to_generic_phy(x) ((struct generic_phy *)(x))
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string) +{
struct udevice *generic_phy_dev;
dev is a shorter name :-)
indeed
int rc = uclass_get_device_by_phandle(UCLASS_PHY, dev,
string, &generic_phy_dev);
if (rc) {
error("unable to find generic_phy device %d\n", rc);
return ERR_PTR(rc);
}
return dev_to_generic_phy(generic_phy_dev);
+}
+int generic_phy_init(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->init) ? ops->init(dev) : 0;
+}
+int generic_phy_reset(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->reset) ? ops->reset(dev) : 0;
+}
+int generic_phy_exit(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->exit) ? ops->exit(dev) : 0;
+}
+int generic_phy_power_on(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->power_on) ? ops->power_on(dev) : 0;
+}
+int generic_phy_power_off(struct generic_phy *generic_phy) +{
struct udevice *dev = generic_phy_to_dev(generic_phy);
struct generic_phy_ops *ops = get_ops(dev);
return (ops && ops->power_off) ? ops->power_off(dev) : 0;
+}
Drop 2 extra blank ilnes
+UCLASS_DRIVER(simple_generic_phy) = {
remove the word 'simple' ?
OK
.id = UCLASS_PHY,
.name = "generic_phy",
+}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 8c92d0b..9d34a32 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -83,6 +83,7 @@ enum uclass_id { UCLASS_VIDEO, /* Video or LCD device */ UCLASS_VIDEO_BRIDGE, /* Video bridge, e.g. DisplayPort to LVDS */ UCLASS_VIDEO_CONSOLE, /* Text console driver for video device */
UCLASS_PHY, /* generic PHY device */ UCLASS_COUNT, UCLASS_INVALID = -1,
diff --git a/include/generic-phy.h b/include/generic-phy.h new file mode 100644 index 0000000..f02e9ce --- /dev/null +++ b/include/generic-phy.h @@ -0,0 +1,38 @@ +/*
- Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
- Written by Jean-Jacques Hiblot jjhiblot@ti.com
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#ifndef __GENERIC_PHY_H +#define __GENERIC_PHY_H
+struct generic_phy; +/*
- struct generic_phy_ops - set of function pointers for phy operations
- @init: operation to be performed for initializing phy
- @exit: operation to be performed while exiting
- @power_on: powering on the phy
- @power_off: powering off the phy
Need to mention that these are all optional (from what I can tell above).
OK
- */
+struct generic_phy_ops {
int (*init)(struct udevice *phy);
int (*exit)(struct udevice *phy);
int (*reset)(struct udevice *phy);
int (*power_on)(struct udevice *phy);
int (*power_off)(struct udevice *phy);
+};
+int generic_phy_init(struct generic_phy *phy);
Why do these not use struct udevice?
It's quite easy for the PHY driver to get its internal data structure from the struct udevice*. I could also have passed struct generic_phy * but it adds another translation that I don't think is necessary.
+int generic_phy_reset(struct generic_phy *phy); +int generic_phy_exit(struct generic_phy *phy); +int generic_phy_power_on(struct generic_phy *phy); +int generic_phy_power_off(struct generic_phy *phy);
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string);
+#endif /*__GENERIC_PHY_H */
1.9.1
Regards, Simon

Hi Jean-Jacques,
On 13 April 2017 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
On 09/04/2017 21:27, Simon Glass wrote:
Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Makefile | 1 + drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/phy/Kconfig | 22 ++++++++++++++ drivers/phy/Makefile | 5 ++++ drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++++++++++++++++++++++ 8 files changed, 147 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 include/generic-phy.h
Can you please create a sandbox driver and a test?
Sure. I'll add something. It'll be pretty basic though
Basic is fine. It needs to create a device or two and call some methods.
diff --git a/Makefile b/Makefile index 2638acf..06454ce 100644 --- a/Makefile +++ b/Makefile @@ -650,6 +650,7 @@ libs-y += fs/ libs-y += net/ libs-y += disk/ libs-y += drivers/ +libs-y += drivers/phy/
Could this go in drivers/Makefile?
OK
libs-y += drivers/dma/ libs-y += drivers/gpio/ libs-y += drivers/i2c/ diff --git a/drivers/Kconfig b/drivers/Kconfig index 0e5d97d..a90ceca 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -88,6 +88,8 @@ source "drivers/video/Kconfig"
source "drivers/watchdog/Kconfig"
+source "drivers/phy/Kconfig"
- config PHYS_TO_BUS bool "Custom physical to bus address mapping" help
diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..4656509 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ endif
ifdef CONFIG_TPL_BUILD diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..b6fed9e --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,22 @@
+menu "PHY Subsystem"
+config GENERIC_PHY
Just a question: do you think we need the word GENERIC in this config? I'm OK with it, but wonder if CONFIG_PHY would be enough?
GENERIC_PHY is the name of the config option in the kernel and the functions are also prefixed with generic_phy_. BTW the functions in linux are not prefixed with generic_phy_ but only phy_, but in the case of u-boot a lot of phy_xxx() functions already exist and are not necessarily static (like phy_reset() for ther ethernet phy).
OK. [..]
- */
+struct generic_phy_ops {
int (*init)(struct udevice *phy);
int (*exit)(struct udevice *phy);
int (*reset)(struct udevice *phy);
int (*power_on)(struct udevice *phy);
int (*power_off)(struct udevice *phy);
+};
+int generic_phy_init(struct generic_phy *phy);
Why do these not use struct udevice?
It's quite easy for the PHY driver to get its internal data structure from the struct udevice*. I could also have passed struct generic_phy * but it adds another translation that I don't think is necessary.
I'd like to change that for consistency. A uclass API is supposed to take a struct udevice * rather than anything else. It is confusing if one uclass does this differently. The translation is cheap and some users will have a struct udevice * readily available.
+int generic_phy_reset(struct generic_phy *phy); +int generic_phy_exit(struct generic_phy *phy); +int generic_phy_power_on(struct generic_phy *phy); +int generic_phy_power_off(struct generic_phy *phy);
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string);
+#endif /*__GENERIC_PHY_H */
1.9.1
Regards, Simon
Regards, Simon

On 14/04/2017 12:36, Simon Glass wrote:
Hi Jean-Jacques,
On 13 April 2017 at 08:17, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
On 09/04/2017 21:27, Simon Glass wrote:
Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
The PHY framework provides a set of APIs to control a PHY. This API is derived from the linux version of the generic PHY framework. Currently the API supports init(), deinit(), power_on, power_off() and reset(). The framework provides a way to get a reference to a phy from the device-tree.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Makefile | 1 + drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/phy/Kconfig | 22 ++++++++++++++ drivers/phy/Makefile | 5 ++++ drivers/phy/phy-uclass.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/generic-phy.h | 38 ++++++++++++++++++++++++ 8 files changed, 147 insertions(+) create mode 100644 drivers/phy/Kconfig create mode 100644 drivers/phy/Makefile create mode 100644 drivers/phy/phy-uclass.c create mode 100644 include/generic-phy.h
Can you please create a sandbox driver and a test?
Sure. I'll add something. It'll be pretty basic though
Basic is fine. It needs to create a device or two and call some methods.
diff --git a/Makefile b/Makefile index 2638acf..06454ce 100644 --- a/Makefile +++ b/Makefile @@ -650,6 +650,7 @@ libs-y += fs/ libs-y += net/ libs-y += disk/ libs-y += drivers/ +libs-y += drivers/phy/
Could this go in drivers/Makefile?
OK
libs-y += drivers/dma/ libs-y += drivers/gpio/ libs-y += drivers/i2c/ diff --git a/drivers/Kconfig b/drivers/Kconfig index 0e5d97d..a90ceca 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -88,6 +88,8 @@ source "drivers/video/Kconfig"
source "drivers/watchdog/Kconfig"
+source "drivers/phy/Kconfig"
- config PHYS_TO_BUS bool "Custom physical to bus address mapping" help
diff --git a/drivers/Makefile b/drivers/Makefile index 5d8baa5..4656509 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_OMAP_USB_PHY) += usb/phy/ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/ obj-$(CONFIG_SPL_USB_HOST_SUPPORT) += block/ obj-$(CONFIG_SPL_MMC_SUPPORT) += block/ +obj-$(CONFIG_SPL_GENERIC_PHY) += phy/ endif
ifdef CONFIG_TPL_BUILD diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig new file mode 100644 index 0000000..b6fed9e --- /dev/null +++ b/drivers/phy/Kconfig @@ -0,0 +1,22 @@
+menu "PHY Subsystem"
+config GENERIC_PHY
Just a question: do you think we need the word GENERIC in this config? I'm OK with it, but wonder if CONFIG_PHY would be enough?
GENERIC_PHY is the name of the config option in the kernel and the functions are also prefixed with generic_phy_. BTW the functions in linux are not prefixed with generic_phy_ but only phy_, but in the case of u-boot a lot of phy_xxx() functions already exist and are not necessarily static (like phy_reset() for ther ethernet phy).
OK. [..]
- */
+struct generic_phy_ops {
int (*init)(struct udevice *phy);
int (*exit)(struct udevice *phy);
int (*reset)(struct udevice *phy);
int (*power_on)(struct udevice *phy);
int (*power_off)(struct udevice *phy);
+};
+int generic_phy_init(struct generic_phy *phy);
Why do these not use struct udevice?
It's quite easy for the PHY driver to get its internal data structure from the struct udevice*. I could also have passed struct generic_phy * but it adds another translation that I don't think is necessary.
I'd like to change that for consistency. A uclass API is supposed to take a struct udevice * rather than anything else. It is confusing if one uclass does this differently. The translation is cheap and some users will have a struct udevice * readily available..
Yes I eventually figured this out while working on the unit tests v3. This has been changed.
Jean-Jacques
+int generic_phy_reset(struct generic_phy *phy); +int generic_phy_exit(struct generic_phy *phy); +int generic_phy_power_on(struct generic_phy *phy); +int generic_phy_power_off(struct generic_phy *phy);
+struct generic_phy *dm_generic_phy_get(struct udevice *dev, const char *string);
+#endif /*__GENERIC_PHY_H */
1.9.1
Regards, Simon
Regards, Simon

This phy is found on omap platforms with sata capabilities. Except for the part related to the DM and the PHY framework, the code is basically a copy paste from arch/arm/mach-omap2/pipe3-phy.c
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- drivers/phy/Kconfig | 12 ++ drivers/phy/Makefile | 1 + drivers/phy/ti-pipe3-phy.c | 368 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 381 insertions(+) create mode 100644 drivers/phy/ti-pipe3-phy.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index b6fed9e..6a48343 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -19,4 +19,16 @@ config SPL_GENERIC_PHY This framework is designed to provide a generic interface for PHY devices.
+config PIPE3_PHY + bool "Support omap's PIPE3 PHY" + depends on GENERIC_PHY + help + Support for the omap PIPE3 phy for sata + +config SPL_PIPE3_PHY + bool "Support omap's PIPE3 PHY in SPL" + depends on SPL_GENERIC_PHY + help + Support for the omap PIPE3 phy for sata in SPL + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index ccd15ed..60c8a56 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -1,4 +1,5 @@ obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o +obj-$(CONFIG_$(SPL_)PIPE3_PHY) += ti-pipe3-phy.o
ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) obj-y += marvell/ diff --git a/drivers/phy/ti-pipe3-phy.c b/drivers/phy/ti-pipe3-phy.c new file mode 100644 index 0000000..94942d3 --- /dev/null +++ b/drivers/phy/ti-pipe3-phy.c @@ -0,0 +1,368 @@ +/* + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ + * Written by Jean-Jacques Hiblot jjhiblot@ti.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <common.h> +#include <dm.h> +#include <dm/device.h> +#include <generic-phy.h> +#include <asm/io.h> +#include <asm/arch/sys_proto.h> +#include <syscon.h> +#include <regmap.h> + +/* PLLCTRL Registers */ +#define PLL_STATUS 0x00000004 +#define PLL_GO 0x00000008 +#define PLL_CONFIGURATION1 0x0000000C +#define PLL_CONFIGURATION2 0x00000010 +#define PLL_CONFIGURATION3 0x00000014 +#define PLL_CONFIGURATION4 0x00000020 + +#define PLL_REGM_MASK 0x001FFE00 +#define PLL_REGM_SHIFT 9 +#define PLL_REGM_F_MASK 0x0003FFFF +#define PLL_REGM_F_SHIFT 0 +#define PLL_REGN_MASK 0x000001FE +#define PLL_REGN_SHIFT 1 +#define PLL_SELFREQDCO_MASK 0x0000000E +#define PLL_SELFREQDCO_SHIFT 1 +#define PLL_SD_MASK 0x0003FC00 +#define PLL_SD_SHIFT 10 +#define SET_PLL_GO 0x1 +#define PLL_TICOPWDN BIT(16) +#define PLL_LDOPWDN BIT(15) +#define PLL_LOCK 0x2 +#define PLL_IDLE 0x1 + +/* Software rest for the SATA PLL (in CTRL_CORE_SMA_SW_0 register)*/ +#define SATA_PLL_SOFT_RESET (1<<18) + +/* PHY POWER CONTROL Register */ +#define OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_MASK 0x003FC000 +#define OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT 0xE + +#define OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_MASK 0xFFC00000 +#define OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_SHIFT 0x16 + +#define OMAP_CTRL_PIPE3_PHY_TX_RX_POWERON 0x3 +#define OMAP_CTRL_PIPE3_PHY_TX_RX_POWEROFF 0x0 + + +#define PLL_IDLE_TIME 100 /* in milliseconds */ +#define PLL_LOCK_TIME 100 /* in milliseconds */ + +struct omap_pipe3 { + void __iomem *pll_ctrl_base; + void __iomem *power_reg; + void __iomem *pll_reset_reg; + struct pipe3_dpll_map *dpll_map; +}; + + +struct pipe3_dpll_params { + u16 m; + u8 n; + u8 freq:3; + u8 sd; + u32 mf; +}; + +struct pipe3_dpll_map { + unsigned long rate; + struct pipe3_dpll_params params; +}; + +static inline u32 omap_pipe3_readl(void __iomem *addr, unsigned offset) +{ + return readl(addr + offset); +} + +static inline void omap_pipe3_writel(void __iomem *addr, unsigned offset, + u32 data) +{ + writel(data, addr + offset); +} + +static struct pipe3_dpll_params *omap_pipe3_get_dpll_params(struct omap_pipe3 + *pipe3) +{ + u32 rate; + struct pipe3_dpll_map *dpll_map = pipe3->dpll_map; + + rate = get_sys_clk_freq(); + + for (; dpll_map->rate; dpll_map++) { + if (rate == dpll_map->rate) + return &dpll_map->params; + } + + printf("%s: No DPLL configuration for %u Hz SYS CLK\n", + __func__, rate); + return NULL; +} + +static int omap_pipe3_wait_lock(struct omap_pipe3 *pipe3) +{ + u32 val; + int timeout = PLL_LOCK_TIME; + + do { + mdelay(1); + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_STATUS); + if (val & PLL_LOCK) + break; + } while (--timeout); + + if (!(val & PLL_LOCK)) { + printf("%s: DPLL failed to lock\n", __func__); + return -EBUSY; + } + + return 0; +} + +static int omap_pipe3_dpll_program(struct omap_pipe3 *pipe3) +{ + u32 val; + struct pipe3_dpll_params *dpll_params; + + dpll_params = omap_pipe3_get_dpll_params(pipe3); + if (!dpll_params) { + printf("%s: Invalid DPLL parameters\n", __func__); + return -EINVAL; + } + + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION1); + val &= ~PLL_REGN_MASK; + val |= dpll_params->n << PLL_REGN_SHIFT; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION1, val); + + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION2); + val &= ~PLL_SELFREQDCO_MASK; + val |= dpll_params->freq << PLL_SELFREQDCO_SHIFT; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION2, val); + + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION1); + val &= ~PLL_REGM_MASK; + val |= dpll_params->m << PLL_REGM_SHIFT; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION1, val); + + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION4); + val &= ~PLL_REGM_F_MASK; + val |= dpll_params->mf << PLL_REGM_F_SHIFT; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION4, val); + + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION3); + val &= ~PLL_SD_MASK; + val |= dpll_params->sd << PLL_SD_SHIFT; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION3, val); + + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_GO, SET_PLL_GO); + + return omap_pipe3_wait_lock(pipe3); +} + +static void omap_control_pipe3_power(struct omap_pipe3 *pipe3, int on) +{ + u32 val, rate; + + val = readl(pipe3->power_reg); + + rate = get_sys_clk_freq(); + rate = rate/1000000; + + if (on) { + val &= ~(OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_MASK | + OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_MASK); + val |= OMAP_CTRL_PIPE3_PHY_TX_RX_POWERON << + OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT; + val |= rate << + OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_FREQ_SHIFT; + } else { + val &= ~OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_MASK; + val |= OMAP_CTRL_PIPE3_PHY_TX_RX_POWEROFF << + OMAP_CTRL_PIPE3_PHY_PWRCTL_CLK_CMD_SHIFT; + } + + writel(val, pipe3->power_reg); +} + +static int pipe3_power_on(struct udevice *dev) +{ + int ret; + u32 val; + struct omap_pipe3 *pipe3 = dev_get_priv(dev); + + /* Program the DPLL only if not locked */ + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_STATUS); + if (!(val & PLL_LOCK)) { + ret = omap_pipe3_dpll_program(pipe3); + if (ret) + return ret; + } else { + /* else just bring it out of IDLE mode */ + val = omap_pipe3_readl(pipe3->pll_ctrl_base, + PLL_CONFIGURATION2); + if (val & PLL_IDLE) { + val &= ~PLL_IDLE; + omap_pipe3_writel(pipe3->pll_ctrl_base, + PLL_CONFIGURATION2, val); + ret = omap_pipe3_wait_lock(pipe3); + if (ret) + return ret; + } + } + + /* Power up the PHY */ + omap_control_pipe3_power(pipe3, 1); + + return 0; +} + +static int pipe3_power_off(struct udevice *dev) +{ + u32 val; + int timeout = PLL_IDLE_TIME; + struct omap_pipe3 *pipe3 = dev_get_priv(dev); + + /* Power down the PHY */ + omap_control_pipe3_power(pipe3, 0); + + /* Put DPLL in IDLE mode */ + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_CONFIGURATION2); + val |= PLL_IDLE; + omap_pipe3_writel(pipe3->pll_ctrl_base, PLL_CONFIGURATION2, val); + + /* wait for LDO and Oscillator to power down */ + do { + mdelay(1); + val = omap_pipe3_readl(pipe3->pll_ctrl_base, PLL_STATUS); + if ((val & PLL_TICOPWDN) && (val & PLL_LDOPWDN)) + break; + } while (--timeout); + + if (!(val & PLL_TICOPWDN) || !(val & PLL_LDOPWDN)) { + printf("%s: Failed to power down DPLL: PLL_STATUS 0x%x\n", + __func__, val); + return -EBUSY; + } + + return 0; +} + +static int pipe3_reset(struct udevice *dev) +{ + u32 val; + struct omap_pipe3 *pipe3 = dev_get_priv(dev); + + val = readl(pipe3->pll_reset_reg); + writel(val | SATA_PLL_SOFT_RESET, pipe3->pll_reset_reg); + mdelay(1); + writel(val & ~SATA_PLL_SOFT_RESET, pipe3->pll_reset_reg); + return 0; +} + +static void *get_reg(struct udevice *dev, const char *name) +{ + struct udevice *syscon; + struct regmap *regmap; + const fdt32_t *cell; + int len, err; + void *base; + + err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev, + name, &syscon); + if (err) { + error("unable to find syscon device for %s (%d)\n", + name, err); + return NULL; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + error("unable to find regmap for %s (%ld)\n", + name, PTR_ERR(regmap)); + return NULL; + } + + cell = fdt_getprop(gd->fdt_blob, dev->of_offset, name, + &len); + if (len < 2*sizeof(fdt32_t)) { + error("offset not available for %s\n", name); + return NULL; + } + + base = regmap_get_range(regmap, 0); + if (!base) + return NULL; + + return fdtdec_get_number(cell + 1, 1) + base; +} + +static int pipe3_phy_probe(struct udevice *dev) +{ + fdt_addr_t addr; + fdt_size_t sz; + struct omap_pipe3 *pipe3 = dev_get_priv(dev); + + addr = dev_get_addr_size_index(dev, 2, &sz); + if (addr == FDT_ADDR_T_NONE) { + error("missing pll ctrl address\n"); + return -EINVAL; + } + + pipe3->pll_ctrl_base = map_physmem(addr, sz, MAP_NOCACHE); + if (!pipe3->pll_ctrl_base) { + error("unable to remap pll ctrl\n"); + return -EINVAL; + } + + pipe3->power_reg = get_reg(dev, "syscon-phy-power"); + if (!pipe3->power_reg) + return -EINVAL; + + pipe3->pll_reset_reg = get_reg(dev, "syscon-pllreset"); + if (!pipe3->pll_reset_reg) + return -EINVAL; + + pipe3->dpll_map = (struct pipe3_dpll_map *)dev_get_driver_data(dev); + + return 0; +} + +static struct pipe3_dpll_map dpll_map_sata[] = { + {12000000, {1000, 7, 4, 6, 0} }, /* 12 MHz */ + {16800000, {714, 7, 4, 6, 0} }, /* 16.8 MHz */ + {19200000, {625, 7, 4, 6, 0} }, /* 19.2 MHz */ + {20000000, {600, 7, 4, 6, 0} }, /* 20 MHz */ + {26000000, {461, 7, 4, 6, 0} }, /* 26 MHz */ + {38400000, {312, 7, 4, 6, 0} }, /* 38.4 MHz */ + { }, /* Terminator */ +}; + +static const struct udevice_id pipe3_phy_ids[] = { + { .compatible = "ti,phy-pipe3-sata", .data = (ulong)&dpll_map_sata }, + { } +}; + + +static struct generic_phy_ops pipe3_phy_ops = { + .power_on = pipe3_power_on, + .power_off = pipe3_power_off, + .reset = pipe3_reset +}; + +U_BOOT_DRIVER(pipe3_phy) = { + .name = "pipe3_phy", + .id = UCLASS_PHY, + .of_match = pipe3_phy_ids, + .ops = &pipe3_phy_ops, + .probe = pipe3_phy_probe, + .priv_auto_alloc_size = sizeof(struct omap_pipe3), +};

On Fri, Apr 07, 2017 at 01:42:03PM +0200, Jean-Jacques Hiblot wrote:
This phy is found on omap platforms with sata capabilities. Except for the part related to the DM and the PHY framework, the code is basically a copy paste from arch/arm/mach-omap2/pipe3-phy.c
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This phy is found on omap platforms with sata capabilities. Except for the part related to the DM and the PHY framework, the code is basically a copy paste from arch/arm/mach-omap2/pipe3-phy.c
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
drivers/phy/Kconfig | 12 ++ drivers/phy/Makefile | 1 + drivers/phy/ti-pipe3-phy.c | 368 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 381 insertions(+) create mode 100644 drivers/phy/ti-pipe3-phy.c
Reviewed-by: Simon Glass sjg@chromium.org

This is needed to probe devices under that bus such as the SATA PHY.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- arch/arm/dts/dra7.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/dra7.dtsi b/arch/arm/dts/dra7.dtsi index 5570e30..6978359 100644 --- a/arch/arm/dts/dra7.dtsi +++ b/arch/arm/dts/dra7.dtsi @@ -1317,7 +1317,7 @@
/* OCP2SCP3 */ ocp2scp@4a090000 { - compatible = "ti,omap-ocp2scp"; + compatible = "ti,omap-ocp2scp", "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges;

On Fri, Apr 07, 2017 at 01:42:04PM +0200, Jean-Jacques Hiblot wrote:
This is needed to probe devices under that bus such as the SATA PHY.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/dts/dra7.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/dra7.dtsi b/arch/arm/dts/dra7.dtsi index 5570e30..6978359 100644 --- a/arch/arm/dts/dra7.dtsi +++ b/arch/arm/dts/dra7.dtsi @@ -1317,7 +1317,7 @@
/* OCP2SCP3 */ ocp2scp@4a090000 {
compatible = "ti,omap-ocp2scp";
compatible = "ti,omap-ocp2scp", "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges;
This is U-Boot specific I assume? It should end up in a -u-boot.dtsi file instead so we don't overwrite it later.

On 09/04/2017 03:13, Tom Rini wrote:
On Fri, Apr 07, 2017 at 01:42:04PM +0200, Jean-Jacques Hiblot wrote:
This is needed to probe devices under that bus such as the SATA PHY.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
arch/arm/dts/dra7.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/dra7.dtsi b/arch/arm/dts/dra7.dtsi index 5570e30..6978359 100644 --- a/arch/arm/dts/dra7.dtsi +++ b/arch/arm/dts/dra7.dtsi @@ -1317,7 +1317,7 @@
/* OCP2SCP3 */ ocp2scp@4a090000 {
compatible = "ti,omap-ocp2scp";
compatible = "ti,omap-ocp2scp", "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges;
This is U-Boot specific I assume? It should end up in a -u-boot.dtsi file instead so we don't overwrite it later.
OK. I'll put it in omap5-u-boot.dtsi

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 | 10 +++++ drivers/block/Makefile | 1 + drivers/block/dwc_ahci.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+) create mode 100644 drivers/block/dwc_ahci.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 88e66e2..b3d35bd 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -48,4 +48,14 @@ 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 + select GENERIC_PHY + 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..bf44946 --- /dev/null +++ b/drivers/block/dwc_ahci.c @@ -0,0 +1,100 @@ +/* + * 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> +#include <generic-phy.h> + +DECLARE_GLOBAL_DATA_PTR; + +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); + struct scsi_platdata *plat = dev_get_platdata(dev); + fdt_addr_t addr; + + plat->max_id = fdtdec_get_uint(gd->fdt_blob, dev->of_offset, "max-id", + CONFIG_SYS_SCSI_MAX_SCSI_ID); + plat->max_lun = fdtdec_get_uint(gd->fdt_blob, dev->of_offset, + "max-lun", CONFIG_SYS_SCSI_MAX_LUN); + + 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; + struct generic_phy *phy = dm_generic_phy_get(dev, "phys"); + + if (IS_ERR(phy)) { + error("can't get the phy from DT\n"); + return PTR_ERR(phy); + } + + ret = generic_phy_init(phy); + if (ret) { + error("unable to initialize the sata phy\n"); + return ret; + } + + ret = generic_phy_power_on(phy); + if (ret) { + error("unable to power on the sata phy\n"); + return 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); + } + + 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, +};

On Fri, Apr 07, 2017 at 01:42:05PM +0200, Jean-Jacques Hiblot wrote:
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
Reviewed-by: Tom Rini trini@konsulko.com

On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
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 | 10 +++++ drivers/block/Makefile | 1 + drivers/block/dwc_ahci.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+) create mode 100644 drivers/block/dwc_ahci.c
Reviewed-by: Simon Glass sjg@chromium.org

This is a cosmetic change. target and LUN have kind of the same role in this function. One of them was passed as a parameter and the other was embedded in a structure. For consistency, pass both of them as parameters.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- common/scsi.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index fb5b407..d55ba89 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -473,14 +473,15 @@ static void scsi_init_dev_desc(struct blk_desc *dev_desc, int devnum) * scsi_detect_dev - Detect scsi device * * @target: target id + * @lun: target lun * @dev_desc: block device description * * The scsi_detect_dev detects and fills a dev_desc structure when the device is - * detected. The LUN number is taken from the struct blk_desc *dev_desc. + * detected. * * 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, int lun, struct blk_desc *dev_desc) { unsigned char perq, modi; lbaint_t capacity; @@ -488,7 +489,7 @@ static int scsi_detect_dev(int target, struct blk_desc *dev_desc) ccb *pccb = (ccb *)&tempccb;
pccb->target = target; - pccb->lun = dev_desc->lun; + pccb->lun = lun; pccb->pdata = (unsigned char *)&tempbuff; pccb->datalen = 512; scsi_setup_inquiry(pccb); @@ -599,8 +600,7 @@ int scsi_scan(int mode) bdesc = dev_get_uclass_platdata(bdev);
scsi_init_dev_desc_priv(bdesc); - bdesc->lun = lun; - ret = scsi_detect_dev(i, bdesc); + ret = scsi_detect_dev(i, lun, bdesc); if (ret) { device_unbind(bdev); continue; @@ -630,8 +630,8 @@ int scsi_scan(int mode) scsi_max_devs = 0; 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, lun, + &scsi_dev_desc[scsi_max_devs]); if (ret) continue;

On Fri, Apr 07, 2017 at 01:42:06PM +0200, Jean-Jacques Hiblot wrote:
This is a cosmetic change. target and LUN have kind of the same role in this function. One of them was passed as a parameter and the other was embedded in a structure. For consistency, pass both of them as parameters.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This is a cosmetic change. target and LUN have kind of the same role in this function. One of them was passed as a parameter and the other was embedded in a structure. For consistency, pass both of them as parameters.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 9 April 2017 at 13:27, Simon Glass sjg@chromium.org wrote:
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
This is a cosmetic change. target and LUN have kind of the same role in this function. One of them was passed as a parameter and the other was embedded in a structure. For consistency, pass both of them as parameters.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

We might want to get information about the scsi device without initializing the partition.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- common/scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/scsi.c b/common/scsi.c index d55ba89..972ef338 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -540,7 +540,6 @@ static int scsi_detect_dev(int target, int lun, 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]); removable: return 0; } @@ -605,6 +604,7 @@ int scsi_scan(int mode) device_unbind(bdev); continue; } + part_init(bdesc);
if (mode == 1) { printf(" Device %d: ", 0); @@ -634,6 +634,7 @@ int scsi_scan(int mode) &scsi_dev_desc[scsi_max_devs]); if (ret) continue; + part_init(&scsi_dev_desc[scsi_max_devs]);
if (mode == 1) { printf(" Device %d: ", 0);

On Fri, Apr 07, 2017 at 01:42:07PM +0200, Jean-Jacques Hiblot wrote:
We might want to get information about the scsi device without initializing the partition.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
We might want to get information about the scsi device without initializing the partition.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

On 9 April 2017 at 13:27, Simon Glass sjg@chromium.org wrote:
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
We might want to get information about the scsi device without initializing the partition.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!

With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading to a divide-by-0 exception. scsi_detect_dev() can be used to get the required parameters (block size and number of blocks) from the drive before calling blk_create_devicef().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com --- common/scsi.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/common/scsi.c b/common/scsi.c index 972ef338..d37222c 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -580,9 +580,19 @@ 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); + ret = scsi_detect_dev(i, lun, &_bd); + 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,20 +600,27 @@ 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);
- scsi_init_dev_desc_priv(bdesc); - ret = scsi_detect_dev(i, lun, bdesc); - if (ret) { - device_unbind(bdev); - continue; - } + bdesc = dev_get_uclass_platdata(bdev); + bdesc->target = i; + bdesc->lun = lun; + bdesc->removable = _bd.removable; + bdesc->type = _bd.type; + memcpy(&bdesc->vendor, &_bd.vendor, + sizeof(_bd.vendor)); + memcpy(&bdesc->product, &_bd.product, + sizeof(_bd.product)); + memcpy(&bdesc->revision, &_bd.revision, + sizeof(_bd.revision)); part_init(bdesc);
if (mode == 1) {

On Fri, Apr 07, 2017 at 01:42:08PM +0200, Jean-Jacques Hiblot wrote:
With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading to a divide-by-0 exception. scsi_detect_dev() can be used to get the required parameters (block size and number of blocks) from the drive before calling blk_create_devicef().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Reviewed-by: Tom Rini trini@konsulko.com

Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading to a divide-by-0 exception. scsi_detect_dev() can be used to get the required parameters (block size and number of blocks) from the drive before calling blk_create_devicef().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/common/scsi.c b/common/scsi.c index 972ef338..d37222c 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -580,9 +580,19 @@ 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);
ret = scsi_detect_dev(i, lun, &_bd);
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,20 +600,27 @@ 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);
scsi_init_dev_desc_priv(bdesc);
ret = scsi_detect_dev(i, lun, bdesc);
if (ret) {
device_unbind(bdev);
continue;
}
bdesc = dev_get_uclass_platdata(bdev);
bdesc->target = i;
bdesc->lun = lun;
bdesc->removable = _bd.removable;
bdesc->type = _bd.type;
memcpy(&bdesc->vendor, &_bd.vendor,
sizeof(_bd.vendor));
memcpy(&bdesc->product, &_bd.product,
sizeof(_bd.product));
memcpy(&bdesc->revision, &_bd.revision,
sizeof(_bd.revision));
Can you please move this block (inside the double for loops) into a separate function? It is getting too long. A follow-up patch is fine since you have already done this.
part_init(bdesc); if (mode == 1) {
-- 1.9.1
Regards, Simon

On 9 April 2017 at 13:27, Simon Glass sjg@chromium.org wrote:
Hi,
On 7 April 2017 at 05:42, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
With DM_SCSI enabled, blk_create_devicef() is called with blkz = 0, leading to a divide-by-0 exception. scsi_detect_dev() can be used to get the required parameters (block size and number of blocks) from the drive before calling blk_create_devicef().
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
common/scsi.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/common/scsi.c b/common/scsi.c index 972ef338..d37222c 100644 --- a/common/scsi.c +++ b/common/scsi.c @@ -580,9 +580,19 @@ 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);
ret = scsi_detect_dev(i, lun, &_bd);
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,20 +600,27 @@ 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);
scsi_init_dev_desc_priv(bdesc);
ret = scsi_detect_dev(i, lun, bdesc);
if (ret) {
device_unbind(bdev);
continue;
}
bdesc = dev_get_uclass_platdata(bdev);
bdesc->target = i;
bdesc->lun = lun;
bdesc->removable = _bd.removable;
bdesc->type = _bd.type;
memcpy(&bdesc->vendor, &_bd.vendor,
sizeof(_bd.vendor));
memcpy(&bdesc->product, &_bd.product,
sizeof(_bd.product));
memcpy(&bdesc->revision, &_bd.revision,
sizeof(_bd.revision));
Can you please move this block (inside the double for loops) into a separate function? It is getting too long. A follow-up patch is fine since you have already done this.
part_init(bdesc); if (mode == 1) {
-- 1.9.1
Regards, Simon
Applied to u-boot-dm, thanks!

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 | 12 +++++++++++- configs/dra7xx_hs_evm_defconfig | 11 ++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig index 42f87b3..8bc395f 100644 --- a/configs/dra7xx_evm_defconfig +++ b/configs/dra7xx_evm_defconfig @@ -59,7 +59,13 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIST="dra7-evm dra72-evm dra72-evm-revc dra71-evm" CONFIG_DM=y CONFIG_SPL_DM=y -# CONFIG_BLK is not set +CONFIG_REGMAP=y +CONFIG_SPL_REGMAP=y +CONFIG_SYSCON=y +CONFIG_SPL_SYSCON=y +CONFIG_BLK=y +CONFIG_DM_SCSI=y +CONFIG_DWC_AHCI=y CONFIG_DFU_MMC=y CONFIG_DFU_RAM=y CONFIG_DFU_SF=y @@ -89,6 +95,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 +108,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_SPL_GENERIC_PHY=y +CONFIG_PIPE3_PHY=y +CONFIG_SPL_PIPE3_PHY=y diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig index f3a9c68..dce3da2 100644 --- a/configs/dra7xx_hs_evm_defconfig +++ b/configs/dra7xx_hs_evm_defconfig @@ -63,7 +63,12 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_LIST="dra7-evm dra72-evm dra72-evm-revc dra71-evm" CONFIG_DM=y CONFIG_SPL_DM=y -# CONFIG_BLK is not set +CONFIG_REGMAP=y +CONFIG_SPL_REGMAP=y +CONFIG_SYSCON=y +CONFIG_SPL_SYSCON=y +CONFIG_DM_SCSI=y +CONFIG_DWC_AHCI=y CONFIG_DFU_MMC=y CONFIG_DFU_RAM=y CONFIG_DFU_SF=y @@ -93,6 +98,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 +111,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_SPL_GENERIC_PHY=y +CONFIG_PIPE3_PHY=y +CONFIG_SPL_PIPE3_PHY=y

On Fri, Apr 07, 2017 at 01:42:09PM +0200, Jean-Jacques Hiblot 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
Reviewed-by: Tom Rini trini@konsulko.com
participants (3)
-
Jean-Jacques Hiblot
-
Simon Glass
-
Tom Rini