add imx usb driver for mx28/6x

- to decrease redundant since both ehci_hcd and ohci_hcd have the same variable
- it helps access phy in usb core code
- phy is more meaningful than transceiver

Signed-off-by Richard Zhao <[email protected]>

Phy may need to change settings when port connect change.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/core/hub.c |    8 ++++++++
 1 file changed, 8 insertions(+)

drivers/usb/core/hub.c +1 -0

index 04fb834..e1a6d31 100644
@@ -20,6 +20,7 @@
 #include <linux/usb.h>
 #include <linux/usbdevice_fs.h>
 #include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
 #include <linux/usb/quirks.h>
 #include <linux/kthread.h>
 #include <linux/mutex.h>

Quoted 23 lines · Richard Zhao

Phy may need to change settings when port connect change.

Signed-off-by Richard Zhao <[email protected]>

---
 drivers/usb/core/hub.c |    8 ++++++++
 1 file changed, 8 insertions(+)

drivers/usb/core/hub.c +1 -0

index 04fb834..e1a6d31 100644
@@ -20,6 +20,7 @@
 #include <linux/usb.h>
 #include <linux/usbdevice_fs.h>
 #include <linux/usb/hcd.h>
+#include <linux/usb/otg.h>
 #include <linux/usb/quirks.h>
 #include <linux/kthread.h>
 #include <linux/mutex.h>

@@ -4034,6 +4035,13 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
 		}
 	}
 

