[U-Boot] [PATCH v3 0/6] arm64: mvebu: Add support for A8K community board
From: Konstantin Porotchkin kostap@marvell.com
This patch series adds initil support for A8K community board MACCHIATOBin manufactured by SolidRun. It should be applied on top of Stefan Roese patches adding support for SD/eMMC devices on Marvell A37x0/A80x0/A70x0 SoCs. The top level patch is called: "arm64: mvebu: Enable SDHCI/MMC support for the db-88f7040/8040" The default board DTS file is based on the original work of Rabeeh Khory from SolidRun.
Changes for v3: - Remove bubt command fixes - thay were already merged to the master branch - Moved the USB VBUS control from proprietary GPIO to a fixed regulator - Added SD and eMMC entries to the DTS - Aligned default configutration with Armada-8040-DB - Rebased on top of master branch
Konstantin Porotchkin (5): arm64: mvebu: gpio: Add GPIO nodes to A8K family devices arm64: mvebu: dts: Add i2c1 pin definitions to CPM mvebu: pcie: Add support for GPIO reset for PCIe device mvebu: usb: xhci: Add VBUS regulator supply to the host driver arm64: mvebu: Add default configuraton for MACCHIATOBin board
Rabeeh Khoury (1): arm64: mvebu: dts: Add DTS file for MACCHIATOBin board
arch/arm/dts/Makefile | 1 + arch/arm/dts/armada-7040.dtsi | 1 + arch/arm/dts/armada-8040-mcbin.dts | 287 ++++++++++++++++++++++ arch/arm/dts/armada-8040.dtsi | 1 + arch/arm/dts/armada-ap806.dtsi | 8 + arch/arm/dts/armada-cp110-master.dtsi | 22 ++ arch/arm/dts/armada-cp110-slave.dtsi | 18 ++ configs/mvebu_mcbin-88f8040_defconfig | 65 +++++ doc/device-tree-bindings/pci/armada8k-pcie.txt | 49 ++++ doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 +++ drivers/pci/pcie_dw_mvebu.c | 20 ++ drivers/usb/host/xhci-mvebu.c | 31 +++ 12 files changed, 531 insertions(+) create mode 100644 arch/arm/dts/armada-8040-mcbin.dts create mode 100644 configs/mvebu_mcbin-88f8040_defconfig create mode 100644 doc/device-tree-bindings/pci/armada8k-pcie.txt create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
From: Konstantin Porotchkin kostap@marvell.com
Add GPIO nodes to AP-806 and CP-110-master DTSI files.
Change-Id: I05958698d460cb721b7d8683d34f74a5ea32532c Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Rebase on top of master branch
arch/arm/dts/armada-7040.dtsi | 1 + arch/arm/dts/armada-8040.dtsi | 1 + arch/arm/dts/armada-ap806.dtsi | 8 ++++++++ arch/arm/dts/armada-cp110-master.dtsi | 18 ++++++++++++++++++ arch/arm/dts/armada-cp110-slave.dtsi | 18 ++++++++++++++++++ 5 files changed, 46 insertions(+)
diff --git a/arch/arm/dts/armada-7040.dtsi b/arch/arm/dts/armada-7040.dtsi index 78d995d..b5be0c4 100644 --- a/arch/arm/dts/armada-7040.dtsi +++ b/arch/arm/dts/armada-7040.dtsi @@ -45,6 +45,7 @@ * one CP110. */
+#include <dt-bindings/gpio/gpio.h> #include "armada-ap806-quad.dtsi" #include "armada-cp110-master.dtsi"
diff --git a/arch/arm/dts/armada-8040.dtsi b/arch/arm/dts/armada-8040.dtsi index 9c1b28c..96cc112 100644 --- a/arch/arm/dts/armada-8040.dtsi +++ b/arch/arm/dts/armada-8040.dtsi @@ -45,6 +45,7 @@ * two CP110. */
+#include <dt-bindings/gpio/gpio.h> #include "armada-ap806-quad.dtsi" #include "armada-cp110-master.dtsi" #include "armada-cp110-slave.dtsi" diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi index 3042cb1..e0d3016 100644 --- a/arch/arm/dts/armada-ap806.dtsi +++ b/arch/arm/dts/armada-ap806.dtsi @@ -158,6 +158,14 @@ }; };
+ ap_gpio0: gpio@6F5040 { + compatible = "marvell,orion-gpio"; + reg = <0x6F5040 0x40>; + ngpios = <20>; + gpio-controller; + #gpio-cells = <2>; + }; + xor@400000 { compatible = "marvell,mv-xor-v2"; reg = <0x400000 0x1000>, diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi index 661a696..6095609 100644 --- a/arch/arm/dts/armada-cp110-master.dtsi +++ b/arch/arm/dts/armada-cp110-master.dtsi @@ -113,6 +113,24 @@ }; };
+ cpm_gpio0: gpio@440100 { + compatible = "marvell,orion-gpio"; + reg = <0x440100 0x40>; + ngpios = <32>; + gpiobase = <20>; + gpio-controller; + #gpio-cells = <2>; + }; + + cpm_gpio1: gpio@440140 { + compatible = "marvell,orion-gpio"; + reg = <0x440140 0x40>; + ngpios = <31>; + gpiobase = <52>; + gpio-controller; + #gpio-cells = <2>; + }; + cpm_sata0: sata@540000 { compatible = "marvell,armada-8k-ahci"; reg = <0x540000 0x30000>; diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi index 92ef55c..ff3fbed 100644 --- a/arch/arm/dts/armada-cp110-slave.dtsi +++ b/arch/arm/dts/armada-cp110-slave.dtsi @@ -100,6 +100,24 @@ }; };
+ cps_gpio0: gpio@440100 { + compatible = "marvell,orion-gpio"; + reg = <0x440100 0x40>; + ngpios = <32>; + gpiobase = <20>; + gpio-controller; + #gpio-cells = <2>; + }; + + cps_gpio1: gpio@440140 { + compatible = "marvell,orion-gpio"; + reg = <0x440140 0x40>; + ngpios = <31>; + gpiobase = <52>; + gpio-controller; + #gpio-cells = <2>; + }; + cps_sata0: sata@540000 { compatible = "marvell,armada-8k-ahci"; reg = <0x540000 0x30000>;
On 08.02.2017 16:34, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
Add GPIO nodes to AP-806 and CP-110-master DTSI files.
Change-Id: I05958698d460cb721b7d8683d34f74a5ea32532c Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Complete series applied to u-boot-mavell/master.
Thanks, Stefan
From: Konstantin Porotchkin kostap@marvell.com
Add i2c-1 pin mappings to CP0(master) DTSI file
Change-Id: I0c6e6de8a557393f518f7df8e6daa6dfce1788b0 Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Rebase on top of master branch
arch/arm/dts/armada-cp110-master.dtsi | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi index 6095609..1f0edde 100644 --- a/arch/arm/dts/armada-cp110-master.dtsi +++ b/arch/arm/dts/armada-cp110-master.dtsi @@ -94,6 +94,10 @@ marvell,pins = < 37 38 >; marvell,function = <2>; }; + cpm_i2c1_pins: cpm-i2c-pins-1 { + marvell,pins = < 35 36 >; + marvell,function = <2>; + }; cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 { marvell,pins = < 44 45 46 47 48 49 50 51 52 53 54 55 >;
From: Konstantin Porotchkin kostap@marvell.com
Add support for "marvell,reset-gpio" property to mvebu DW PCIe driver. This option is valid when CONFIG_DM_GPIO=y
Change-Id: Ic17c500449050c2fbb700731f1a9ca8b83298986 Signed-off-by: Konstantin Porotchkin kostap@marvell.com Signed-off-by: Rabeeh Khoury rabeeh@solid-run.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Rebase on top of master branch
doc/device-tree-bindings/pci/armada8k-pcie.txt | 49 ++++++++++++++++++++++++++ drivers/pci/pcie_dw_mvebu.c | 20 +++++++++++ 2 files changed, 69 insertions(+) create mode 100644 doc/device-tree-bindings/pci/armada8k-pcie.txt
diff --git a/doc/device-tree-bindings/pci/armada8k-pcie.txt b/doc/device-tree-bindings/pci/armada8k-pcie.txt new file mode 100644 index 0000000..7230f10 --- /dev/null +++ b/doc/device-tree-bindings/pci/armada8k-pcie.txt @@ -0,0 +1,49 @@ +Armada-8K PCIe DT details: +========================== + +Armada-8k uses synopsis designware PCIe controller. + +Required properties: +- compatible : should be "marvell,armada8k-pcie", "snps,dw-pcie". +- reg: base addresses and lengths of the pcie control and global control registers. + "ctrl" registers points to the global control registers, while the "config" space + points to the pcie configuration registers as mentioned in dw-pcie dt bindings in the link below. +- interrupt-map-mask and interrupt-map, standard PCI properties to + define the mapping of the PCIe interface to interrupt numbers. +- All other definitions as per generic PCI bindings +See Linux kernel documentation: +"Documentation/devicetree/bindings/pci/designware-pcie.txt" + +Optional properties: +PHY support is still not supported for armada-8k, once it will, the following parameters can be used: +- phys : phandle to phy node associated with pcie controller. +- phy-names : must be "pcie-phy" +- marvell,reset-gpio : specifies a gpio that needs to be activated for plug-in + card reset signal release. +Example: + +cpm_pcie0: pcie@f2600000 { + compatible = "marvell,armada8k-pcie", "snps,dw-pcie"; + reg = <0 0xf2600000 0 0x10000>, + <0 0xf6f00000 0 0x80000>; + reg-names = "ctrl", "config"; + #address-cells = <3>; + #size-cells = <2>; + #interrupt-cells = <1>; + device_type = "pci"; + dma-coherent; + + bus-range = <0 0xff>; + ranges = + /* downstream I/O */ + <0x81000000 0 0xf9000000 0 0xf9000000 0 0x10000 + /* non-prefetchable memory */ + 0x82000000 0 0xf6000000 0 0xf6000000 0 0xf00000>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic 0 GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>; + num-lanes = <1>; + clocks = <&cpm_syscon0 1 13>; + marvell,reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_HIGH>; + status = "disabled"; +}; diff --git a/drivers/pci/pcie_dw_mvebu.c b/drivers/pci/pcie_dw_mvebu.c index 17fa024..d4776a9 100644 --- a/drivers/pci/pcie_dw_mvebu.c +++ b/drivers/pci/pcie_dw_mvebu.c @@ -15,6 +15,7 @@ #include <dm.h> #include <pci.h> #include <asm/io.h> +#include <asm-generic/gpio.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -461,6 +462,25 @@ static int pcie_dw_mvebu_probe(struct udevice *dev) struct pcie_dw_mvebu *pcie = dev_get_priv(dev); struct udevice *ctlr = pci_get_controller(dev); struct pci_controller *hose = dev_get_uclass_priv(ctlr); +#ifdef CONFIG_DM_GPIO + struct gpio_desc reset_gpio; + + gpio_request_by_name(dev, "marvell,reset-gpio", 0, &reset_gpio, + GPIOD_IS_OUT); + /* + * Issue reset to add-in card trough the dedicated GPIO. + * Some boards are connecting the card reset pin to common system + * reset wire and others are using separate GPIO port. + * In the last case we have to release a reset of the addon card + * using this GPIO. + */ + if (dm_gpio_is_valid(&reset_gpio)) { + dm_gpio_set_value(&reset_gpio, 1); + mdelay(200); + } +#else + debug("PCIE Reset on GPIO support is missing\n"); +#endif /* CONFIG_DM_GPIO */
pcie->first_busno = dev->seq;
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Moved VBUS control from private GPIO to a fixed regulator - Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 ++++++++++++++++++++ drivers/usb/host/xhci-mvebu.c | 31 +++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers + +This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller + +Required properties : + - compatible: should be one or more of: + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs + - reg: should contain address and length of the standard XHCI + register set for the device. + - interrupts: one XHCI interrupt should be described here. + +Optional properties: + - clocks: reference to a clock + - vbus-supply : If present, specifies the fixed regulator to be turned on + for providing power to the USB VBUS rail. + +Example: + cpm_usb3_0: usb3@500000 { + compatible = "marvell,armada-8k-xhci", + "generic-xhci"; + reg = <0x500000 0x4000>; + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpm_syscon0 1 22>; + vbus-supply = <®_usb3h0_vbus>; + status = "disabled"; + }; diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED + const void *fdt = gd->fdt_blob; + int node = dev->of_offset; + const fdt32_t *regulator; + int size;
+ /* + * The VBUS supply regulator is not probed automatically + * Trigger the regulator probe upon USB port bring up + */ + regulator = fdt_getprop(fdt, node, "vbus-supply", &size); + if (regulator) { + uint32_t phandle; + struct udevice *config; + int reg_node, ret; + + phandle = fdt32_to_cpu(*regulator); + reg_node = fdt_node_offset_by_phandle(fdt, phandle); + if (reg_node < 0) { + dev_err(dev, "vbus-supply has invalid phandle\n"); + return -EINVAL; + } + ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR, + reg_node, &config); + if (ret) { + dev_err(dev, "failed to get VBUS regulator device\n"); + return ret; + } + } +#else + debug("VBUS regulator support is missing\n"); +#endif ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len);
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 ++++++++++++++++++++ drivers/usb/host/xhci-mvebu.c | 31 +++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
- vbus-supply : If present, specifies the fixed regulator to be turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;
Where is the regulator enabled ?
}- }
+#else
- debug("VBUS regulator support is missing\n");
+#endif ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len);
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 ++++++++++++++++++++ drivers/usb/host/xhci-mvebu.c | 31 +++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel. I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
- vbus-supply : If present, specifies the fixed regulator to be turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
}- }
+#else
- debug("VBUS regulator support is missing\n");
+#endif ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len);
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter
probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28
++++++++++++++++++++
drivers/usb/host/xhci-mvebu.c | 31
+++++++++++++++++++++++
2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
- vbus-supply : If present, specifies the fixed regulator to be
turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c
b/drivers/usb/host/xhci-mvebu.c
index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
- if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
}- }
+#else
- debug("VBUS regulator support is missing\n");
+#endif ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len);
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter
probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28
++++++++++++++++++++
drivers/usb/host/xhci-mvebu.c | 31
+++++++++++++++++++++++
2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it. Should I simply remove this property from the text file?
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
- vbus-supply : If present, specifies the fixed regulator to be
turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c
b/drivers/usb/host/xhci-mvebu.c
index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function.
- if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
What other type of regulators can be used for powering on the USB port? I believe they are always 5V fixed, are they?
}- }
+#else
- debug("VBUS regulator support is missing\n");
+#endif ctx->hcd = (struct xhci_hccr *)plat->hcd_base; len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)); hcor = (struct xhci_hcor *)((uintptr_t)ctx->hcd + len);
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter
probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28
++++++++++++++++++++
drivers/usb/host/xhci-mvebu.c | 31
+++++++++++++++++++++++
2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
Should I simply remove this property from the text file?
See above.
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
- vbus-supply : If present, specifies the fixed regulator to be
turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c
b/drivers/usb/host/xhci-mvebu.c
index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function.
regulator_get_by_platname("vbus-supply", ®); doesn't work here ?
- if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
What other type of regulators can be used for powering on the USB port? I believe they are always 5V fixed, are they?
Anything which can turn 5V on/off , be it some i2c chip, spi chip, GPIO ...
On 02/08/2017 06:42 PM, Marek Vasut wrote:
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote:
From: Konstantin Porotchkin kostap@marvell.com
The USB device should linked to VBUS regulator through "vbus-supply" DTS property. This patch adds handling for "vbus-supply" property inside the USB device entry for turning on the VBUS regulator upon the host adapter
probe.
Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Marek Vasut marex@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com
Changes for v3:
- Moved VBUS control from private GPIO to a fixed regulator
- Rebase on top of master branch
doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28
++++++++++++++++++++
drivers/usb/host/xhci-mvebu.c | 31
+++++++++++++++++++++++
2 files changed, 59 insertions(+) create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
new file mode 100644 index 0000000..672a829 --- /dev/null +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt @@ -0,0 +1,28 @@ +Marvell SOC USB controllers
+This controller is integrated in Armada 3700/8K. +It uses the same properties as a generic XHCI host controller
+Required properties :
- compatible: should be one or more of:
- "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
- "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
- reg: should contain address and length of the standard XHCI
- register set for the device.
- interrupts: one XHCI interrupt should be described here.
+Optional properties:
- clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
I see what you saying. I will move the "clocks" entry to the "mandatory" section. Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry.
Should I simply remove this property from the text file?
See above.
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
- vbus-supply : If present, specifies the fixed regulator to be
turned on
- for providing power to the USB VBUS rail.
+Example:
- cpm_usb3_0: usb3@500000 {
compatible = "marvell,armada-8k-xhci","generic-xhci";reg = <0x500000 0x4000>;interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;clocks = <&cpm_syscon0 1 22>;vbus-supply = <®_usb3h0_vbus>;status = "disabled";- };
diff --git a/drivers/usb/host/xhci-mvebu.c
b/drivers/usb/host/xhci-mvebu.c
index 46eb937..149f6a4 100644 --- a/drivers/usb/host/xhci-mvebu.c +++ b/drivers/usb/host/xhci-mvebu.c @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) struct mvebu_xhci *ctx = dev_get_priv(dev); struct xhci_hcor *hcor; int len; +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
Got it. I will change the code for using the regulator framework in USB driver.
const void *fdt = gd->fdt_blob;
int node = dev->of_offset;
const fdt32_t *regulator;
int size;
/*
* The VBUS supply regulator is not probed automatically* Trigger the regulator probe upon USB port bring up*/regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function.
regulator_get_by_platname("vbus-supply", ®); doesn't work here ?
Thank you for the tip! i will definitely try this. Unfortunately I am not yet fluent in all the available DM APIs.
- if (regulator) {
uint32_t phandle;struct udevice *config;int reg_node, ret;phandle = fdt32_to_cpu(*regulator);reg_node = fdt_node_offset_by_phandle(fdt, phandle);if (reg_node < 0) {dev_err(dev, "vbus-supply has invalid phandle\n");return -EINVAL;}ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR,reg_node, &config);if (ret) {dev_err(dev, "failed to get VBUS regulator device\n");return ret;Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
What other type of regulators can be used for powering on the USB port? I believe they are always 5V fixed, are they?
Anything which can turn 5V on/off , be it some i2c chip, spi chip, GPIO ...
On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote:
On 02/08/2017 06:42 PM, Marek Vasut wrote:
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote:
On 02/08/2017 04:34 PM, kostap@marvell.com wrote: > From: Konstantin Porotchkin kostap@marvell.com > > The USB device should linked to VBUS regulator through "vbus-supply" > DTS property. > This patch adds handling for "vbus-supply" property inside the USB > device entry for turning on the VBUS regulator upon the host adapter probe. > > Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de > Signed-off-by: Konstantin Porotchkin kostap@marvell.com > Cc: Stefan Roese sr@denx.de > Cc: Marek Vasut marex@denx.de > Cc: Nadav Haklai nadavh@marvell.com > Cc: Neta Zur Hershkovits neta@marvell.com > Cc: Igal Liberman igall@marvell.com > Cc: Haim Boot hayim@marvell.com > --- > Changes for v3: > - Moved VBUS control from private GPIO to a fixed regulator > - Rebase on top of master branch > > > doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 ++++++++++++++++++++ > drivers/usb/host/xhci-mvebu.c | 31 +++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > create mode 100644 > doc/device-tree-bindings/usb/marvell.xhci-usb.txt > > diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt > new file mode 100644 > index 0000000..672a829 > --- /dev/null > +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt > @@ -0,0 +1,28 @@ > +Marvell SOC USB controllers > + > +This controller is integrated in Armada 3700/8K. > +It uses the same properties as a generic XHCI host controller > + > +Required properties : > + - compatible: should be one or more of: > + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs > + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs > + - reg: should contain address and length of the standard XHCI > + register set for the device. > + - interrupts: one XHCI interrupt should be described here. > + > +Optional properties: > + - clocks: reference to a clock
What clock ? Why are clock optional ? This probably needs clock-names too.
This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
I see what you saying. I will move the "clocks" entry to the "mandatory" section.
Why ? What clock are those ? Are they mandatory ?
Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry.
Then why are these clock here at all ?
Should I simply remove this property from the text file?
See above.
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on
> + - vbus-supply : If present, specifies the fixed regulator to be turned on > + for providing power to the USB VBUS rail. > + > +Example: > + cpm_usb3_0: usb3@500000 { > + compatible = "marvell,armada-8k-xhci", > + "generic-xhci"; > + reg = <0x500000 0x4000>; > + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpm_syscon0 1 22>; > + vbus-supply = <®_usb3h0_vbus>; > + status = "disabled"; > + }; > diff --git a/drivers/usb/host/xhci-mvebu.c b/drivers/usb/host/xhci-mvebu.c > index 46eb937..149f6a4 100644 > --- a/drivers/usb/host/xhci-mvebu.c > +++ b/drivers/usb/host/xhci-mvebu.c > @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) > struct mvebu_xhci *ctx = dev_get_priv(dev); > struct xhci_hcor *hcor; > int len; > +#ifdef CONFIG_DM_REGULATOR_FIXED
Just make the driver depend on REGULATOR_FIXED
I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
Got it. I will change the code for using the regulator framework in USB driver.
Thanks :)
> + const void *fdt = gd->fdt_blob; > + int node = dev->of_offset; > + const fdt32_t *regulator; > + int size; > > + /* > + * The VBUS supply regulator is not probed automatically > + * Trigger the regulator probe upon USB port bring up > + */ > + regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function.
regulator_get_by_platname("vbus-supply", ®); doesn't work here ?
Thank you for the tip! i will definitely try this. Unfortunately I am not yet fluent in all the available DM APIs.
That's what the review is for .
> + if (regulator) { > + uint32_t phandle; > + struct udevice *config; > + int reg_node, ret; > + > + phandle = fdt32_to_cpu(*regulator); > + reg_node = fdt_node_offset_by_phandle(fdt, phandle); > + if (reg_node < 0) { > + dev_err(dev, "vbus-supply has invalid phandle\n"); > + return -EINVAL; > + } > + ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR, > + reg_node, &config); > + if (ret) { > + dev_err(dev, "failed to get VBUS regulator device\n"); > + return ret;
Where is the regulator enabled ?
The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
What other type of regulators can be used for powering on the USB port? I believe they are always 5V fixed, are they?
Anything which can turn 5V on/off , be it some i2c chip, spi chip, GPIO ...
On 02/09/2017 10:32 AM, Marek Vasut wrote:
On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote:
On 02/08/2017 06:42 PM, Marek Vasut wrote:
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 05:35 PM, Marek Vasut wrote: > On 02/08/2017 04:34 PM, kostap@marvell.com wrote: >> From: Konstantin Porotchkin kostap@marvell.com >> >> The USB device should linked to VBUS regulator through "vbus-supply" >> DTS property. >> This patch adds handling for "vbus-supply" property inside the USB >> device entry for turning on the VBUS regulator upon the host adapter > probe. >> >> Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de >> Signed-off-by: Konstantin Porotchkin kostap@marvell.com >> Cc: Stefan Roese sr@denx.de >> Cc: Marek Vasut marex@denx.de >> Cc: Nadav Haklai nadavh@marvell.com >> Cc: Neta Zur Hershkovits neta@marvell.com >> Cc: Igal Liberman igall@marvell.com >> Cc: Haim Boot hayim@marvell.com >> --- >> Changes for v3: >> - Moved VBUS control from private GPIO to a fixed regulator >> - Rebase on top of master branch >> >> >> doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 > ++++++++++++++++++++ >> drivers/usb/host/xhci-mvebu.c | 31 > +++++++++++++++++++++++ >> 2 files changed, 59 insertions(+) >> create mode 100644 >> doc/device-tree-bindings/usb/marvell.xhci-usb.txt >> >> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt > b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >> new file mode 100644 >> index 0000000..672a829 >> --- /dev/null >> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >> @@ -0,0 +1,28 @@ >> +Marvell SOC USB controllers >> + >> +This controller is integrated in Armada 3700/8K. >> +It uses the same properties as a generic XHCI host controller >> + >> +Required properties : >> + - compatible: should be one or more of: >> + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs >> + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs >> + - reg: should contain address and length of the standard XHCI >> + register set for the device. >> + - interrupts: one XHCI interrupt should be described here. >> + >> +Optional properties: >> + - clocks: reference to a clock > > What clock ? Why are clock optional ? > This probably needs clock-names too. This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
I see what you saying. I will move the "clocks" entry to the "mandatory" section.
Why ? What clock are those ? Are they mandatory ?
They are not mandatory. This entry can be used for enabling gated clocks on a specific platform. However Kernel code does not handle missing clock entry as an error. It just assumes that the clock is not gated and therefore should not be enabled upon host controller probe. So maybe I got you wrong. My feeling was that you requested to make this entry mandatory.
Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry.
Then why are these clock here at all ?
Because this is an optional parameter and can be used for enabling gated clock if one is defined.
Should I simply remove this property from the text file?
See above.
I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file as a base for my add-on > >> + - vbus-supply : If present, specifies the fixed regulator to be > turned on >> + for providing power to the USB VBUS rail. >> + >> +Example: >> + cpm_usb3_0: usb3@500000 { >> + compatible = "marvell,armada-8k-xhci", >> + "generic-xhci"; >> + reg = <0x500000 0x4000>; >> + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&cpm_syscon0 1 22>; >> + vbus-supply = <®_usb3h0_vbus>; >> + status = "disabled"; >> + }; >> diff --git a/drivers/usb/host/xhci-mvebu.c > b/drivers/usb/host/xhci-mvebu.c >> index 46eb937..149f6a4 100644 >> --- a/drivers/usb/host/xhci-mvebu.c >> +++ b/drivers/usb/host/xhci-mvebu.c >> @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) >> struct mvebu_xhci *ctx = dev_get_priv(dev); >> struct xhci_hcor *hcor; >> int len; >> +#ifdef CONFIG_DM_REGULATOR_FIXED > > Just make the driver depend on REGULATOR_FIXED I think the driver can work without regulator if the VBUS rail wired to the 5V power line. We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
Got it. I will change the code for using the regulator framework in USB driver.
I tried your suggestion and it works for MACCHIATOBin board. However if I not surround this new code with "ifdef CONFIG_DM_REGULATOR_FIXED", the build for other boards based on same SoC will fail. So I have 2 possible solutions for this issue - make the mvebu_xhci driver depend on CONFIG_DM_REGULATOR_FIXED and add this configuration entry to defconfigs of all affected boards, or use the "ifdef" as in previous code. What is preferred?
Thanks :)
>> + const void *fdt = gd->fdt_blob; >> + int node = dev->of_offset; >> + const fdt32_t *regulator; >> + int size; >> >> + /* >> + * The VBUS supply regulator is not probed automatically >> + * Trigger the regulator probe upon USB port bring up >> + */ >> + regulator = fdt_getprop(fdt, node, "vbus-supply", &size);
I think this should use regulator_*() calls from include/power/regulator.h
I can call regulator_set_enable() at the end, but I have to locate it first and get the udev for it. However it will be enabled already at the time I will call this function.
regulator_get_by_platname("vbus-supply", ®); doesn't work here ?
Thank you for the tip! i will definitely try this. Unfortunately I am not yet fluent in all the available DM APIs.
That's what the review is for .
Yes, I understand. And thank you for the fast review! I wish all my patches get such attention :-)
>> + if (regulator) { >> + uint32_t phandle; >> + struct udevice *config; >> + int reg_node, ret; >> + >> + phandle = fdt32_to_cpu(*regulator); >> + reg_node = fdt_node_offset_by_phandle(fdt, phandle); >> + if (reg_node < 0) { >> + dev_err(dev, "vbus-supply has invalid phandle\n"); >> + return -EINVAL; >> + } >> + ret = uclass_get_device_by_of_offset(UCLASS_REGULATOR, >> + reg_node, &config); >> + if (ret) { >> + dev_err(dev, "failed to get VBUS regulator device\n"); >> + return ret; > > Where is the regulator enabled ? The regulator is fixed and it is "always-on", so assumed it is enough to probe it. I have found that once it probed, the USB device becomes powered.
And once someone attaches a different regulator than fixed, this will break :)
What other type of regulators can be used for powering on the USB port? I believe they are always 5V fixed, are they?
Anything which can turn 5V on/off , be it some i2c chip, spi chip, GPIO ...
On 02/09/2017 10:08 AM, Konstantin Porotchkin wrote:
On 02/09/2017 10:32 AM, Marek Vasut wrote:
On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote:
On 02/08/2017 06:42 PM, Marek Vasut wrote:
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote:
On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: > Hi, Marek, > > On 02/08/2017 05:35 PM, Marek Vasut wrote: >> On 02/08/2017 04:34 PM, kostap@marvell.com wrote: >>> From: Konstantin Porotchkin kostap@marvell.com >>> >>> The USB device should linked to VBUS regulator through >>> "vbus-supply" >>> DTS property. >>> This patch adds handling for "vbus-supply" property inside the USB >>> device entry for turning on the VBUS regulator upon the host >>> adapter >> probe. >>> >>> Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de >>> Signed-off-by: Konstantin Porotchkin kostap@marvell.com >>> Cc: Stefan Roese sr@denx.de >>> Cc: Marek Vasut marex@denx.de >>> Cc: Nadav Haklai nadavh@marvell.com >>> Cc: Neta Zur Hershkovits neta@marvell.com >>> Cc: Igal Liberman igall@marvell.com >>> Cc: Haim Boot hayim@marvell.com >>> --- >>> Changes for v3: >>> - Moved VBUS control from private GPIO to a fixed regulator >>> - Rebase on top of master branch >>> >>> >>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 >> ++++++++++++++++++++ >>> drivers/usb/host/xhci-mvebu.c | 31 >> +++++++++++++++++++++++ >>> 2 files changed, 59 insertions(+) >>> create mode 100644 >>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> >>> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >> b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> new file mode 100644 >>> index 0000000..672a829 >>> --- /dev/null >>> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> @@ -0,0 +1,28 @@ >>> +Marvell SOC USB controllers >>> + >>> +This controller is integrated in Armada 3700/8K. >>> +It uses the same properties as a generic XHCI host controller >>> + >>> +Required properties : >>> + - compatible: should be one or more of: >>> + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx >>> SoCs >>> + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs >>> + - reg: should contain address and length of the standard XHCI >>> + register set for the device. >>> + - interrupts: one XHCI interrupt should be described here. >>> + >>> +Optional properties: >>> + - clocks: reference to a clock >> >> What clock ? Why are clock optional ? >> This probably needs clock-names too. > This is the way it defined in Linux Kernel.
Then it probably could use fixing there too. Like seriously, what clock are those ? And why are they optional ? If neither you or me understand that from the documentation, then the documentation is crap and needs fixing. It being the same way in Linux is not an argument for sticking to it.
From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
I see what you saying. I will move the "clocks" entry to the "mandatory" section.
Why ? What clock are those ? Are they mandatory ?
They are not mandatory. This entry can be used for enabling gated clocks on a specific platform. However Kernel code does not handle missing clock entry as an error. It just assumes that the clock is not gated and therefore should not be enabled upon host controller probe. So maybe I got you wrong. My feeling was that you requested to make this entry mandatory.
I have no idea what close those are (based on the description), so I cannot decide either way. If this is something which is present only on selected platforms, then they are optional indeed.
Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry.
Then why are these clock here at all ?
Because this is an optional parameter and can be used for enabling gated clock if one is defined.
So these are different clock from the SERDES clock, yes ?
Should I simply remove this property from the text file?
See above.
> I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file > as a > base for my add-on >> >>> + - vbus-supply : If present, specifies the fixed regulator to be >> turned on >>> + for providing power to the USB VBUS rail. >>> + >>> +Example: >>> + cpm_usb3_0: usb3@500000 { >>> + compatible = "marvell,armada-8k-xhci", >>> + "generic-xhci"; >>> + reg = <0x500000 0x4000>; >>> + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; >>> + clocks = <&cpm_syscon0 1 22>; >>> + vbus-supply = <®_usb3h0_vbus>; >>> + status = "disabled"; >>> + }; >>> diff --git a/drivers/usb/host/xhci-mvebu.c >> b/drivers/usb/host/xhci-mvebu.c >>> index 46eb937..149f6a4 100644 >>> --- a/drivers/usb/host/xhci-mvebu.c >>> +++ b/drivers/usb/host/xhci-mvebu.c >>> @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) >>> struct mvebu_xhci *ctx = dev_get_priv(dev); >>> struct xhci_hcor *hcor; >>> int len; >>> +#ifdef CONFIG_DM_REGULATOR_FIXED >> >> Just make the driver depend on REGULATOR_FIXED > I think the driver can work without regulator if the VBUS rail > wired to > the 5V power line. > We only need regulator support if this is GPIO controlled
In that case, the regulator won't be found and the driver will ignore it. Btw I think that violates the USB spec ;-)
Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
Got it. I will change the code for using the regulator framework in USB driver.
I tried your suggestion and it works for MACCHIATOBin board. However if I not surround this new code with "ifdef CONFIG_DM_REGULATOR_FIXED", the build for other boards based on same SoC will fail. So I have 2 possible solutions for this issue - make the mvebu_xhci driver depend on CONFIG_DM_REGULATOR_FIXED and add this configuration entry to defconfigs of all affected boards, or use the "ifdef" as in previous code. What is preferred?
Probably just enabling the regulator framework should be enough ?
[...]
On 02/09/2017 11:09 AM, Marek Vasut wrote:
On 02/09/2017 10:08 AM, Konstantin Porotchkin wrote:
On 02/09/2017 10:32 AM, Marek Vasut wrote:
On 02/09/2017 09:00 AM, Konstantin Porotchkin wrote:
On 02/08/2017 06:42 PM, Marek Vasut wrote:
On 02/08/2017 05:28 PM, Konstantin Porotchkin wrote:
Hi, Marek,
On 02/08/2017 06:04 PM, Marek Vasut wrote: > On 02/08/2017 04:45 PM, Konstantin Porotchkin wrote: >> Hi, Marek, >> >> On 02/08/2017 05:35 PM, Marek Vasut wrote: >>> On 02/08/2017 04:34 PM, kostap@marvell.com wrote: >>>> From: Konstantin Porotchkin kostap@marvell.com >>>> >>>> The USB device should linked to VBUS regulator through >>>> "vbus-supply" >>>> DTS property. >>>> This patch adds handling for "vbus-supply" property inside the USB >>>> device entry for turning on the VBUS regulator upon the host >>>> adapter >>> probe. >>>> >>>> Change-Id: Ibcf72d82298be42353ca03fee064ae8077a7b9de >>>> Signed-off-by: Konstantin Porotchkin kostap@marvell.com >>>> Cc: Stefan Roese sr@denx.de >>>> Cc: Marek Vasut marex@denx.de >>>> Cc: Nadav Haklai nadavh@marvell.com >>>> Cc: Neta Zur Hershkovits neta@marvell.com >>>> Cc: Igal Liberman igall@marvell.com >>>> Cc: Haim Boot hayim@marvell.com >>>> --- >>>> Changes for v3: >>>> - Moved VBUS control from private GPIO to a fixed regulator >>>> - Rebase on top of master branch >>>> >>>> >>>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 28 >>> ++++++++++++++++++++ >>>> drivers/usb/host/xhci-mvebu.c | 31 >>> +++++++++++++++++++++++ >>>> 2 files changed, 59 insertions(+) >>>> create mode 100644 >>>> doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>> >>>> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>> b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>> new file mode 100644 >>>> index 0000000..672a829 >>>> --- /dev/null >>>> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt >>>> @@ -0,0 +1,28 @@ >>>> +Marvell SOC USB controllers >>>> + >>>> +This controller is integrated in Armada 3700/8K. >>>> +It uses the same properties as a generic XHCI host controller >>>> + >>>> +Required properties : >>>> + - compatible: should be one or more of: >>>> + - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx >>>> SoCs >>>> + - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs >>>> + - reg: should contain address and length of the standard XHCI >>>> + register set for the device. >>>> + - interrupts: one XHCI interrupt should be described here. >>>> + >>>> +Optional properties: >>>> + - clocks: reference to a clock >>> >>> What clock ? Why are clock optional ? >>> This probably needs clock-names too. >> This is the way it defined in Linux Kernel. > > Then it probably could use fixing there too. Like seriously, what > clock > are those ? And why are they optional ? If neither you or me > understand > that from the documentation, then the documentation is crap and needs > fixing. It being the same way in Linux is not an argument for > sticking > to it. From my point of view this clock entry is optional too. I am not handling it in any way and the core XHCI driver doesn't uses it.
DT is describing the hardware, not the software that is using it. These two things are separate. If the clock are mandatory for the hardware to work, they must be mandatory in the DT.
I see what you saying. I will move the "clocks" entry to the "mandatory" section.
Why ? What clock are those ? Are they mandatory ?
They are not mandatory. This entry can be used for enabling gated clocks on a specific platform. However Kernel code does not handle missing clock entry as an error. It just assumes that the clock is not gated and therefore should not be enabled upon host controller probe. So maybe I got you wrong. My feeling was that you requested to make this entry mandatory.
I have no idea what close those are (based on the description), so I cannot decide either way. If this is something which is present only on selected platforms, then they are optional indeed.
Please keep in mind that it will not be currently enforced by the SW. For USB 3.0 case the clock parameters are actually defined by SERDES configuration, which has a separate DTS entry.
Then why are these clock here at all ?
Because this is an optional parameter and can be used for enabling gated clock if one is defined.
So these are different clock from the SERDES clock, yes ?
As far as I know the SERDES entry defines the clock speed, which affects the initialization flow including the clock dividers. The clock in USb entry looks like for gating only.
Should I simply remove this property from the text file?
See above.
>> I took the Documentation/devicetree/bindings/usb/usb-xhci.txt file >> as a >> base for my add-on >>> >>>> + - vbus-supply : If present, specifies the fixed regulator to be >>> turned on >>>> + for providing power to the USB VBUS rail. >>>> + >>>> +Example: >>>> + cpm_usb3_0: usb3@500000 { >>>> + compatible = "marvell,armada-8k-xhci", >>>> + "generic-xhci"; >>>> + reg = <0x500000 0x4000>; >>>> + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; >>>> + clocks = <&cpm_syscon0 1 22>; >>>> + vbus-supply = <®_usb3h0_vbus>; >>>> + status = "disabled"; >>>> + }; >>>> diff --git a/drivers/usb/host/xhci-mvebu.c >>> b/drivers/usb/host/xhci-mvebu.c >>>> index 46eb937..149f6a4 100644 >>>> --- a/drivers/usb/host/xhci-mvebu.c >>>> +++ b/drivers/usb/host/xhci-mvebu.c >>>> @@ -45,7 +45,38 @@ static int xhci_usb_probe(struct udevice *dev) >>>> struct mvebu_xhci *ctx = dev_get_priv(dev); >>>> struct xhci_hcor *hcor; >>>> int len; >>>> +#ifdef CONFIG_DM_REGULATOR_FIXED >>> >>> Just make the driver depend on REGULATOR_FIXED >> I think the driver can work without regulator if the VBUS rail >> wired to >> the 5V power line. >> We only need regulator support if this is GPIO controlled > > In that case, the regulator won't be found and the driver will ignore > it. Btw I think that violates the USB spec ;-) > Currently the armada-8040-DB/armada-7040-DB boards use i2c connected VBUS enable control. If I made this driver depend on REGULATOR_FIXED, it will not work for these boards. They do not have regulator support enabled so far. I do not want to break another systems by adding support for this board.
Oh, right. Then I believe using the regulator framework will help. The driver can depend on the regulator framework, which will abstract away the used regulator.
Got it. I will change the code for using the regulator framework in USB driver.
I tried your suggestion and it works for MACCHIATOBin board. However if I not surround this new code with "ifdef CONFIG_DM_REGULATOR_FIXED", the build for other boards based on same SoC will fail. So I have 2 possible solutions for this issue - make the mvebu_xhci driver depend on CONFIG_DM_REGULATOR_FIXED and add this configuration entry to defconfigs of all affected boards, or use the "ifdef" as in previous code. What is preferred?
Probably just enabling the regulator framework should be enough ?
[...]
From: Rabeeh Khoury rabeeh@solid-run.com
Added A8040 dts file for community board MACCHIATIBin. The patch includes the following features: AP - Serial console (connected to onboard FTDI usb to serial) CP0 - PCIe x4, SATA, I2C and 10G KR (connected to Marvell 3310 10G copper / SFP+ phy) CP1 - Boot SPI, USB3 host, 2xSATA, 10G KR (connected to Marvell 3310 10G copper / SFP+ phy), SGMII connected to onboard 1512 1Gbps copper phy, and additional SGMII connected to SFP (default 1Gbps can be configured to 2.5Gbps).
Network interface naming - egiga0 - CP0 KR egiga1 - CP1 KR egiga2 - CP1 RJ45 1Gbps connector (recommended for TFTP boot) egiga3 - CP1 SFP default 1Gbps and can be modified to 2.5Gbps
Signed-off-by: Konstantin Porotchkin kostap@marvell.com Signed-off-by: Rabeeh Khoury rabeeh@solid-run.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Add SD and eMMC entries - Rebase on top of master branch
arch/arm/dts/Makefile | 1 + arch/arm/dts/armada-8040-mcbin.dts | 287 +++++++++++++++++++++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100644 arch/arm/dts/armada-8040-mcbin.dts
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 397a0ae..a3b139b 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -76,6 +76,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \ armada-385-amc.dtb \ armada-7040-db.dtb \ armada-8040-db.dtb \ + armada-8040-mcbin.dtb \ armada-xp-gp.dtb \ armada-xp-maxbcm.dtb \ armada-xp-synology-ds414.dtb \ diff --git a/arch/arm/dts/armada-8040-mcbin.dts b/arch/arm/dts/armada-8040-mcbin.dts new file mode 100644 index 0000000..15da53d --- /dev/null +++ b/arch/arm/dts/armada-8040-mcbin.dts @@ -0,0 +1,287 @@ +/* + * Copyright (C) 2016 Marvell International Ltd. + * + * SPDX-License-Identifier: GPL-2.0 + * https://spdx.org/licenses + */ + +#include "armada-8040.dtsi" /* include SoC device tree */ + +/ { + model = "MACCHIATOBin-8040"; + compatible = "marvell,armada8040-mcbin", + "marvell,armada8040"; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + aliases { + i2c0 = &cpm_i2c0; + i2c1 = &cpm_i2c1; + spi0 = &cps_spi1; + gpio0 = &ap_gpio0; + gpio1 = &cpm_gpio0; + gpio2 = &cpm_gpio1; + }; + + memory@00000000 { + device_type = "memory"; + reg = <0x0 0x0 0x0 0x80000000>; + }; + +reg_usb3h0_vbus: usb3-vbus0 { + compatible = "regulator-fixed"; + pinctrl-names = "default"; + pinctrl-0 = <&cpm_xhci_vbus_pins>; + regulator-name = "reg-usb3h0-vbus"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + startup-delay-us = <500000>; + enable-active-high; + regulator-always-on; + regulator-boot-on; + gpio = <&cpm_gpio1 15 GPIO_ACTIVE_HIGH>; /* GPIO[47] */ + }; +}; + +/* Accessible over the mini-USB CON9 connector on the main board */ +&uart0 { + status = "okay"; +}; + +&ap_pinctl { + /* + * MPP Bus: + * eMMC [0-10] + * UART0 [11,19] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 1 1 1 1 1 1 1 1 1 1 + 1 3 0 0 0 0 0 0 0 3 >; +}; + +/* on-board eMMC */ +&ap_sdhci0 { + pinctrl-names = "default"; + pinctrl-0 = <&ap_emmc_pins>; + bus-width= <8>; + status = "okay"; +}; + +&cpm_pinctl { + /* + * MPP Bus: + * [0-31] = 0xff: Keep default CP0_shared_pins: + * [11] CLKOUT_MPP_11 (out) + * [23] LINK_RD_IN_CP2CP (in) + * [25] CLKOUT_MPP_25 (out) + * [29] AVS_FB_IN_CP2CP (in) + * [32,34] SMI + * [33] MSS power down + * [35-38] CP0 I2C1 and I2C0 + * [39] MSS CKE Enable + * [40,41] CP0 UART1 TX/RX + * [42,43] XSMI (controls two 10G phys) + * [47] USB VBUS EN + * [48] FAN PWM + * [49] 10G port 1 interrupt + * [50] 10G port 0 interrupt + * [51] 2.5G SFP TX fault + * [52] PCIe reset out + * [53] 2.5G SFP mode + * [54] 2.5G SFP LOS + * [55] Micro SD card detect + * [56-61] Micro SD + * [62] CP1 KR SFP FAULT + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0 7 0xa 7 2 2 2 2 0xa + 7 7 8 8 0 0 0 0 0 0 + 0 0 0 0 0 0 0xe 0xe 0xe 0xe + 0xe 0xe 0 >; + + cpm_xhci_vbus_pins: cpm-xhci-vbus-pins { + marvell,pins = < 47 >; + marvell,function = <0>; + }; + + cpm_pcie_reset_pins: cpm-pcie-reset-pins { + marvell,pins = < 52 >; + marvell,function = <0>; + }; +}; + +/* uSD slot */ +&cpm_sdhci0 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_sdhci_pins>; + bus-width= <4>; + status = "okay"; +}; + +/* PCIe x4 */ +&cpm_pcie0 { + num-lanes = <4>; + pinctrl-names = "default"; + pinctrl-0 = <&cpm_pcie_reset_pins>; + marvell,reset-gpio = <&cpm_gpio1 20 GPIO_ACTIVE_HIGH>; /* GPIO[52] */ + status = "okay"; +}; + +&cpm_i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_i2c0_pins>; + status = "okay"; + clock-frequency = <100000>; +}; + +&cpm_i2c1 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_i2c1_pins>; + status = "okay"; + clock-frequency = <100000>; +}; + +&cpm_sata0 { + status = "okay"; +}; + +&cpm_comphy { + /* + * CP0 Serdes Configuration: + * Lane 0: PCIe0 (x4) + * Lane 1: PCIe0 (x4) + * Lane 2: PCIe0 (x4) + * Lane 3: PCIe0 (x4) + * Lane 4: KR (10G) + * Lane 5: SATA1 + */ + phy0 { + phy-type = <PHY_TYPE_PEX0>; + }; + phy1 { + phy-type = <PHY_TYPE_PEX0>; + }; + phy2 { + phy-type = <PHY_TYPE_PEX0>; + }; + phy3 { + phy-type = <PHY_TYPE_PEX0>; + }; + phy4 { + phy-type = <PHY_TYPE_KR>; + }; + phy5 { + phy-type = <PHY_TYPE_SATA1>; + }; +}; + +&cps_sata0 { + status = "okay"; +}; + +&cps_usb3_0 { + vbus-supply = <®_usb3h0_vbus>; + status = "okay"; +}; + +&cps_utmi0 { + status = "okay"; +}; + +&cps_pinctl { + /* + * MPP Bus: + * [0-5] TDM + * [6,7] CP1_UART 0 + * [8] CP1 10G SFP LOS + * [9] CP1 10G PHY RESET + * [10] CP1 10G SFP TX Disable + * [11] CP1 10G SFP Mode + * [12] SPI1 CS1n + * [13] SPI1 MISO (TDM and SPI ROM shared) + * [14] SPI1 CS0n + * [15] SPI1 MOSI (TDM and SPI ROM shared) + * [16] SPI1 CLK (TDM and SPI ROM shared) + * [24] CP1 2.5G SFP TX Disable + * [26] CP0 10G SFP TX Fault + * [27] CP0 10G SFP Mode + * [28] CP0 10G SFP LOS + * [29] CP0 10G SFP TX Disable + * [30] USB Over current indication + * [31] 10G Port 0 phy reset + * [32-62] = 0xff: Keep default CP1_shared_pins: + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 0x4 0x4 0x4 0x4 0x4 0x4 0x8 0x8 0x0 0x0 + 0x0 0x0 0x3 0x3 0x3 0x3 0x3 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0x0 0xff 0x0 0x0 0x0 0x0 + 0x0 0x0 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff>; +}; + +&cps_spi1 { + pinctrl-names = "default"; + pinctrl-0 = <&cps_spi1_pins>; + status = "okay"; + + spi-flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <10000000>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "U-Boot"; + reg = <0 0x200000>; + }; + partition@400000 { + label = "Filesystem"; + reg = <0x200000 0xce0000>; + }; + }; + }; +}; + +&cps_comphy { + /* + * CP1 Serdes Configuration: + * Lane 0: SGMII2 + * Lane 1: SATA 0 + * Lane 2: USB HOST 0 + * Lane 3: SATA1 + * Lane 4: KR (10G) + * Lane 5: SGMII3 + */ + phy0 { + phy-type = <PHY_TYPE_SGMII2>; + phy-speed = <PHY_SPEED_1_25G>; + }; + phy1 { + phy-type = <PHY_TYPE_SATA0>; + }; + phy2 { + phy-type = <PHY_TYPE_USB3_HOST0>; + }; + phy3 { + phy-type = <PHY_TYPE_SATA1>; + }; + phy4 { + phy-type = <PHY_TYPE_KR>; + }; + phy5 { + phy-type = <PHY_TYPE_SGMII3>; + }; +};
From: Konstantin Porotchkin kostap@marvell.com
Add default configuration for MACHHIATOBin community board based on Aramda-8040 SoC. This config has MMC support removed, pending mainline support.
Change-Id: Ic6b562065c0929ec338492452f765115c15a6188 Signed-off-by: Konstantin Porotchkin kostap@marvell.com Cc: Stefan Roese sr@denx.de Cc: Nadav Haklai nadavh@marvell.com Cc: Neta Zur Hershkovits neta@marvell.com Cc: Igal Liberman igall@marvell.com Cc: Haim Boot hayim@marvell.com --- Changes for v3: - Align the configuration with Armada-8040-DB - Rebase on top of master branch
configs/mvebu_mcbin-88f8040_defconfig | 65 +++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 configs/mvebu_mcbin-88f8040_defconfig
diff --git a/configs/mvebu_mcbin-88f8040_defconfig b/configs/mvebu_mcbin-88f8040_defconfig new file mode 100644 index 0000000..000e400 --- /dev/null +++ b/configs/mvebu_mcbin-88f8040_defconfig @@ -0,0 +1,65 @@ +CONFIG_ARM=y +CONFIG_ARCH_MVEBU=y +CONFIG_SYS_MALLOC_F_LEN=0x2000 +CONFIG_TARGET_MVEBU_ARMADA_8K=y +CONFIG_DEFAULT_DEVICE_TREE="armada-8040-mcbin" +CONFIG_SMBIOS_PRODUCT_NAME="" +CONFIG_AHCI=y +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_SYS_CONSOLE_INFO_QUIET=y +# CONFIG_DISPLAY_CPUINFO is not set +# CONFIG_DISPLAY_BOARDINFO is not set +CONFIG_HUSH_PARSER=y +# CONFIG_CMD_IMLS is not set +# CONFIG_CMD_FLASH is not set +CONFIG_CMD_SF=y +CONFIG_CMD_SPI=y +CONFIG_CMD_I2C=y +CONFIG_CMD_USB=y +# CONFIG_CMD_FPGA is not set +# CONFIG_CMD_SETEXPR is not set +CONFIG_CMD_TFTPPUT=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_MII=y +CONFIG_CMD_PING=y +CONFIG_CMD_CACHE=y +CONFIG_CMD_TIME=y +CONFIG_CMD_MVEBU_BUBT=y +CONFIG_CMD_EXT4=y +CONFIG_CMD_EXT4_WRITE=y +CONFIG_CMD_FAT=y +CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_GPIO=y +CONFIG_CMD_REGULATOR=y +CONFIG_BLOCK_CACHE=y +CONFIG_DM_I2C=y +CONFIG_SYS_I2C_MVTWSI=y +CONFIG_DM_GPIO=y +CONFIG_MVEBU_GPIO=y +CONFIG_DM_REGULATOR=y +CONFIG_DM_REGULATOR_FIXED=y +CONFIG_MISC=y +CONFIG_SPI_FLASH=y +CONFIG_SPI_FLASH_MACRONIX=y +CONFIG_SPI_FLASH_SPANSION=y +CONFIG_SPI_FLASH_STMICRO=y +CONFIG_SPI_FLASH_WINBOND=y +CONFIG_PHYLIB=y +CONFIG_PCI=y +CONFIG_DM_PCI=y +CONFIG_PCIE_DW_MVEBU=y +CONFIG_MVEBU_COMPHY_SUPPORT=y +CONFIG_PINCTRL=y +# CONFIG_SPL_SERIAL_PRESENT is not set +CONFIG_DEBUG_UART=y +CONFIG_DEBUG_UART_BASE=0xf0512000 +CONFIG_DEBUG_UART_CLOCK=200000000 +CONFIG_DEBUG_UART_SHIFT=2 +CONFIG_DEBUG_UART_ANNOUNCE=y +CONFIG_SYS_NS16550=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_EHCI_HCD=y +CONFIG_USB_STORAGE=y +CONFIG_SMBIOS_MANUFACTURER=""
participants (4)
-
Konstantin Porotchkin -
kostap＠marvell.com
-
Marek Vasut -
Stefan Roese