drivers/usb/core/hub.c static void hub_port_connect_change(struct usb_hub *hub, int port1, · line 4038

Quoted 1 line

drivers/usb/core/hub.c +1 -0

+	if (unlikely(hcd->phy && !hdev->level)) {

This is okay, but it's more common to test for root hubs with "!hdev->parent".

drivers/usb/core/hub.c static void hub_port_connect_change(struct usb_hub *hub, int port1, · line 4047

Quoted 9 lines

drivers/usb/core/hub.c +6 -0

+		if (portstatus & USB_PORT_STAT_CONNECTION)
+			usb_phy_notify_connect(hcd->phy, port1);
+		else
+			usb_phy_notify_disconnect(hcd->phy, port1);
+	}
+
 	/* Return now if debouncing failed or nothing is connected or
 	 * the device was "removed".
 	 */

Alan Stern

Quoted 4 lines

drivers/usb/core/hub.c static void hub_port_connect_change(struct usb_hub *hub, int port1, · line 4038

Quoted 1 line

drivers/usb/core/hub.c +1 -0

+	if (unlikely(hcd->phy && !hdev->level)) {

This is okay, but it's more common to test for root hubs with "!hdev->parent".

So it will be like:
if (unlikely(hcd->phy && !hdev->parent)) {

And,
I didn't check (portchange & USB_PORT_STAT_C_CONNECTION), because if the device is connected on boot, there'll be no USB_PORT_STAT_C_CONNECTION. Is it acceptable?

mxs phy is used in Freescale i.MX SoCs, for example imx23, imx28, imx6Q. This patch adds the basic host support.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Acked-by Felipe Balbi <[email protected]>

---
 Documentation/devicetree/bindings/usb/mxs-phy.txt |   13 ++
 drivers/usb/otg/Kconfig                           |    8 +
 drivers/usb/otg/Makefile                          |    1 +
 drivers/usb/otg/mxs-phy.c                         |  204 +++++++++++++++++++++
 4 files changed, 226 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/mxs-phy.txt
 create mode 100644 drivers/usb/otg/mxs-phy.c

Documentation/devicetree/bindings/usb/mxs-phy.txt new file +13 -0

new file mode 100644
index 0000000..5835b27
@@ -0,0 +1,13 @@
+* Freescale MXS USB Phy Device
+
+Required properties:
+- compatible: Should be "fsl,imx23-usbphy"
+- reg: Should contain registers location and length
+- interrupts: Should contain phy interrupt
+
+Example:
+usbphy1: usbphy at 020c9000 {
+	compatible = "fsl,imx6q-usbphy", "fsl,imx23-usbphy";
+	reg = <0x020c9000 0x1000>;
+	interrupts = <0 44 0x04>;
+};

drivers/usb/otg/Kconfig +8 -0

index 5c87db0..3a9bc44 100644
@@ -116,6 +116,14 @@ config FSL_USB2_OTG
 	help
 	  Enable this to support Freescale USB OTG transceiver.
 
+config USB_MXS_PHY
+	tristate "Freescale MXS USB PHY support"
+	select USB_OTG_UTILS
+	help
+	  Enable this to support the Freescale MXS USB PHY.
+
+	  MXS Phy is used by some of the i.MX SoCs, for example imx23/28/6x.
+
 config USB_MV_OTG
 	tristate "Marvell USB OTG support"
 	depends on USB_EHCI_MV && USB_MV_UDC && USB_SUSPEND

drivers/usb/otg/Makefile +1 -0

index 41aa509..a844b8d 100644
@@ -20,4 +20,5 @@ obj-$(CONFIG_USB_MSM_OTG)	+= msm_otg.o
 obj-$(CONFIG_AB8500_USB)	+= ab8500-usb.o
 fsl_usb2_otg-objs		:= fsl_otg.o otg_fsm.o
 obj-$(CONFIG_FSL_USB2_OTG)	+= fsl_usb2_otg.o
+obj-$(CONFIG_USB_MXS_PHY)	+= mxs-phy.o
 obj-$(CONFIG_USB_MV_OTG)	+= mv_otg.o

drivers/usb/otg/mxs-phy.c new file +47 -0

new file mode 100644
index 0000000..a55aaed
@@ -0,0 +1,204 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Marek Vasut <[email protected]>
+ * on behalf of DENX Software Engineering GmbH
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/usb/otg.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+
+#define DRIVER_NAME "mxs_phy"
+
+#define HW_USBPHY_PWD				0x00
+#define HW_USBPHY_CTRL				0x30
+#define HW_USBPHY_CTRL_SET			0x34
+#define HW_USBPHY_CTRL_CLR			0x38
+
+#define BM_USBPHY_CTRL_SFTRST			BIT(31)
+#define BM_USBPHY_CTRL_CLKGATE			BIT(30)
+#define BM_USBPHY_CTRL_ENUTMILEVEL3		BIT(15)
+#define BM_USBPHY_CTRL_ENUTMILEVEL2		BIT(14)
+#define BM_USBPHY_CTRL_ENHOSTDISCONDETECT	BIT(1)
+
+struct mxs_phy {
+	struct usb_phy phy;
+	struct clk *clk;
+	int connected:1;
+};
+
+#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
+
+static void mxs_phy_hw_init(struct mxs_phy *mxs_phy)
+{
+	void __iomem *base = mxs_phy->phy.io_priv;
+

Quoted 8 lines

mxs phy is used in Freescale i.MX SoCs, for example imx23, imx28, imx6Q. This patch adds the basic host support.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Acked-by Felipe Balbi <[email protected]>

drivers/usb/otg/mxs-phy.c line 57

Quoted 4 lines

drivers/usb/otg/mxs-phy.c new file +4 -0

+	/* Remove CLKGATE and SFTRST */
+	writel_relaxed(BM_USBPHY_CTRL_CLKGATE | BM_USBPHY_CTRL_SFTRST,
+			base + HW_USBPHY_CTRL_CLR);
+	udelay(10);

Is stmp_reset_block() suitable for what you want to do here?

drivers/usb/otg/mxs-phy.c line 139

Quoted 7 lines

drivers/usb/otg/mxs-phy.c new file +7 -0

+	base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!base)
+		return -EBUSY;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "can't get the clock!");

Please add the return value to these kind of messages.

drivers/usb/otg/mxs-phy.c line 145

Quoted 6 lines

drivers/usb/otg/mxs-phy.c new file +6 -0

+		return PTR_ERR(clk);
+	}
+
+	mxs_phy = devm_kzalloc(&pdev->dev, sizeof(*mxs_phy), GFP_KERNEL);
+	if (!mxs_phy) {
+		dev_err(&pdev->dev, "Failed to allocate USB PHY structure!\n");

This message is rather useless. It is really not expected for kzalloc to fail. If it fails here you really have problems elsewhere and this message won't help you debugging it.

Sascha

Dear Sascha Hauer,

Quoted 4 lines

This message is rather useless. It is really not expected for kzalloc to fail. If it fails here you really have problems elsewhere and this message won't help you debugging it.

It's not useless, but it'll hardly ever be displayed if you run out of memory so badly this kzalloc() will fail.

Dear Sascha Hauer,

Quoted 9 lines · Richard Zhao

Quoted 8 lines

mxs phy is used in Freescale i.MX SoCs, for example imx23, imx28, imx6Q. This patch adds the basic host support.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Acked-by Felipe Balbi <[email protected]>

[...]

Quoted 6 lines

drivers/usb/otg/mxs-phy.c line 57

Quoted 4 lines

drivers/usb/otg/mxs-phy.c new file +4 -0

+	/* Remove CLKGATE and SFTRST */
+	writel_relaxed(BM_USBPHY_CTRL_CLKGATE | BM_USBPHY_CTRL_SFTRST,
+			base + HW_USBPHY_CTRL_CLR);
+	udelay(10);

Is stmp_reset_block() suitable for what you want to do here?

IIRC it is.

Quoted 10 lines

drivers/usb/otg/mxs-phy.c line 139

Quoted 8 lines

drivers/usb/otg/mxs-phy.c new file +7 -0

+	base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!base)
+		return -EBUSY;
+
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "can't get the clock!");

Please add the return value to these kind of messages.

Quoted 11 lines

drivers/usb/otg/mxs-phy.c line 145

Quoted 7 lines

drivers/usb/otg/mxs-phy.c new file +6 -0

+		return PTR_ERR(clk);
+	}
+
+	mxs_phy = devm_kzalloc(&pdev->dev, sizeof(*mxs_phy), GFP_KERNEL);
+	if (!mxs_phy) {
+		dev_err(&pdev->dev, "Failed to allocate USB PHY structure!\n");

This message is rather useless. It is really not expected for kzalloc to fail. If it fails here you really have problems elsewhere and this message won't help you debugging it.

It's not useless, but it'll hardly ever be displayed if you run out of memory so badly this kzalloc() will fail.

Quoted 1 line

Sascha

Best regards,

This patch supports only the host-mode functionality so far.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
 .../devicetree/bindings/usb/ci13xxx-imx.txt        |   20 +++
 drivers/usb/chipidea/Makefile                      |    3 +
 drivers/usb/chipidea/ci13xxx_imx.c                 |  189 ++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt new file +5 -0

new file mode 100644
index 0000000..beb75d6
@@ -0,0 +1,20 @@
+* Freescale i.MX ci13xxx usb controllers
+
+Required properties:
+- compatible: Should be "fsl,imx31-usb"
+- reg: Should contain registers location and length

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt new file +5 -0

+	reg = <0x02184000 0x200>;
+	interrupts = <0 43 0x04>;
+	fsl,usbphy = <&usbphy1>;
+	fsl,vbus-power-gpios = <&gpio3 22 0>;
+};

drivers/usb/chipidea/Makefile +6 -0

index b69900a..5c66d9c 100644
@@ -14,3 +14,6 @@ ifneq ($(CONFIG_PCI),)
 	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_pci.o
 endif
 
+ifneq ($(CONFIG_OF_DEVICE),)
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+endif
+
+static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
+{

drivers/usb/chipidea/Makefile +6 -0

+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Can't get device resources!\n");

drivers/usb/chipidea/Makefile +12 -0

+			struct usb_phy *phy;
+			phy = pdev_to_phy(phy_pdev);
+			if (phy &&
+			    try_module_get(phy_pdev->dev.driver->owner)) {
+				usb_phy_init(phy);
+				data->phy = phy;
+			}
+		}
+	}
+
+	/* we only support host now, so enable vbus here */
+	reg_vbus = devm_regulator_get(&pdev->dev, "vbus");

drivers/usb/chipidea/Makefile +4 -0

+
+	ci13xxx_imx_platdata.phy = data->phy;
+
+	if (!pdev->dev.dma_mask) {

Quoted 17 lines

This patch supports only the host-mode functionality so far.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---

 .../devicetree/bindings/usb/ci13xxx-imx.txt        |   20 +++
 drivers/usb/chipidea/Makefile                      |    3 +
 drivers/usb/chipidea/ci13xxx_imx.c                 |  189
 ++++++++++++++++++++ 3 files changed, 212 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c

Quoted 8 lines

drivers/usb/chipidea/ci13xxx_imx.c new file +8 -0

+
+	ci13xxx_imx_platdata.phy = data->phy;
+
+	if (!pdev->dev.dma_mask) {
+		pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+				      sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+		if (!pdev->dev.dma_mask) {
+			ret = -ENOMEM;

drivers/usb/chipidea/ci13xxx_imx.c line 118

Quoted 6 lines

drivers/usb/chipidea/ci13xxx_imx.c new file +6 -0

+			dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
+			goto err;
+		}
+		*pdev->dev.dma_mask = DMA_BIT_MASK(32);
+		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
+	}

Do you need this? I assume not.

Sascha

Dear Sascha Hauer,

Quoted 6 lines

Quoted 6 lines

drivers/usb/chipidea/ci13xxx_imx.c new file +6 -0

+
+	if (!pdev->dev.dma_mask) {
+		pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+				      sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+		if (!pdev->dev.dma_mask) {
+			ret = -ENOMEM;

Quoted 8 lines

drivers/usb/chipidea/ci13xxx_imx.c line 118

Quoted 6 lines

drivers/usb/chipidea/ci13xxx_imx.c new file +6 -0

+			dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
+			goto err;
+		}
+		*pdev->dev.dma_mask = DMA_BIT_MASK(32);
+		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
+	}

Do you need this? I assume not.

Ain't this the dma mask crap without which the usb stuff won't work?

Quoted 17 lines · Marek Vasut

Dear Sascha Hauer,

Quoted 14 lines

drivers/usb/chipidea/ci13xxx_imx.c line 118

Quoted 12 lines

drivers/usb/chipidea/ci13xxx_imx.c new file +12 -0

+
+	if (!pdev->dev.dma_mask) {
+		pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+				      sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+		if (!pdev->dev.dma_mask) {
+			ret = -ENOMEM;
+			dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
+			goto err;
+		}
+		*pdev->dev.dma_mask = DMA_BIT_MASK(32);
+		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
+	}

Do you need this? I assume not.

Quoted 1 line

Ain't this the dma mask crap without which the usb stuff won't work?

Yes, the device needs this, but afaik the of platform code will set this for you. You only have to do this manually for devices which you allocate yourself.

Sascha

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Quoted 32 lines · Richard Zhao

This patch supports only the host-mode functionality so far.

Signed-off-by Richard Zhao <[email protected]>

Signed-off-by Marek Vasut <[email protected]>

Cc Peter Chen <[email protected]>

Cc Alexander Shishkin <[email protected]>

Cc Felipe Balbi <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
 .../devicetree/bindings/usb/ci13xxx-imx.txt        |   20 +++
 drivers/usb/chipidea/Makefile                      |    3 +
 drivers/usb/chipidea/ci13xxx_imx.c                 |  189 ++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 create mode 100644 drivers/usb/chipidea/ci13xxx_imx.c

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt new file +10 -0

new file mode 100644
index 0000000..beb75d6
@@ -0,0 +1,20 @@
+* Freescale i.MX ci13xxx usb controllers
+
+Required properties:
+- compatible: Should be "fsl,imx31-usb"
+- reg: Should contain registers location and length
+- interrupts: Should contain controller interrupt
+
+Optional properties:
+- fsl,usbphy: phandler of usb phy that connects to the only one port
+- fsl,hub-reset-gpios: gpio used to reset on-board usb hub

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt line 15

Quoted 5 lines

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt new file +5 -0

+- fsl,vbus-power-gpios: gpio used to set vbus power of the only one port
+
+Examples:
+usb at 02184000 { /* USB OTG */
+	compatible = "fsl,imx6q-usb", "fsl,imx31-usb";

The i.MX27 also has this core. As it's older that's probably the right fallback instead of i.MX31.

Quoted 20 lines

Documentation/devicetree/bindings/usb/ci13xxx-imx.txt new file +5 -0

+	reg = <0x02184000 0x200>;
+	interrupts = <0 43 0x04>;
+	fsl,usbphy = <&usbphy1>;
+	fsl,vbus-power-gpios = <&gpio3 22 0>;
+};

drivers/usb/chipidea/Makefile +6 -0

index b69900a..5c66d9c 100644
@@ -14,3 +14,6 @@ ifneq ($(CONFIG_PCI),)
 	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_pci.o
 endif
 
+ifneq ($(CONFIG_OF_DEVICE),)
+	obj-$(CONFIG_USB_CHIPIDEA)	+= ci13xxx_imx.o
+endif
+
+static int __devinit ci13xxx_imx_probe(struct platform_device *pdev)
+{

drivers/usb/chipidea/Makefile ifneq ($(CONFIG_PCI),)

Quoted 10 lines

drivers/usb/chipidea/Makefile +10 -0

+	struct ci13xxx_imx_data *data;
+	struct platform_device *plat_ci, *phy_pdev;
+	struct device_node *phy_np;
+	struct resource *res;
+	struct regulator *reg_vbus;
+	int ret;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		dev_err(&pdev->dev, "Failed to allocate CI13xxx-IMX data!\n");

Same as in the other patch: This message contains no useful information.

Quoted 10 lines

drivers/usb/chipidea/Makefile +10 -0

+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Can't get device resources!\n");
+		return -ENOENT;
+	}
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);

drivers/usb/chipidea/Makefile

Quoted 6 lines

drivers/usb/chipidea/Makefile +6 -0

+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev, "Failed to get clock!\n");
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);

You forgot to undo this is the error path.

drivers/usb/chipidea/Makefile

Quoted 8 lines

drivers/usb/chipidea/Makefile +7 -0

+	if (ret) {
+		dev_err(&pdev->dev, "Failed to prepare or enable clock!\n");
+		return ret;
+	}
+
+	phy_np = of_parse_phandle(pdev->dev.of_node, "fsl,usbphy", 0);
+	if (phy_np) {

Ok, we maybe can do without a phy on some devices, but if we have one...

drivers/usb/chipidea/Makefile

Quoted 4 lines

drivers/usb/chipidea/Makefile +3 -0

+		data->phy_np = phy_np;
+		phy_pdev = of_find_device_by_node(phy_np);
+		if (phy_pdev) {

... shouldn't it be an error if we do not find the device?

Quoted 18 lines

drivers/usb/chipidea/Makefile +17 -0

+			struct usb_phy *phy;
+			phy = pdev_to_phy(phy_pdev);
+			if (phy &&
+			    try_module_get(phy_pdev->dev.driver->owner)) {
+				usb_phy_init(phy);
+				data->phy = phy;
+			}
+		}
+	}
+
+	/* we only support host now, so enable vbus here */
+	reg_vbus = devm_regulator_get(&pdev->dev, "vbus");
+	if (!IS_ERR(reg_vbus)) {
+		ret = regulator_enable(reg_vbus);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Failed to enable vbus regulator!\n");

drivers/usb/chipidea/Makefile

Quoted 5 lines

drivers/usb/chipidea/Makefile +5 -0

+			goto put_np;
+		}
+		data->reg_vbus = reg_vbus;
+	} else
+		reg_vbus = NULL;

If you have braces in one path of a if/else add them to the other path aswell.

Quoted 5 lines

drivers/usb/chipidea/Makefile +4 -0

+
+	ci13xxx_imx_platdata.phy = data->phy;
+
+	if (!pdev->dev.dma_mask) {

drivers/usb/chipidea/Makefile

Quoted 10 lines

drivers/usb/chipidea/Makefile +10 -0

+		pdev->dev.dma_mask = devm_kzalloc(&pdev->dev,
+				      sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
+		if (!pdev->dev.dma_mask) {
+			ret = -ENOMEM;
+			dev_err(&pdev->dev, "Failed to alloc dma_mask!\n");
+			goto err;
+		}
+		*pdev->dev.dma_mask = DMA_BIT_MASK(32);
+		dma_set_coherent_mask(&pdev->dev, *pdev->dev.dma_mask);
+	}

Do you need this? I assume not.

Sascha

Signature · 8 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +0 -0

new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +5 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of gpios.

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

index e0ec929..1dd2261 100644
@@ -17,6 +17,13 @@
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 	memory {
 		reg = <0x10000000 0x40000000>;
 	};

arch/arm/mach-imx/mach-imx6q.c +35 -0

index b47e98b..577cf19 100644
@@ -19,6 +19,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 	imx6q_sabrelite_cko1_setup();
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 static void __init imx6q_init_machine(void)
 {
 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 	imx6q_pm_init();
 }
 

Quoted 28 lines

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---

 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35
 ++++++++++++++++++++ 3 files changed, 54 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/config-on-boot.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +3 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot

Quoted 5 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +1 -0

+  It must be in root node. config-on-boot means to describe settings

that needs + to be set one time on boot but aren't owned by any driver, or the owned driver + is too generic to handle such settings. For example, usb hub uses generic + driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description and should be solved within the kernel or bootloader. Either fix this in u-boot

the platform code

, or
make the generic driver support this in a generic way.

Rob

Quoted 90 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +5 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of

gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

@@ -17,6 +17,13 @@
 
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 
 	memory {

arch/arm/boot/dts/imx6q-sabrelite.dts +0 -0

 		reg = <0x10000000 0x40000000>;
 
 	};

(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644

arch/arm/mach-imx/mach-imx6q.c +1 -0

@@ -19,6 +19,7 @@
 
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 
+#include <linux/of_gpio.h>
 

#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/pinctrl/machine.h>

arch/arm/mach-imx/mach-imx6q.c +32 -0

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 
 	imx6q_sabrelite_cko1_setup();
 
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 

static void __init imx6q_init_machine(void)
{

arch/arm/mach-imx/mach-imx6q.c +0 -0

 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 

Quoted 8 lines

arch/arm/mach-imx/mach-imx6q.c +2 -0

 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 
 	imx6q_pm_init();
 

}

Dear Rob Herring,

Quoted 29 lines · Richard Zhao

Quoted 28 lines

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---

 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35
 ++++++++++++++++++++ 3 files changed, 54 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/config-on-boot.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +3 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot

Quoted 8 lines

Quoted 5 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +1 -0

+  It must be in root node. config-on-boot means to describe settings

that needs + to be set one time on boot but aren't owned by any driver, or the owned driver + is too generic to handle such settings. For example, usb hub uses generic + driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description and should be solved within the kernel or bootloader. Either fix this in u-boot

I don't want to be the bastard here, but this shouldn't go into uboot.

Quoted 1 line

the platform code

Yes, it should be here. But then, it's some usb reset, so maybe the PHY should somehow handle it?

Quoted 2 lines

, or
make the generic driver support this in a generic way.

Well, this is platform specific stuff, so it shouldn't be in any way part of the driver.

Quoted 94 lines

Rob

Quoted 90 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +5 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of

gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

@@ -17,6 +17,13 @@
 
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 
 	memory {

arch/arm/boot/dts/imx6q-sabrelite.dts +0 -0

 		reg = <0x10000000 0x40000000>;
 
 	};

(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644

arch/arm/mach-imx/mach-imx6q.c +1 -0

@@ -19,6 +19,7 @@
 
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 
+#include <linux/of_gpio.h>
 

#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/pinctrl/machine.h>

arch/arm/mach-imx/mach-imx6q.c +32 -0

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 
 	imx6q_sabrelite_cko1_setup();
 
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 

static void __init imx6q_init_machine(void)
{

arch/arm/mach-imx/mach-imx6q.c +0 -0

 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 

Quoted 8 lines

Quoted 8 lines

arch/arm/mach-imx/mach-imx6q.c +2 -0

 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 
 	imx6q_pm_init();
 

}

Best regards,
Marek Vasut

Quoted 40 lines

Dear Rob Herring,

Quoted 37 lines · Richard Zhao

Quoted 29 lines

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---

 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35
 ++++++++++++++++++++ 3 files changed, 54 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/config-on-boot.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +4 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings

Quoted 4 lines

that needs + to be set one time on boot but aren't owned by any driver, or the owned driver + is too generic to handle such settings. For example, usb hub uses generic + driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description and should be solved within the kernel or bootloader. Either fix this in u-boot

Quoted 1 line

I don't want to be the bastard here, but this shouldn't go into uboot.

Just listing possible options. Obviously, no one wants the dirty stuff in their code...

There would never be a need in u-boot to support boot from USB mass storage?

In which case you would have to handle this reset line leaving it deasserted and linux would likely never know. (Yes, I know we can't rely on bootloader setup.)

Quoted 8 lines

Quoted 1 line

the platform code

Yes, it should be here. But then, it's some usb reset, so maybe the PHY should somehow handle it?

Quoted 2 lines

, or
make the generic driver support this in a generic way.

Quoted 2 lines

Well, this is platform specific stuff, so it shouldn't be in any way part of the driver.

An embedded hub with gpio controlled reset line sounds fairly generic to me. The only platform specific part is which gpio line which is nothing new to deal with.

On the other hand, this is really a flaw in the h/w design as whether the hub is integrated on board or a separate hub should be transparent to s/w.

So just keeping it contained in platform code may be the best option.

Rob

Quoted 103 lines

Quoted 92 lines

Rob

Quoted 90 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +4 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is

set. + The value is a array of 0 or 1. Total count eaquals the number of gpios. diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index e0ec929..1dd2261 100644

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

@@ -17,6 +17,13 @@
 
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 
 	memory {

arch/arm/boot/dts/imx6q-sabrelite.dts +0 -0

 		reg = <0x10000000 0x40000000>;
 
 	};

(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6q.c index b47e98b..577cf19 100644

arch/arm/mach-imx/mach-imx6q.c +1 -0

@@ -19,6 +19,7 @@
 
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 
+#include <linux/of_gpio.h>
 

#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/pinctrl/machine.h>

arch/arm/mach-imx/mach-imx6q.c +32 -0

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 
 	imx6q_sabrelite_cko1_setup();
 
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 

static void __init imx6q_init_machine(void)
{

arch/arm/mach-imx/mach-imx6q.c +0 -0

 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 

Quoted 8 lines

Quoted 8 lines

arch/arm/mach-imx/mach-imx6q.c +2 -0

 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 
 	imx6q_pm_init();
 

}

Best regards,
Marek Vasut

Quoted 16 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt line 7

Quoted 13 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +7 -0

new file mode 100644
index 0000000..f98ed74
@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

and should be solved within the
kernel or bootloader. Either fix this in u-boot,

the platform code, or

make the generic driver support this in a generic way.

Rob

Quoted 83 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +5 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of gpios.

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

index e0ec929..1dd2261 100644
@@ -17,6 +17,13 @@
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 	memory {
 		reg = <0x10000000 0x40000000>;
 	};

arch/arm/mach-imx/mach-imx6q.c +35 -0

index b47e98b..577cf19 100644
@@ -19,6 +19,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 	imx6q_sabrelite_cko1_setup();
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 static void __init imx6q_init_machine(void)
 {
 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 	imx6q_pm_init();
 }
 

Quoted 28 lines · Richard Zhao

Quoted 27 lines

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---

 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35
 ++++++++++++++++++++ 3 files changed, 54 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/arm/config-on-boot.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/arm/config-on-boot.txt new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +2 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+

Quoted 8 lines

Quoted 6 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +2 -0

+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings

that needs + to be set one time on boot but aren't owned by any driver, or the owned driver + is too generic to handle such settings. For example, usb hub uses generic + driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

It's misc things, but is hw description.

Quoted 2 lines

and should be solved within the
kernel or bootloader. Either fix this in u-boot,

Kernel might be better not to depend on uboot.

Quoted 1 line

the platform code, or

How do I get gpio in platfrom code without dts description?

Quoted 1 line

make the generic driver support this in a generic way.

It's called just after populate devices. It's hard for generic driver to decide when it's called.

Thanks
Richard

Write a driver that does the GPIO setup in some generic way? Wasn't there something about this stuff going on? Shawn, you had some comments /wrt the M28 DTS. This might be what you meant?

Quoted 17 lines · Rob Herring

Quoted 16 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Quoted 15 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt line 7

Quoted 13 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +7 -0

new file mode 100644
index 0000000..f98ed74
@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

It's misc things, but is hw description.

Quoted 2 lines

and should be solved within the
kernel or bootloader. Either fix this in u-boot,

Kernel might be better not to depend on uboot.

Quoted 1 line

the platform code, or

How do I get gpio in platfrom code without dts description?

Quoted 1 line

make the generic driver support this in a generic way.

It's called just after populate devices. It's hard for generic driver to decide when it's called.

Thanks
Richard

Quoted 87 lines

Rob

Quoted 83 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +5 -0

+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of gpios.

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

index e0ec929..1dd2261 100644
@@ -17,6 +17,13 @@
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 	memory {
 		reg = <0x10000000 0x40000000>;
 	};

arch/arm/mach-imx/mach-imx6q.c +35 -0

index b47e98b..577cf19 100644
@@ -19,6 +19,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 	imx6q_sabrelite_cko1_setup();
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {
+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");
+	}
+
+	of_node_put(np);
+}
+
+static void __init imx6q_post_populate(void)
+{
+	imx6q_config_on_boot();
+}
+
 static void __init imx6q_init_machine(void)
 {
 	/*

@@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
+	imx6q_post_populate();
+
 	imx6q_pm_init();
 }
 

Quoted 33 lines · Richard Zhao

Quoted 32 lines · Rob Herring

Quoted 22 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +0 -0

new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt line 7

Quoted 7 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +7 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

Quoted 1 line

It's misc things, but is hw description.

You are defining something based on whether linux has a driver or not. That should not matter to DT. That's backwards.

Quoted 5 lines

Quoted 3 lines

and should be solved within the
kernel or bootloader. Either fix this in u-boot,

Kernel might be better not to depend on uboot.

Quoted 1 line

the platform code, or

Quoted 4 lines

How do I get gpio in platfrom code without dts description?

Quoted 1 line

make the generic driver support this in a generic way.

It's called just after populate devices. It's hard for generic driver to decide when it's called.

You need to describe that you have a hub on the usb bus and add the gpio line to that node. Just like PCI is probe-able, you still need DT nodes sometimes for cases like this.

A simpler approach would be to just add
the gpio to the ehci controller node, but that's not exactly correct.

Rob

Quoted 6 lines

Thanks
Richard

Quoted 3 lines

Rob

Quoted 34 lines · Rob Herring

Quoted 33 lines · Richard Zhao

Quoted 32 lines · Rob Herring

Quoted 22 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +0 -0

new file mode 100644
index 0000000..f98ed74

Documentation/devicetree/bindings/arm/config-on-boot.txt line 7

Quoted 7 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +7 -0

@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

Quoted 4 lines

Quoted 1 line

It's misc things, but is hw description.

You are defining something based on whether linux has a driver or not. That should not matter to DT. That's backwards.

I just define the board needs some misc settings. If you think config-on-boot is not good, we can name it like config-misc.

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

and should be solved within the
kernel or bootloader. Either fix this in u-boot,

Kernel might be better not to depend on uboot.

Quoted 1 line

the platform code, or

How do I get gpio in platfrom code without dts description?

Quoted 1 line

make the generic driver support this in a generic way.

It's called just after populate devices. It's hard for generic driver to decide when it's called.

Quoted 3 lines

You need to describe that you have a hub on the usb bus and add the gpio line to that node. Just like PCI is probe-able, you still need DT nodes sometimes for cases like this.

PCI has dev id and may add quirks. But for embedded, I don't know how to connect a of node to a hub device enumerated by usb core code. And it may also pollute the usb core code.

Quoted 2 lines

A simpler approach would be to just add
the gpio to the ehci controller node, but that's not exactly correct.

That's exactly why this patch comes out.

I mean just something like "fsl,hub-reset-gpios" in the ehci device node. It's at least under a usb node. Whether the ehci driver handles this or you just have a separate piece of code to find this property and setup the gpio is up to you.

Quoted 5 lines · Rob Herring

I mean just something like "fsl,hub-reset-gpios" in the ehci device node. It's at least under a usb node. Whether the ehci driver handles this or you just have a separate piece of code to find this property and setup the gpio is up to you.

I ever had an argument on this. If we have "fsl,hub-reset-gpios" defined in ehci device node, it should simply be ehci driver who needs to find this property and set up the gpio. Defining the property in ehci device node while having platform code to find the property just seems wired to me. That said, if it's not ehci driver who will use the property, we should not define the property in in ehci device node.

Quoted 8 lines · Richard Zhao

Quoted 7 lines · Rob Herring

Quoted 6 lines · Richard Zhao

Quoted 5 lines · Rob Herring

Quoted 4 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Documentation/devicetree/bindings/arm/config-on-boot.txt +0 -0

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt line 7

Quoted 5 lines

Documentation/devicetree/bindings/arm/config-on-boot.txt +5 -0

+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.

NAK. This is not a h/w description

Quoted 7 lines

Quoted 3 lines

You need to describe that you have a hub on the usb bus and add the gpio line to that node. Just like PCI is probe-able, you still need DT nodes sometimes for cases like this.

PCI has dev id and may add quirks. But for embedded, I don't know how to connect a of node to a hub device enumerated by usb core code. And it may also pollute the usb core code.

USB also uses device and manufacturer ids. One of the main reasons for putting pci devices into dts is to describe out of band signals just like this.

Creating a binding and support code for full usb bus topology would be a lot of work which is why I propose a simpler approach below.

Quoted 3 lines

Quoted 2 lines

A simpler approach would be to just add
the gpio to the ehci controller node, but that's not exactly correct.

That's exactly why this patch comes out.

I mean just something like "fsl,hub-reset-gpios" in the ehci device node. It's at least under a usb node. Whether the ehci driver handles this or you just have a separate piece of code to find this property and setup the gpio is up to you.

I haven't been following the thread, but just noticed it. Very similar things (DT nodes for probed devices) are coming up quite a bit recently, for example:

Re: Where to power on the wifi device before loading the driver.
http://www.spinics.net/lists/arm-kernel/msg180368.html

dt: rfkill-gpio: add bindings documentation
http://www.spinics.net/lists/linux-tegra/msg03977.html

I wonder if there's any kind of infra-structure or standardized bindings that would be useful here?

Quoted 5 lines

I haven't been following the thread, but just noticed it. Very similar things (DT nodes for probed devices) are coming up quite a bit recently, for example:

Quoted 8 lines

Re: Where to power on the wifi device before loading the driver.
http://www.spinics.net/lists/arm-kernel/msg180368.html

dt: rfkill-gpio: add bindings documentation
http://www.spinics.net/lists/linux-tegra/msg03977.html

I wonder if there's any kind of infra-structure or standardized bindings that would be useful here?

Hi Stephen,
For your suggestion:

sdhci at 78000000 {
	compatible = "nvidia,tegra30-sdhci", "nvidia,tegra20-sdhci";
	reg = <0x78000000 0x200>;
	interrupts = <0 14 0x04>;

	sdio-device {
		reset-gpios = <...>;
		enable-gpios = <...>;
	};
};

I have a question, if reset-gpio or enable-gpio is not set, how bus (sd or usb) know it is a sdio device as the wifi is not powered on at the time, it will not response host's command, the hub at usb bus is the same.

That's a good point. We need those properties to be part of the device that controls the port/slot/connector, not the thing that plugs into it. Having each such binding and driver implement this themselves might not be a good idea though. Perhaps we can standardize on a mechanism for this; something vaguely like rfkill, but for arbitrary types of devices connected to ports.

For USB at least, where the power/reset/... GPIOs might affect just one port on a hub rather than the root USB host controller, the issue of representing these properties on a dynamically probed device still applies. Something like:

ehci {
    /* The following node is dynamically detected, although the hub
     * IC is physically soldered onto the board
     */
    hub {
        hub { /* also dynamic */
            port at 2 {
                child-supply = <&regulator>;
                reset-gpios = <&gpio 0 0>;
            };
        };
    };
};

But without knowing what might be connected to port at 2, how do we know how to sequence the power supply and reset GPIOs according to its requirements? I guess this is part of the specification of the port itself though, not the attached device, since anything that is allowed to plug in must conform to whatever sequencing the port is designed to give. If the connected device is known, the port at 2 node in DT can encode whatever sequencing requirements are associated with the known connected device.

Quoted 28 lines

That's a good point. We need those properties to be part of the device that controls the port/slot/connector, not the thing that plugs into it. Having each such binding and driver implement this themselves might not be a good idea though. Perhaps we can standardize on a mechanism for this; something vaguely like rfkill, but for arbitrary types of devices connected to ports.

For USB at least, where the power/reset/... GPIOs might affect just one port on a hub rather than the root USB host controller, the issue of representing these properties on a dynamically probed device still applies. Something like:

ehci {
    /* The following node is dynamically detected, although the hub
     * IC is physically soldered onto the board
     */
    hub {
        hub { /* also dynamic */
            port at 2 {
                child-supply = <&regulator>;
                reset-gpios = <&gpio 0 0>;
            };
        };
    };
};

Quoted 8 lines

But without knowing what might be connected to port at 2, how do we know how to sequence the power supply and reset GPIOs according to its requirements? I guess this is part of the specification of the port itself though, not the attached device, since anything that is allowed to plug in must conform to whatever sequencing the port is designed to give. If the connected device is known, the port at 2 node in DT can encode whatever sequencing requirements are associated with the known connected device.

In fact, for specific usb/sdio interface device, there should be a specific interface class driver for it, the best place is at that class driver, like wifi or 3g driver, as it is chip/device specific.

But the generic 3G and HUB devices are special, there is only one common driver for each of them (drivers/usb/core/hub.c & drivers/serial/option.c)

But like you said, there's no way to tell what's attached to a USB or SDIO port before probing it, at least if the port is at all user-accessible. The class driver for the host-side of the port (USB hub or SDIO controller) would be the one that needs to implement this, not the class driver for the thing that could be plugged in (WiFi or 3G device).

Quoted 1 line

But the generic 3G and HUB devices are special, there is only one

common driver

Quoted 1 line

for each of them (drivers/usb/core/hub.c & drivers/serial/option.c)

(quoted diff) +16 -0

+static void __init imx6q_config_on_boot(void)
+{
+ ? ? ? struct device_node *np;
+ ? ? ? struct property *pp;
+ ? ? ? int cnt, len, i;
+ ? ? ? int gpio;
+
+ ? ? ? np = of_find_node_by_path("/config-on-boot");
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? return;
+ ? ? ? cnt = of_gpio_named_count(np, "output-gpios");
+ ? ? ? pp = of_find_property(np, "output-gpio-values", &len);
+ ? ? ? if (!pp || cnt != len / sizeof(u32)) {
+ ? ? ? ? ? ? ? pr_err("Invalid config-on-boot gpios!\n");
+ ? ? ? ? ? ? ? of_node_put(np);
+ ? ? ? ? ? ? ? return;

Quoted 16 lines · Richard Zhao

(quoted diff) +16 -0

+static void __init imx6q_config_on_boot(void)
+{
+ ? ? ? struct device_node *np;
+ ? ? ? struct property *pp;
+ ? ? ? int cnt, len, i;
+ ? ? ? int gpio;
+
+ ? ? ? np = of_find_node_by_path("/config-on-boot");
+ ? ? ? if (!np)
+ ? ? ? ? ? ? ? return;
+ ? ? ? cnt = of_gpio_named_count(np, "output-gpios");
+ ? ? ? pp = of_find_property(np, "output-gpio-values", &len);
+ ? ? ? if (!pp || cnt != len / sizeof(u32)) {
+ ? ? ? ? ? ? ? pr_err("Invalid config-on-boot gpios!\n");
+ ? ? ? ? ? ? ? of_node_put(np);
+ ? ? ? ? ? ? ? return;

Review

Quoted 10 lines

(quoted diff) +10 -0

+ ? ? ? }
+ ? ? ? for (i = 0; i < cnt; i++) {
+ ? ? ? ? ? ? ? gpio = of_get_named_gpio(np, "output-gpios", i);
+ ? ? ? ? ? ? ? if (gpio_is_valid(gpio))
+ ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "config-on-boot");
+ ? ? ? }
+
+ ? ? ? of_node_put(np);
+}

Couldn't this function be made generic and put outside of mach-imx6q.c so that other platforms could use it?

I need the same for mxs.

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---
 .../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
 arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
 3 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +12 -0

new file mode 100644
index 0000000..f98ed74
@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.
+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of gpios.

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

index e0ec929..1dd2261 100644
@@ -17,6 +17,13 @@
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
 
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 	memory {
 		reg = <0x10000000 0x40000000>;
 	};

arch/arm/mach-imx/mach-imx6q.c +13 -0

index b47e98b..577cf19 100644
@@ -19,6 +19,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 	imx6q_sabrelite_cko1_setup();
 }
 
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);

Quoted 84 lines · Richard Zhao

Sometimes, boards have gpios that don't own by any driver or owner by a generic driver that don't like hacks. Such gpios is normally output and need setup once on boot. So I introduce the config-on-boot gpios.

Signed-off-by Richard Zhao <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Rob Herring <[email protected]>

Cc Grant Likely <[email protected]>

---

.../devicetree/bindings/arm/config-on-boot.txt     |   12 +++++++
arch/arm/boot/dts/imx6q-sabrelite.dts              |    7 ++++
arch/arm/mach-imx/mach-imx6q.c                     |   35 ++++++++++++++++++++
3 files changed, 54 insertions(+)

create mode 100644 Documentation/devicetree/bindings/arm/config-on-boot.txt

Documentation/devicetree/bindings/arm/config-on-boot.txt new file +12 -0

new file mode 100644
index 0000000..f98ed74
@@ -0,0 +1,12 @@
+* Configure on Boot
+
+Node name: config-on-boot
+  It must be in root node. config-on-boot means to describe settings that needs
+  to be set one time on boot but aren't owned by any driver, or the owned driver
+  is too generic to handle such settings. For example, usb hub uses generic
+  driver in usb core code, a on-board usb may need deassert reset pin.
+
+Optional properties:
+- output-gpios: Output gpio array that needs to set.
+- output-gpio-values: This property is required if output-gpios is set.
+  The value is a array of 0 or 1. Total count eaquals the number of gpios.

arch/arm/boot/dts/imx6q-sabrelite.dts +7 -0

index e0ec929..1dd2261 100644
@@ -17,6 +17,13 @@
 	model = "Freescale i.MX6 Quad SABRE Lite Board";
 	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
+	config-on-boot {
+		output-gpios = <
+				&gpio3 22 0>;	/* vbus reset */
+		output-gpio-values = <
+				1>;		/* vbus reset */
+	};
+
 	memory {
 		reg = <0x10000000 0x40000000>;
 	};

arch/arm/mach-imx/mach-imx6q.c +19 -0

index b47e98b..577cf19 100644
@@ -19,6 +19,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>

@@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
 	imx6q_sabrelite_cko1_setup();
 }
+static void __init imx6q_config_on_boot(void)
+{
+	struct device_node *np;
+	struct property *pp;
+	int cnt, len, i;
+	int gpio;
+
+	np = of_find_node_by_path("/config-on-boot");
+	if (!np)
+		return;
+	cnt = of_gpio_named_count(np, "output-gpios");
+	pp = of_find_property(np, "output-gpio-values", &len);
+	if (!pp || cnt != len / sizeof(u32)) {
+		pr_err("Invalid config-on-boot gpios!\n");
+		of_node_put(np);
+		return;
+	}
+	for (i = 0; i < cnt; i++) {

arch/arm/mach-imx/mach-imx6q.c static void __init imx6q_sabrelite_init(void) · line 138

Quoted 4 lines

arch/arm/mach-imx/mach-imx6q.c +4 -0

+		gpio = of_get_named_gpio(np, "output-gpios", i);
+		if (gpio_is_valid(gpio))
+			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
+					"config-on-boot");

While trying to use this code: Is there a special reason why all pins are set to HIGH here? The contents of 'output-gpio-values' seem to be ignored?

Best regards

Dirk

Signed-off-by Richard Zhao <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

arch/arm/mach-imx/clk-imx6q.c +4 -1

index f99509a..8c4166a 100644
@@ -152,7 +152,7 @@ enum mx6q_clks {
 	ssi2, ssi3, uart_ipg, uart_serial, usboh3, usdhc1, usdhc2, usdhc3,
 	usdhc4, vdo_axi, vpu_axi, cko1, pll1_sys, pll2_bus, pll3_usb_otg,
 	pll4_audio, pll5_video, pll6_mlb, pll7_usb_host, pll8_enet, ssi1_ipg,
-	ssi2_ipg, ssi3_ipg, clk_max
+	ssi2_ipg, ssi3_ipg, usbphy1, usbphy2, clk_max
 };
 
 static struct clk *clk[clk_max];

@@ -197,6 +197,9 @@ int __init mx6q_clocks_init(void)
 	clk[pll7_usb_host] = imx_clk_pllv3(IMX_PLLV3_USB,	"pll7_usb_host","osc", base + 0x20, 0x2000,   0x3);
 	clk[pll8_enet]     = imx_clk_pllv3(IMX_PLLV3_ENET,	"pll8_enet",	"osc", base + 0xe0, 0x182000, 0x3);
 
+	clk[usbphy1] = imx_clk_gate("usbphy1", "pll3_usb_otg", base + 0x10, 6);
+	clk[usbphy2] = imx_clk_gate("usbphy2", "pll7_usb_host", base + 0x20, 6);
+
 	/*                                name              parent_name        reg       idx */
 	clk[pll2_pfd0_352m] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base + 0x100, 0);
 	clk[pll2_pfd1_594m] = imx_clk_pfd("pll2_pfd1_594m", "pll2_bus",     base + 0x100, 1);

Could you recheck with your hardware guys which input clocks this device really has. On earlier i.MX there are up to three clocks this module needs. Sooner or later we want to use this driver on the other i.MX aswell, and then we'll need support for the other clocks here, no matter if they are software controllable on i.MX6 or not.

+ ? ? ? ? ? ? ? ? ? ? ? iomuxc at 020e0000 {
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pinctrl-names = "default";
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? pinctrl-0 = <&pinctrl_gpio_hog>;

The work is based on ?ci13xxx rework done by Alexander Shishkin.

Status:
?- this version only support host
?- usbotg controler works at host role

Changes since last version:
?- collect ack received.

Richard Zhao (13):
?usb: otg: add notify_connect/notify_disconnect callback ?USB: move transceiver from ehci_hcd and ohci_hcd to hcd and rename it ? ?as phy
?USB: notify phy when root hub port connect change
?usb: chipidea: permit driver bindings pass phy pointer ?usb: otg: add basic mxs phy driver support
?usb: chipidea: add imx platform driver
?ARM: imx6q: correct device name of usbphy and usb controller clock ? ?export
?ARM: imx6q: add config-on-boot gpios
?ARM: imx6q: add usbphy clocks
?ARM: imx6q: disable usb charger detector
?ARM: dts: imx6q-sabrelite: add usb devices
?ARM: mxs: clk_register_clkdev mx28 usb clocks
?ARM: dts: imx28-evk: add usb devices

Hi Richard,

On Wed, Jun 13, 2012 at 9:34 AM, Richard Zhao

Quoted 9 lines · <[email protected]> wrote:

The work is based on ci13xxx rework done by Alexander Shishkin.

Status:
- this version only support host
- usbotg controler works at host role

Changes since last version:
- collect ack received.

Quoted 16 lines

Richard Zhao (13):
usb: otg: add notify_connect/notify_disconnect callback
USB: move transceiver from ehci_hcd and ohci_hcd to hcd and rename it
as phy
USB: notify phy when root hub port connect change
usb: chipidea: permit driver bindings pass phy pointer
usb: otg: add basic mxs phy driver support
usb: chipidea: add imx platform driver
ARM: imx6q: correct device name of usbphy and usb controller clock
export
ARM: imx6q: add config-on-boot gpios
ARM: imx6q: add usbphy clocks
ARM: imx6q: disable usb charger detector
ARM: dts: imx6q-sabrelite: add usb devices
ARM: mxs: clk_register_clkdev mx28 usb clocks
ARM: dts: imx28-evk: add usb devices

I am trying your patch series on a mx28evk and I am not able to have my USB pen drive detected.

I tried enabling all the required config options in .config.

Could you please share your .config?

Thanks,

Fabio Estevam

Do you have root TT enabled? If the pendrive isn't detected at all, it's

probably the PHY not enabling (disabling?) the discon detector upon init. But the PHY driver is from my code, so that should work. Do you see any response at all upon connecting the stick?

Quoted 3 lines

Do you have root TT enabled? If the pendrive isn't detected at all,

it's

USB_EHCI_ROOT_HUB_TT is enabled.

Quoted 1 line

probably the PHY not enabling (disabling?) the discon detector upon

init. But

Quoted 1 line

the PHY driver is from my code, so that should work. Do you see any

response at

Quoted 1 line

all upon connecting the stick?

Nothing shows up in the console after connecting the stick.

Attached my .config file.

Thanks,

Fabio Estevam

Quoted 4 lines

Nothing shows up in the console after connecting the stick.

Attached my .config file.

Make sure the vbus is on for your usb port.

Peter,

On Mon, Jun 18, 2012 at 10:14 PM, Chen Peter-B29397

Quoted 4 lines · <[email protected]> wrote:

Quoted 3 lines

Nothing shows up in the console after connecting the stick.

Attached my .config file.

Quoted 1 line

Make sure the vbus is on for your usb port.

So what is the magic needed here? Isn't Richard's series supposed to work as is?

What should be done in addition to Richard's patch in order to get USB host to work on mx28evk?

Quoted 2 lines

What should be done in addition to Richard's patch in order to get USB host to work on mx28evk?

Ah right, you need to toggle the power supply (vbus) GPIO.

Nothing shows up in the console after connecting the stick.

Attached my .config file.

Quoted 2 lines

Attached my .config file.

You haven't enabled the following two in your .config.

CONFIG_USB_CHIPIDEA
CONFIG_USB_CHIPIDEA_HOST

Can someone confirm that this patch series works on a mx28evk? If it does, please send me the .config that works.

Quoted 3 lines · Fabio Estevam

Can someone confirm that this patch series works on a mx28evk? If it does, please send me the .config that works.

The series works fine on my imx28-evk board with the .config attached.

Excellent! It works fine now ;-)

Quoted 1 line · Fabio Estevam

Excellent! It works fine now ;-)

In case someone wants to try this USB patch series:
https://github.com/Freescale/linux-mainline/commits/patches-3.5-rc3