All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata
@ 2016-12-21  8:14 Peng Fan
  2016-12-21  8:14 ` [U-Boot] [PATCH 2/3] usb: ehci-mx6: handle vbus-supply Peng Fan
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-21  8:14 UTC (permalink / raw)
  To: u-boot

Implement ofdata_to_platdata to set the type to host or device.
 - Check "dr-mode" property.
 - If there is no "dr-mode", check phy_ctrl for i.MX6
   and phy_status for i.MX7

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/usb/host/ehci-mx6.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 48889c1..91413c9 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -15,10 +15,13 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 #include <asm/imx-common/iomux-v3.h>
+#include <asm/imx-common/sys_proto.h>
 #include <dm.h>
 
 #include "ehci.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define USB_OTGREGS_OFFSET	0x000
 #define USB_H1REGS_OFFSET	0x200
 #define USB_H2REGS_OFFSET	0x400
@@ -48,6 +51,7 @@
 #define ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS	0x00000040
 
 #define USBNC_OFFSET		0x200
+#define USBNC_PHY_STATUS_OFFSET	0x23C
 #define USBNC_PHYSTATUS_ID_DIG	(1 << 4) /* otg_id status */
 #define USBNC_PHYCFG2_ACAENB	(1 << 4) /* otg_id detection enable */
 #define UCTRL_PWR_POL		(1 << 9) /* OTG Polarity of Power Pin */
@@ -417,6 +421,67 @@ static const struct ehci_ops mx6_ehci_ops = {
 	.init_after_reset = mx6_init_after_reset
 };
 
+static int ehci_usb_ofdata_to_platdata(struct udevice *dev)
+{
+	struct usb_platdata *plat = dev_get_platdata(dev);
+	void *__iomem addr = (void *__iomem)dev_get_addr(dev);
+	void *__iomem phy_ctrl, *__iomem phy_status;
+	const void *blob = gd->fdt_blob;
+	int offset = dev->of_offset, phy_off;
+	const char *mode;
+	u32 val;
+
+	mode = fdt_getprop(blob, offset, "dr_mode", NULL);
+	if (mode) {
+		if (strcmp(mode, "peripheral") == 0)
+			plat->init_type = USB_INIT_DEVICE;
+		else if (strcmp(mode, "host") == 0)
+			plat->init_type = USB_INIT_HOST;
+		else if (strcmp(mode, "otg") == 0)
+			plat->init_type = USB_INIT_HOST;
+		else
+			return -EINVAL;
+	} else {
+		/*
+		 * About fsl,usbphy, Refer to
+		 * Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt.
+		 */
+		if (is_mx6()) {
+			phy_off = fdtdec_lookup_phandle(blob,
+							offset,
+							"fsl,usbphy");
+			if (phy_off < 0)
+				return -EINVAL;
+
+			addr = (void __iomem *)fdtdec_get_addr(blob, phy_off,
+							       "reg");
+			if ((fdt_addr_t)addr == FDT_ADDR_T_NONE)
+				return -EINVAL;
+
+			phy_ctrl = (void __iomem *)(addr + USBPHY_CTRL);
+			val = readl(phy_ctrl);
+
+			if (val & USBPHY_CTRL_OTG_ID)
+				plat->init_type = USB_INIT_DEVICE;
+			else
+				plat->init_type = USB_INIT_HOST;
+		} else if (is_mx7()) {
+			phy_status = (void __iomem *)(addr +
+						      USBNC_PHY_STATUS_OFFSET);
+			val = readl(phy_status);
+
+			if (val & USBNC_PHYSTATUS_ID_DIG)
+				plat->init_type = USB_INIT_DEVICE;
+			else
+				plat->init_type = USB_INIT_HOST;
+		} else {
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int ehci_usb_probe(struct udevice *dev)
 {
 	struct usb_platdata *plat = dev_get_platdata(dev);
@@ -460,6 +525,7 @@ U_BOOT_DRIVER(usb_mx6) = {
 	.name	= "ehci_mx6",
 	.id	= UCLASS_USB,
 	.of_match = mx6_usb_ids,
+	.ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
 	.probe	= ehci_usb_probe,
 	.remove = ehci_deregister,
 	.ops	= &ehci_usb_ops,
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 2/3] usb: ehci-mx6: handle vbus-supply
  2016-12-21  8:14 [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Peng Fan
@ 2016-12-21  8:14 ` Peng Fan
  2016-12-21  8:14 ` [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support Peng Fan
  2016-12-21 14:07 ` [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Marek Vasut
  2 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-21  8:14 UTC (permalink / raw)
  To: u-boot

Drop board_ehci_power when dm usb used and switch to use
regulator api to handle vbus.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/usb/host/ehci-mx6.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 91413c9..123befe 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -17,6 +17,7 @@
 #include <asm/imx-common/iomux-v3.h>
 #include <asm/imx-common/sys_proto.h>
 #include <dm.h>
+#include <power/regulator.h>
 
 #include "ehci.h"
 
@@ -388,6 +389,7 @@ int ehci_hcd_stop(int index)
 struct ehci_mx6_priv_data {
 	struct ehci_ctrl ctrl;
 	struct usb_ehci *ehci;
+	struct udevice *vbus_supply;
 	enum usb_init_type init_type;
 	int portnr;
 };
@@ -403,7 +405,15 @@ static int mx6_init_after_reset(struct ehci_ctrl *dev)
 	if (ret)
 		return ret;
 
-	board_ehci_power(priv->portnr, (type == USB_INIT_DEVICE) ? 0 : 1);
+	if (priv->vbus_supply) {
+		ret = regulator_set_enable(priv->vbus_supply,
+					   (type == USB_INIT_DEVICE) ?
+					   false : true);
+		if (ret) {
+			puts("Error enabling VBUS supply\n");
+			return ret;
+		}
+	}
 
 	if (type == USB_INIT_DEVICE)
 		return 0;
@@ -487,19 +497,33 @@ static int ehci_usb_probe(struct udevice *dev)
 	struct usb_platdata *plat = dev_get_platdata(dev);
 	struct usb_ehci *ehci = (struct usb_ehci *)dev_get_addr(dev);
 	struct ehci_mx6_priv_data *priv = dev_get_priv(dev);
+	enum usb_init_type type = plat->init_type;
 	struct ehci_hccr *hccr;
 	struct ehci_hcor *hcor;
 	int ret;
 
 	priv->ehci = ehci;
 	priv->portnr = dev->seq;
-	priv->init_type = plat->init_type;
+	priv->init_type = type;
+
+	ret = device_get_supply_regulator(dev, "vbus-supply",
+					  &priv->vbus_supply);
+	if (ret)
+		debug("%s: No vbus supply\n", dev->name);
 
 	ret = ehci_mx6_common_init(ehci, priv->portnr);
 	if (ret)
 		return ret;
 
-	board_ehci_power(priv->portnr, (priv->init_type == USB_INIT_DEVICE) ? 0 : 1);
+	if (priv->vbus_supply) {
+		ret = regulator_set_enable(priv->vbus_supply,
+					   (type == USB_INIT_DEVICE) ?
+					   false : true);
+		if (ret) {
+			puts("Error enabling VBUS supply\n");
+			return ret;
+		}
+	}
 
 	if (priv->init_type == USB_INIT_HOST) {
 		setbits_le32(&ehci->usbmode, CM_HOST);
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-21  8:14 [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Peng Fan
  2016-12-21  8:14 ` [U-Boot] [PATCH 2/3] usb: ehci-mx6: handle vbus-supply Peng Fan
@ 2016-12-21  8:14 ` Peng Fan
  2016-12-21 14:01   ` Jagan Teki
  2016-12-21 14:09   ` Marek Vasut
  2016-12-21 14:07 ` [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Marek Vasut
  2 siblings, 2 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-21  8:14 UTC (permalink / raw)
  To: u-boot

Add usb support for mx6sllevk board.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 board/freescale/mx6sllevk/mx6sllevk.c | 18 ++++++++++++++++++
 configs/mx6sllevk_defconfig           |  5 +++++
 configs/mx6sllevk_plugin_defconfig    |  5 +++++
 include/configs/mx6sllevk.h           |  9 +++++++++
 4 files changed, 37 insertions(+)

diff --git a/board/freescale/mx6sllevk/mx6sllevk.c b/board/freescale/mx6sllevk/mx6sllevk.c
index 74a27a3..e6679fd 100644
--- a/board/freescale/mx6sllevk/mx6sllevk.c
+++ b/board/freescale/mx6sllevk/mx6sllevk.c
@@ -129,3 +129,21 @@ int mmc_map_to_kernel_blk(int devno)
 {
 	return devno;
 }
+
+#define USB_OTHERREGS_OFFSET   0x800
+#define UCTRL_PWR_POL          (1 << 9)
+
+int board_ehci_hcd_init(int port)
+{
+	u32 *usbnc_usb_ctrl;
+
+	if (port > 1)
+		return -EINVAL;
+
+	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
+				 port * 4);
+
+	/* Set Power polarity */
+	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
+	return 0;
+}
diff --git a/configs/mx6sllevk_defconfig b/configs/mx6sllevk_defconfig
index 8ae049e..267cdc6 100644
--- a/configs/mx6sllevk_defconfig
+++ b/configs/mx6sllevk_defconfig
@@ -10,6 +10,7 @@ CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_USB=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_DHCP=y
 CONFIG_CMD_PING=y
@@ -34,3 +35,7 @@ CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_PFUZE100=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_STORAGE=y
diff --git a/configs/mx6sllevk_plugin_defconfig b/configs/mx6sllevk_plugin_defconfig
index e6be979..63d5bbc 100644
--- a/configs/mx6sllevk_plugin_defconfig
+++ b/configs/mx6sllevk_plugin_defconfig
@@ -11,6 +11,7 @@ CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_CMD_MMC=y
 CONFIG_CMD_I2C=y
+CONFIG_CMD_USB=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_DHCP=y
 CONFIG_CMD_PING=y
@@ -35,3 +36,7 @@ CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_PFUZE100=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_STORAGE=y
diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h
index b9f25cf..3f665a3 100644
--- a/include/configs/mx6sllevk.h
+++ b/include/configs/mx6sllevk.h
@@ -149,4 +149,13 @@
 
 #define CONFIG_IOMUX_LPSR
 
+/* USB Configs */
+#ifdef CONFIG_CMD_USB
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_ASIX
+#define CONFIG_USB_ETHER_RTL8152
+#define CONFIG_MXC_USB_PORTSC		(PORT_PTS_UTMI | PORT_PTS_PTW)
+#define CONFIG_USB_MAX_CONTROLLER_COUNT	2
+#endif
+
 #endif				/* __CONFIG_H */
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-21  8:14 ` [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support Peng Fan
@ 2016-12-21 14:01   ` Jagan Teki
  2016-12-22  1:16     ` Peng Fan
  2016-12-21 14:09   ` Marek Vasut
  1 sibling, 1 reply; 19+ messages in thread
From: Jagan Teki @ 2016-12-21 14:01 UTC (permalink / raw)
  To: u-boot

On Wed, Dec 21, 2016 at 9:14 AM, Peng Fan <peng.fan@nxp.com> wrote:
> Add usb support for mx6sllevk board.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  board/freescale/mx6sllevk/mx6sllevk.c | 18 ++++++++++++++++++
>  configs/mx6sllevk_defconfig           |  5 +++++
>  configs/mx6sllevk_plugin_defconfig    |  5 +++++
>  include/configs/mx6sllevk.h           |  9 +++++++++
>  4 files changed, 37 insertions(+)
>
> diff --git a/board/freescale/mx6sllevk/mx6sllevk.c b/board/freescale/mx6sllevk/mx6sllevk.c
> index 74a27a3..e6679fd 100644
> --- a/board/freescale/mx6sllevk/mx6sllevk.c
> +++ b/board/freescale/mx6sllevk/mx6sllevk.c
> @@ -129,3 +129,21 @@ int mmc_map_to_kernel_blk(int devno)
>  {
>         return devno;
>  }
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL          (1 << 9)
> +
> +int board_ehci_hcd_init(int port)

Can't we do this from driver itself, .probe or somewhere?

> +{
> +       u32 *usbnc_usb_ctrl;
> +
> +       if (port > 1)
> +               return -EINVAL;
> +
> +       usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
> +                                port * 4);
> +
> +       /* Set Power polarity */
> +       setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> +       return 0;
> +}
> diff --git a/configs/mx6sllevk_defconfig b/configs/mx6sllevk_defconfig
> index 8ae049e..267cdc6 100644
> --- a/configs/mx6sllevk_defconfig
> +++ b/configs/mx6sllevk_defconfig
> @@ -10,6 +10,7 @@ CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_MEMTEST=y
>  CONFIG_CMD_MMC=y
>  CONFIG_CMD_I2C=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_PING=y
> @@ -34,3 +35,7 @@ CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_PFUZE100=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_USB=y
> +CONFIG_DM_USB=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_STORAGE=y
> diff --git a/configs/mx6sllevk_plugin_defconfig b/configs/mx6sllevk_plugin_defconfig
> index e6be979..63d5bbc 100644
> --- a/configs/mx6sllevk_plugin_defconfig
> +++ b/configs/mx6sllevk_plugin_defconfig
> @@ -11,6 +11,7 @@ CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_MEMTEST=y
>  CONFIG_CMD_MMC=y
>  CONFIG_CMD_I2C=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_PING=y
> @@ -35,3 +36,7 @@ CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_PFUZE100=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_USB=y
> +CONFIG_DM_USB=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_STORAGE=y
> diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h
> index b9f25cf..3f665a3 100644
> --- a/include/configs/mx6sllevk.h
> +++ b/include/configs/mx6sllevk.h
> @@ -149,4 +149,13 @@
>
>  #define CONFIG_IOMUX_LPSR
>
> +/* USB Configs */
> +#ifdef CONFIG_CMD_USB
> +#define CONFIG_USB_HOST_ETHER
> +#define CONFIG_USB_ETHER_ASIX
> +#define CONFIG_USB_ETHER_RTL8152
> +#define CONFIG_MXC_USB_PORTSC          (PORT_PTS_UTMI | PORT_PTS_PTW)
> +#define CONFIG_USB_MAX_CONTROLLER_COUNT        2

I think this is even should managed from driver isn't it?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata
  2016-12-21  8:14 [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Peng Fan
  2016-12-21  8:14 ` [U-Boot] [PATCH 2/3] usb: ehci-mx6: handle vbus-supply Peng Fan
  2016-12-21  8:14 ` [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support Peng Fan
@ 2016-12-21 14:07 ` Marek Vasut
  2016-12-22  1:20   ` Peng Fan
  2 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-21 14:07 UTC (permalink / raw)
  To: u-boot

On 12/21/2016 09:14 AM, Peng Fan wrote:
> Implement ofdata_to_platdata to set the type to host or device.
>  - Check "dr-mode" property.
>  - If there is no "dr-mode", check phy_ctrl for i.MX6
>    and phy_status for i.MX7
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  drivers/usb/host/ehci-mx6.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> index 48889c1..91413c9 100644
> --- a/drivers/usb/host/ehci-mx6.c
> +++ b/drivers/usb/host/ehci-mx6.c
> @@ -15,10 +15,13 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/clock.h>
>  #include <asm/imx-common/iomux-v3.h>
> +#include <asm/imx-common/sys_proto.h>
>  #include <dm.h>
>  
>  #include "ehci.h"
>  
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  #define USB_OTGREGS_OFFSET	0x000
>  #define USB_H1REGS_OFFSET	0x200
>  #define USB_H2REGS_OFFSET	0x400
> @@ -48,6 +51,7 @@
>  #define ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS	0x00000040
>  
>  #define USBNC_OFFSET		0x200
> +#define USBNC_PHY_STATUS_OFFSET	0x23C
>  #define USBNC_PHYSTATUS_ID_DIG	(1 << 4) /* otg_id status */
>  #define USBNC_PHYCFG2_ACAENB	(1 << 4) /* otg_id detection enable */
>  #define UCTRL_PWR_POL		(1 << 9) /* OTG Polarity of Power Pin */
> @@ -417,6 +421,67 @@ static const struct ehci_ops mx6_ehci_ops = {
>  	.init_after_reset = mx6_init_after_reset
>  };
>  
> +static int ehci_usb_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct usb_platdata *plat = dev_get_platdata(dev);
> +	void *__iomem addr = (void *__iomem)dev_get_addr(dev);
> +	void *__iomem phy_ctrl, *__iomem phy_status;
> +	const void *blob = gd->fdt_blob;
> +	int offset = dev->of_offset, phy_off;
> +	const char *mode;
> +	u32 val;
> +
> +	mode = fdt_getprop(blob, offset, "dr_mode", NULL);
> +	if (mode) {
> +		if (strcmp(mode, "peripheral") == 0)
> +			plat->init_type = USB_INIT_DEVICE;
> +		else if (strcmp(mode, "host") == 0)
> +			plat->init_type = USB_INIT_HOST;
> +		else if (strcmp(mode, "otg") == 0)
> +			plat->init_type = USB_INIT_HOST;

Shouldn't this case check the phy status register ?

> +		else
> +			return -EINVAL;
> +	} else {

You can probably do return 0 in the if (mode) branch and then indent
this whole else branch to the left.

> +		/*
> +		 * About fsl,usbphy, Refer to
> +		 * Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt.
> +		 */
> +		if (is_mx6()) {
> +			phy_off = fdtdec_lookup_phandle(blob,
> +							offset,
> +							"fsl,usbphy");
> +			if (phy_off < 0)
> +				return -EINVAL;
> +
> +			addr = (void __iomem *)fdtdec_get_addr(blob, phy_off,
> +							       "reg");
> +			if ((fdt_addr_t)addr == FDT_ADDR_T_NONE)
> +				return -EINVAL;
> +
> +			phy_ctrl = (void __iomem *)(addr + USBPHY_CTRL);
> +			val = readl(phy_ctrl);
> +
> +			if (val & USBPHY_CTRL_OTG_ID)
> +				plat->init_type = USB_INIT_DEVICE;
> +			else
> +				plat->init_type = USB_INIT_HOST;
> +		} else if (is_mx7()) {
> +			phy_status = (void __iomem *)(addr +
> +						      USBNC_PHY_STATUS_OFFSET);
> +			val = readl(phy_status);
> +
> +			if (val & USBNC_PHYSTATUS_ID_DIG)
> +				plat->init_type = USB_INIT_DEVICE;
> +			else
> +				plat->init_type = USB_INIT_HOST;
> +		} else {
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int ehci_usb_probe(struct udevice *dev)
>  {
>  	struct usb_platdata *plat = dev_get_platdata(dev);
> @@ -460,6 +525,7 @@ U_BOOT_DRIVER(usb_mx6) = {
>  	.name	= "ehci_mx6",
>  	.id	= UCLASS_USB,
>  	.of_match = mx6_usb_ids,
> +	.ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
>  	.probe	= ehci_usb_probe,
>  	.remove = ehci_deregister,
>  	.ops	= &ehci_usb_ops,
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-21  8:14 ` [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support Peng Fan
  2016-12-21 14:01   ` Jagan Teki
@ 2016-12-21 14:09   ` Marek Vasut
  2016-12-22  1:13     ` Peng Fan
  1 sibling, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-21 14:09 UTC (permalink / raw)
  To: u-boot

On 12/21/2016 09:14 AM, Peng Fan wrote:
> Add usb support for mx6sllevk board.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  board/freescale/mx6sllevk/mx6sllevk.c | 18 ++++++++++++++++++
>  configs/mx6sllevk_defconfig           |  5 +++++
>  configs/mx6sllevk_plugin_defconfig    |  5 +++++
>  include/configs/mx6sllevk.h           |  9 +++++++++
>  4 files changed, 37 insertions(+)
> 
> diff --git a/board/freescale/mx6sllevk/mx6sllevk.c b/board/freescale/mx6sllevk/mx6sllevk.c
> index 74a27a3..e6679fd 100644
> --- a/board/freescale/mx6sllevk/mx6sllevk.c
> +++ b/board/freescale/mx6sllevk/mx6sllevk.c
> @@ -129,3 +129,21 @@ int mmc_map_to_kernel_blk(int devno)
>  {
>  	return devno;
>  }
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL          (1 << 9)
> +
> +int board_ehci_hcd_init(int port)
> +{
> +	u32 *usbnc_usb_ctrl;
> +
> +	if (port > 1)
> +		return -EINVAL;
> +
> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
> +				 port * 4);
> +
> +	/* Set Power polarity */
> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> +	return 0;
> +}

Is this function similar to what usb_oc_config() does ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-21 14:09   ` Marek Vasut
@ 2016-12-22  1:13     ` Peng Fan
  2016-12-22  4:39       ` Marek Vasut
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2016-12-22  1:13 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Wednesday, December 21, 2016 10:10 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/21/2016 09:14 AM, Peng Fan wrote:
> > Add usb support for mx6sllevk board.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
[......]

> > +
> > +#define USB_OTHERREGS_OFFSET   0x800
> > +#define UCTRL_PWR_POL          (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port)
> > +{
> > +	u32 *usbnc_usb_ctrl;
> > +
> > +	if (port > 1)
> > +		return -EINVAL;
> > +
> > +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> +
> > +				 port * 4);
> > +
> > +	/* Set Power polarity */
> > +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +	return 0;
> > +}
> 
> Is this function similar to what usb_oc_config() does ?

No, this bit is not for overcurrent. According to RM, this is OTG Power Polarity
This bit should be set according to power switch's enable polarity.
1 Power switch has an active-high enable input
0 Power switch has an active-low enable input

This is board specific.

Regards,
Peng.

> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-21 14:01   ` Jagan Teki
@ 2016-12-22  1:16     ` Peng Fan
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-22  1:16 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Jagan Teki [mailto:jagan at openedev.com]
> Sent: Wednesday, December 21, 2016 10:02 PM
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>; Marek Vasut <marex@denx.de>; u-
> boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On Wed, Dec 21, 2016 at 9:14 AM, Peng Fan <peng.fan@nxp.com> wrote:
> > Add usb support for mx6sllevk board.
> >
[.........]

> > +#define UCTRL_PWR_POL          (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port)
> 
> Can't we do this from driver itself, .probe or somewhere?

This is to handle OTG power polarity, which is board specific.
There is no dts property now, so I add this in board file just like other fsl boards.
When there is an dts property upstreamed in Linux side, we could use that and move this code into driver part.

> 
> > +{
> > +       u32 *usbnc_usb_ctrl;
> > +
> > +       if (port > 1)
> > +               return -EINVAL;
> > +
> > +       usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> +
> > +                                port * 4);
> > +
> > +       /* Set Power polarity */
> > +       setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +       return 0;
> > +}
> > diff --git a/configs/mx6sllevk_defconfig b/configs/mx6sllevk_defconfig
> > index 8ae049e..267cdc6 100644
> > --- a/configs/mx6sllevk_defconfig
> > +++ b/configs/mx6sllevk_defconfig
> > @@ -10,6 +10,7 @@ CONFIG_CMD_BOOTZ=y
> >  CONFIG_CMD_MEMTEST=y
> >  CONFIG_CMD_MMC=y
> >  CONFIG_CMD_I2C=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_CMD_GPIO=y
> >  CONFIG_CMD_DHCP=y
> >  CONFIG_CMD_PING=y
> > @@ -34,3 +35,7 @@ CONFIG_DM_REGULATOR=y
> > CONFIG_DM_REGULATOR_PFUZE100=y  CONFIG_DM_REGULATOR_FIXED=y
> > CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB=y
> > +CONFIG_USB_EHCI_HCD=y
> > +CONFIG_USB_STORAGE=y
> > diff --git a/configs/mx6sllevk_plugin_defconfig
> > b/configs/mx6sllevk_plugin_defconfig
> > index e6be979..63d5bbc 100644
> > --- a/configs/mx6sllevk_plugin_defconfig
> > +++ b/configs/mx6sllevk_plugin_defconfig
> > @@ -11,6 +11,7 @@ CONFIG_CMD_BOOTZ=y
> >  CONFIG_CMD_MEMTEST=y
> >  CONFIG_CMD_MMC=y
> >  CONFIG_CMD_I2C=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_CMD_GPIO=y
> >  CONFIG_CMD_DHCP=y
> >  CONFIG_CMD_PING=y
> > @@ -35,3 +36,7 @@ CONFIG_DM_REGULATOR=y
> > CONFIG_DM_REGULATOR_PFUZE100=y  CONFIG_DM_REGULATOR_FIXED=y
> > CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB=y
> > +CONFIG_USB_EHCI_HCD=y
> > +CONFIG_USB_STORAGE=y
> > diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h
> > index b9f25cf..3f665a3 100644
> > --- a/include/configs/mx6sllevk.h
> > +++ b/include/configs/mx6sllevk.h
> > @@ -149,4 +149,13 @@
> >
> >  #define CONFIG_IOMUX_LPSR
> >
> > +/* USB Configs */
> > +#ifdef CONFIG_CMD_USB
> > +#define CONFIG_USB_HOST_ETHER
> > +#define CONFIG_USB_ETHER_ASIX
> > +#define CONFIG_USB_ETHER_RTL8152
> > +#define CONFIG_MXC_USB_PORTSC          (PORT_PTS_UTMI |
> PORT_PTS_PTW)
> > +#define CONFIG_USB_MAX_CONTROLLER_COUNT        2
> 
> I think this is even should managed from driver isn't it?

Thanks for pointing this out. This is not needed by DM USB, I'll remove it in V2.

Thanks,
Peng.

> 
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
> Maintainer Hyderabad, India.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata
  2016-12-21 14:07 ` [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Marek Vasut
@ 2016-12-22  1:20   ` Peng Fan
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-22  1:20 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Wednesday, December 21, 2016 10:08 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com; Simon Glass
> <sjg@chromium.org>
> Subject: Re: [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata
> 
> On 12/21/2016 09:14 AM, Peng Fan wrote:
> > Implement ofdata_to_platdata to set the type to host or device.
> >  - Check "dr-mode" property.
> >  - If there is no "dr-mode", check phy_ctrl for i.MX6
> >    and phy_status for i.MX7
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> >  drivers/usb/host/ehci-mx6.c | 66
> > +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 66 insertions(+)
> >
> > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> > index 48889c1..91413c9 100644
> > --- a/drivers/usb/host/ehci-mx6.c
> > +++ b/drivers/usb/host/ehci-mx6.c
> > @@ -15,10 +15,13 @@
> >  #include <asm/arch/imx-regs.h>
> >  #include <asm/arch/clock.h>
> >  #include <asm/imx-common/iomux-v3.h>
> > +#include <asm/imx-common/sys_proto.h>
> >  #include <dm.h>
> >
> >  #include "ehci.h"
> >
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> >  #define USB_OTGREGS_OFFSET	0x000
> >  #define USB_H1REGS_OFFSET	0x200
> >  #define USB_H2REGS_OFFSET	0x400
> > @@ -48,6 +51,7 @@
> >  #define ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS	0x00000040
> >
> >  #define USBNC_OFFSET		0x200
> > +#define USBNC_PHY_STATUS_OFFSET	0x23C
> >  #define USBNC_PHYSTATUS_ID_DIG	(1 << 4) /* otg_id status */
> >  #define USBNC_PHYCFG2_ACAENB	(1 << 4) /* otg_id detection enable */
> >  #define UCTRL_PWR_POL		(1 << 9) /* OTG Polarity of Power Pin */
> > @@ -417,6 +421,67 @@ static const struct ehci_ops mx6_ehci_ops = {
> >  	.init_after_reset = mx6_init_after_reset  };
> >
> > +static int ehci_usb_ofdata_to_platdata(struct udevice *dev) {
> > +	struct usb_platdata *plat = dev_get_platdata(dev);
> > +	void *__iomem addr = (void *__iomem)dev_get_addr(dev);
> > +	void *__iomem phy_ctrl, *__iomem phy_status;
> > +	const void *blob = gd->fdt_blob;
> > +	int offset = dev->of_offset, phy_off;
> > +	const char *mode;
> > +	u32 val;
> > +
> > +	mode = fdt_getprop(blob, offset, "dr_mode", NULL);
> > +	if (mode) {
> > +		if (strcmp(mode, "peripheral") == 0)
> > +			plat->init_type = USB_INIT_DEVICE;
> > +		else if (strcmp(mode, "host") == 0)
> > +			plat->init_type = USB_INIT_HOST;
> > +		else if (strcmp(mode, "otg") == 0)
> > +			plat->init_type = USB_INIT_HOST;
> 
> Shouldn't this case check the phy status register ?

Yeah. Thanks for pointing this out. Will fix it in V2.

> 
> > +		else
> > +			return -EINVAL;
> > +	} else {
> 
> You can probably do return 0 in the if (mode) branch and then indent this
> whole else branch to the left.

Will fix this in V2.

Thanks,
Peng.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  1:13     ` Peng Fan
@ 2016-12-22  4:39       ` Marek Vasut
  2016-12-22  6:03         ` Peng Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-22  4:39 UTC (permalink / raw)
  To: u-boot

On 12/22/2016 02:13 AM, Peng Fan wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Wednesday, December 21, 2016 10:10 PM
>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>> Add usb support for mx6sllevk board.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> ---
> [......]
> 
>>> +
>>> +#define USB_OTHERREGS_OFFSET   0x800
>>> +#define UCTRL_PWR_POL          (1 << 9)
>>> +
>>> +int board_ehci_hcd_init(int port)
>>> +{
>>> +	u32 *usbnc_usb_ctrl;
>>> +
>>> +	if (port > 1)
>>> +		return -EINVAL;
>>> +
>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
>> +
>>> +				 port * 4);
>>> +
>>> +	/* Set Power polarity */
>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>> +	return 0;
>>> +}
>>
>> Is this function similar to what usb_oc_config() does ?
> 
> No, this bit is not for overcurrent. According to RM, this is OTG Power Polarity
> This bit should be set according to power switch's enable polarity.
> 1 Power switch has an active-high enable input
> 0 Power switch has an active-low enable input
> 
> This is board specific.

Great, except it should also be part of the driver , the same way as
polarity is configured, yes ?


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  4:39       ` Marek Vasut
@ 2016-12-22  6:03         ` Peng Fan
  2016-12-22  6:11           ` Marek Vasut
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2016-12-22  6:03 UTC (permalink / raw)
  To: u-boot


Hi Marek,

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, December 22, 2016 12:40 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 02:13 AM, Peng Fan wrote:
> >
> >
> >> -----Original Message-----
> >> From: Marek Vasut [mailto:marex at denx.de]
> >> Sent: Wednesday, December 21, 2016 10:10 PM
> >> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/21/2016 09:14 AM, Peng Fan wrote:
> >>> Add usb support for mx6sllevk board.
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> Cc: Stefano Babic <sbabic@denx.de>
> >>> ---
> > [......]
> >
> >>> +
> >>> +#define USB_OTHERREGS_OFFSET   0x800
> >>> +#define UCTRL_PWR_POL          (1 << 9)
> >>> +
> >>> +int board_ehci_hcd_init(int port)
> >>> +{
> >>> +	u32 *usbnc_usb_ctrl;
> >>> +
> >>> +	if (port > 1)
> >>> +		return -EINVAL;
> >>> +
> >>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> >> +
> >>> +				 port * 4);
> >>> +
> >>> +	/* Set Power polarity */
> >>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>> +	return 0;
> >>> +}
> >>
> >> Is this function similar to what usb_oc_config() does ?
> >
> > No, this bit is not for overcurrent. According to RM, this is OTG
> > Power Polarity This bit should be set according to power switch's enable
> polarity.
> > 1 Power switch has an active-high enable input
> > 0 Power switch has an active-low enable input
> >
> > This is board specific.
> 
> Great, except it should also be part of the driver , the same way as polarity is
> configured, yes ?

I just found that there is code for mx7, http://lists.denx.de/pipermail/u-boot/2016-July/260321.html

Then do you agree I add such piece code in usb_power_config for mx6?
"
#ifdef CONFIG_MXC_USB_OTG_HACTIVE
        setbits_le32(ctrl, UCTRL_PWR_POL); 
#else 
        clrbits_le32(ctrl, UCTRL_PWR_POL);        
#endif
"

Thanks,
Peng
> 
> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  6:03         ` Peng Fan
@ 2016-12-22  6:11           ` Marek Vasut
  2016-12-22  6:38             ` Peng Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-22  6:11 UTC (permalink / raw)
  To: u-boot

On 12/22/2016 07:03 AM, Peng Fan wrote:
> 
> Hi Marek,
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, December 22, 2016 12:40 PM
>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 02:13 AM, Peng Fan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Wednesday, December 21, 2016 10:10 PM
>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>
>>>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>>>> Add usb support for mx6sllevk board.
>>>>>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>>> ---
>>> [......]
>>>
>>>>> +
>>>>> +#define USB_OTHERREGS_OFFSET   0x800
>>>>> +#define UCTRL_PWR_POL          (1 << 9)
>>>>> +
>>>>> +int board_ehci_hcd_init(int port)
>>>>> +{
>>>>> +	u32 *usbnc_usb_ctrl;
>>>>> +
>>>>> +	if (port > 1)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
>>>> +
>>>>> +				 port * 4);
>>>>> +
>>>>> +	/* Set Power polarity */
>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>>>> +	return 0;
>>>>> +}
>>>>
>>>> Is this function similar to what usb_oc_config() does ?
>>>
>>> No, this bit is not for overcurrent. According to RM, this is OTG
>>> Power Polarity This bit should be set according to power switch's enable
>> polarity.
>>> 1 Power switch has an active-high enable input
>>> 0 Power switch has an active-low enable input
>>>
>>> This is board specific.
>>
>> Great, except it should also be part of the driver , the same way as polarity is
>> configured, yes ?
> 
> I just found that there is code for mx7, http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> 
> Then do you agree I add such piece code in usb_power_config for mx6?
> "
> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>         setbits_le32(ctrl, UCTRL_PWR_POL); 
> #else 
>         clrbits_le32(ctrl, UCTRL_PWR_POL);        
> #endif
> "

Didn't you add DT support in a patch sent like ... yesterday ? :) Just
use DT property instead of ifdef.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  6:11           ` Marek Vasut
@ 2016-12-22  6:38             ` Peng Fan
  2016-12-22  7:11               ` Marek Vasut
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2016-12-22  6:38 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, December 22, 2016 2:12 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 07:03 AM, Peng Fan wrote:
> >
> > Hi Marek,
> >
> >> -----Original Message-----
> >> From: Marek Vasut [mailto:marex at denx.de]
> >> Sent: Thursday, December 22, 2016 12:40 PM
> >> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/22/2016 02:13 AM, Peng Fan wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Marek Vasut [mailto:marex at denx.de]
> >>>> Sent: Wednesday, December 21, 2016 10:10 PM
> >>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>>>
> >>>> On 12/21/2016 09:14 AM, Peng Fan wrote:
> >>>>> Add usb support for mx6sllevk board.
> >>>>>
> >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>>>> Cc: Stefano Babic <sbabic@denx.de>
> >>>>> ---
> >>> [......]
> >>>
> >>>>> +
> >>>>> +#define USB_OTHERREGS_OFFSET   0x800
> >>>>> +#define UCTRL_PWR_POL          (1 << 9)
> >>>>> +
> >>>>> +int board_ehci_hcd_init(int port) {
> >>>>> +	u32 *usbnc_usb_ctrl;
> >>>>> +
> >>>>> +	if (port > 1)
> >>>>> +		return -EINVAL;
> >>>>> +
> >>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> USB_OTHERREGS_OFFSET
> >>>> +
> >>>>> +				 port * 4);
> >>>>> +
> >>>>> +	/* Set Power polarity */
> >>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>>>> +	return 0;
> >>>>> +}
> >>>>
> >>>> Is this function similar to what usb_oc_config() does ?
> >>>
> >>> No, this bit is not for overcurrent. According to RM, this is OTG
> >>> Power Polarity This bit should be set according to power switch's
> >>> enable
> >> polarity.
> >>> 1 Power switch has an active-high enable input
> >>> 0 Power switch has an active-low enable input
> >>>
> >>> This is board specific.
> >>
> >> Great, except it should also be part of the driver , the same way as
> >> polarity is configured, yes ?
> >
> > I just found that there is code for mx7,
> > http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >
> > Then do you agree I add such piece code in usb_power_config for mx6?
> > "
> > #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >         setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >         clrbits_le32(ctrl, UCTRL_PWR_POL);
> > #endif
> > "
> 
> Didn't you add DT support in a patch sent like ... yesterday ? :) Just use DT
> property instead of ifdef.

There is no property for otg power polatiry in upstream Linux. I would not like to introduce one
in U-Boot. We can drop the ifdef when there is an property in upstream. What do you think?

Regards,
Peng.

> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  6:38             ` Peng Fan
@ 2016-12-22  7:11               ` Marek Vasut
  2016-12-22  8:38                 ` Peng Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-22  7:11 UTC (permalink / raw)
  To: u-boot

On 12/22/2016 07:38 AM, Peng Fan wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, December 22, 2016 2:12 PM
>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 07:03 AM, Peng Fan wrote:
>>>
>>> Hi Marek,
>>>
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Thursday, December 22, 2016 12:40 PM
>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>
>>>> On 12/22/2016 02:13 AM, Peng Fan wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>> Sent: Wednesday, December 21, 2016 10:10 PM
>>>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>>>
>>>>>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>>>>>> Add usb support for mx6sllevk board.
>>>>>>>
>>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>>>>> ---
>>>>> [......]
>>>>>
>>>>>>> +
>>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
>>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
>>>>>>> +
>>>>>>> +int board_ehci_hcd_init(int port) {
>>>>>>> +	u32 *usbnc_usb_ctrl;
>>>>>>> +
>>>>>>> +	if (port > 1)
>>>>>>> +		return -EINVAL;
>>>>>>> +
>>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>> USB_OTHERREGS_OFFSET
>>>>>> +
>>>>>>> +				 port * 4);
>>>>>>> +
>>>>>>> +	/* Set Power polarity */
>>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>
>>>>>> Is this function similar to what usb_oc_config() does ?
>>>>>
>>>>> No, this bit is not for overcurrent. According to RM, this is OTG
>>>>> Power Polarity This bit should be set according to power switch's
>>>>> enable
>>>> polarity.
>>>>> 1 Power switch has an active-high enable input
>>>>> 0 Power switch has an active-low enable input
>>>>>
>>>>> This is board specific.
>>>>
>>>> Great, except it should also be part of the driver , the same way as
>>>> polarity is configured, yes ?
>>>
>>> I just found that there is code for mx7,
>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>>>
>>> Then do you agree I add such piece code in usb_power_config for mx6?
>>> "
>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
>>>         clrbits_le32(ctrl, UCTRL_PWR_POL);
>>> #endif
>>> "
>>
>> Didn't you add DT support in a patch sent like ... yesterday ? :) Just use DT
>> property instead of ifdef.
> 
> There is no property for otg power polatiry in upstream Linux. I would not like to introduce one
> in U-Boot. We can drop the ifdef when there is an property in upstream. What do you think?

So uh, how can the USB work in mainline Linux on that board ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  7:11               ` Marek Vasut
@ 2016-12-22  8:38                 ` Peng Fan
  2016-12-22  8:46                   ` Marek Vasut
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2016-12-22  8:38 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, December 22, 2016 3:12 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 07:38 AM, Peng Fan wrote:
> >
> >
> >> -----Original Message-----
> >> From: Marek Vasut [mailto:marex at denx.de]
> >> Sent: Thursday, December 22, 2016 2:12 PM
> >> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/22/2016 07:03 AM, Peng Fan wrote:
> >>>
> >>> Hi Marek,
> >>>
> >>>> -----Original Message-----
> >>>> From: Marek Vasut [mailto:marex at denx.de]
> >>>> Sent: Thursday, December 22, 2016 12:40 PM
> >>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>>>
> >>>> On 12/22/2016 02:13 AM, Peng Fan wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Marek Vasut [mailto:marex at denx.de]
> >>>>>> Sent: Wednesday, December 21, 2016 10:10 PM
> >>>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> >>>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> >>>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>>>>>
> >>>>>> On 12/21/2016 09:14 AM, Peng Fan wrote:
> >>>>>>> Add usb support for mx6sllevk board.
> >>>>>>>
> >>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>>>>>> Cc: Stefano Babic <sbabic@denx.de>
> >>>>>>> ---
> >>>>> [......]
> >>>>>
> >>>>>>> +
> >>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
> >>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
> >>>>>>> +
> >>>>>>> +int board_ehci_hcd_init(int port) {
> >>>>>>> +	u32 *usbnc_usb_ctrl;
> >>>>>>> +
> >>>>>>> +	if (port > 1)
> >>>>>>> +		return -EINVAL;
> >>>>>>> +
> >>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> >> USB_OTHERREGS_OFFSET
> >>>>>> +
> >>>>>>> +				 port * 4);
> >>>>>>> +
> >>>>>>> +	/* Set Power polarity */
> >>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>>>>>> +	return 0;
> >>>>>>> +}
> >>>>>>
> >>>>>> Is this function similar to what usb_oc_config() does ?
> >>>>>
> >>>>> No, this bit is not for overcurrent. According to RM, this is OTG
> >>>>> Power Polarity This bit should be set according to power switch's
> >>>>> enable
> >>>> polarity.
> >>>>> 1 Power switch has an active-high enable input
> >>>>> 0 Power switch has an active-low enable input
> >>>>>
> >>>>> This is board specific.
> >>>>
> >>>> Great, except it should also be part of the driver , the same way
> >>>> as polarity is configured, yes ?
> >>>
> >>> I just found that there is code for mx7,
> >>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >>>
> >>> Then do you agree I add such piece code in usb_power_config for mx6?
> >>> "
> >>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >>>         clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> >>
> >> Didn't you add DT support in a patch sent like ... yesterday ? :)
> >> Just use DT property instead of ifdef.
> >
> > There is no property for otg power polatiry in upstream Linux. I would
> > not like to introduce one in U-Boot. We can drop the ifdef when there is an
> property in upstream. What do you think?
> 
> So uh, how can the USB work in mainline Linux on that board ?

Confirmed with IC, if using gpio to control vbus power, no need to configure this bit.
Since in dts, we are using gpio regulator to control vbus, this piece code in patch 3/3
can be discarded. I'll drop it in V2.

Thanks,
Peng.

> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  8:38                 ` Peng Fan
@ 2016-12-22  8:46                   ` Marek Vasut
  2016-12-22  8:55                     ` Peng Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-22  8:46 UTC (permalink / raw)
  To: u-boot

On 12/22/2016 09:38 AM, Peng Fan wrote:
> 
> 
>> -----Original Message-----
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Thursday, December 22, 2016 3:12 PM
>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 07:38 AM, Peng Fan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Thursday, December 22, 2016 2:12 PM
>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>
>>>> On 12/22/2016 07:03 AM, Peng Fan wrote:
>>>>>
>>>>> Hi Marek,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>> Sent: Thursday, December 22, 2016 12:40 PM
>>>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>>>
>>>>>> On 12/22/2016 02:13 AM, Peng Fan wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>>>> Sent: Wednesday, December 21, 2016 10:10 PM
>>>>>>>> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
>>>>>>>> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
>>>>>>>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>>>>>>>
>>>>>>>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>>>>>>>> Add usb support for mx6sllevk board.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>>>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>>>>>>> ---
>>>>>>> [......]
>>>>>>>
>>>>>>>>> +
>>>>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
>>>>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
>>>>>>>>> +
>>>>>>>>> +int board_ehci_hcd_init(int port) {
>>>>>>>>> +	u32 *usbnc_usb_ctrl;
>>>>>>>>> +
>>>>>>>>> +	if (port > 1)
>>>>>>>>> +		return -EINVAL;
>>>>>>>>> +
>>>>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>>>> USB_OTHERREGS_OFFSET
>>>>>>>> +
>>>>>>>>> +				 port * 4);
>>>>>>>>> +
>>>>>>>>> +	/* Set Power polarity */
>>>>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>>>>>>>> +	return 0;
>>>>>>>>> +}
>>>>>>>>
>>>>>>>> Is this function similar to what usb_oc_config() does ?
>>>>>>>
>>>>>>> No, this bit is not for overcurrent. According to RM, this is OTG
>>>>>>> Power Polarity This bit should be set according to power switch's
>>>>>>> enable
>>>>>> polarity.
>>>>>>> 1 Power switch has an active-high enable input
>>>>>>> 0 Power switch has an active-low enable input
>>>>>>>
>>>>>>> This is board specific.
>>>>>>
>>>>>> Great, except it should also be part of the driver , the same way
>>>>>> as polarity is configured, yes ?
>>>>>
>>>>> I just found that there is code for mx7,
>>>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>>>>>
>>>>> Then do you agree I add such piece code in usb_power_config for mx6?
>>>>> "
>>>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>>>>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
>>>>>         clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
>>>>
>>>> Didn't you add DT support in a patch sent like ... yesterday ? :)
>>>> Just use DT property instead of ifdef.
>>>
>>> There is no property for otg power polatiry in upstream Linux. I would
>>> not like to introduce one in U-Boot. We can drop the ifdef when there is an
>> property in upstream. What do you think?
>>
>> So uh, how can the USB work in mainline Linux on that board ?
> 
> Confirmed with IC, if using gpio to control vbus power, no need to configure this bit.
> Since in dts, we are using gpio regulator to control vbus, this piece code in patch 3/3
> can be discarded. I'll drop it in V2.

Hum, OK. Let's revisit this when we need this then ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  8:46                   ` Marek Vasut
@ 2016-12-22  8:55                     ` Peng Fan
  2016-12-22  8:58                       ` Marek Vasut
  0 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2016-12-22  8:55 UTC (permalink / raw)
  To: u-boot



> >>>>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
> >>>>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
> >>>>>>>>> +
> >>>>>>>>> +int board_ehci_hcd_init(int port) {
> >>>>>>>>> +	u32 *usbnc_usb_ctrl;
> >>>>>>>>> +
> >>>>>>>>> +	if (port > 1)
> >>>>>>>>> +		return -EINVAL;
> >>>>>>>>> +
> >>>>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> >>>> USB_OTHERREGS_OFFSET
> >>>>>>>> +
> >>>>>>>>> +				 port * 4);
> >>>>>>>>> +
> >>>>>>>>> +	/* Set Power polarity */
> >>>>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>>>>>>>> +	return 0;
> >>>>>>>>> +}
> >>>>>>>>
> >>>>>>>> Is this function similar to what usb_oc_config() does ?
> >>>>>>>
> >>>>>>> No, this bit is not for overcurrent. According to RM, this is
> >>>>>>> OTG Power Polarity This bit should be set according to power
> >>>>>>> switch's enable
> >>>>>> polarity.
> >>>>>>> 1 Power switch has an active-high enable input
> >>>>>>> 0 Power switch has an active-low enable input
> >>>>>>>
> >>>>>>> This is board specific.
> >>>>>>
> >>>>>> Great, except it should also be part of the driver , the same way
> >>>>>> as polarity is configured, yes ?
> >>>>>
> >>>>> I just found that there is code for mx7,
> >>>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >>>>>
> >>>>> Then do you agree I add such piece code in usb_power_config for mx6?
> >>>>> "
> >>>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >>>>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >>>>>         clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> >>>>
> >>>> Didn't you add DT support in a patch sent like ... yesterday ? :)
> >>>> Just use DT property instead of ifdef.
> >>>
> >>> There is no property for otg power polatiry in upstream Linux. I
> >>> would not like to introduce one in U-Boot. We can drop the ifdef
> >>> when there is an
> >> property in upstream. What do you think?
> >>
> >> So uh, how can the USB work in mainline Linux on that board ?
> >
> > Confirmed with IC, if using gpio to control vbus power, no need to configure
> this bit.
> > Since in dts, we are using gpio regulator to control vbus, this piece
> > code in patch 3/3 can be discarded. I'll drop it in V2.
> 
> Hum, OK. Let's revisit this when we need this then ?

Take i.MX6SLL as an example,
The pin key_col4 can be configured with the two functions.
[1] MX6_PAD_KEY_COL4__USB_OTG1_PWR 
[2] MX6_PAD_KEY_COL4__GPIO4_IO00

In fsl i.mx uboot code, the common way is to use [1] and configure the polarity to enable the vbus power.
But in kernel, it is configured as [2] and use gpio regulator to enable the vbus power.

This is board specific. There is no upstream property for this bit. When there is an upstream property for this,
We could add it in probe function for those that use [1] in dts pinmux.

Now I am using the same dts from kernel which this pinmux configured as [2], so this piece code is not needed for now.

Thanks,
Peng.
> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  8:55                     ` Peng Fan
@ 2016-12-22  8:58                       ` Marek Vasut
  2016-12-22  9:04                         ` Peng Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Marek Vasut @ 2016-12-22  8:58 UTC (permalink / raw)
  To: u-boot

On 12/22/2016 09:55 AM, Peng Fan wrote:
> 
> 
>>>>>>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
>>>>>>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
>>>>>>>>>>> +
>>>>>>>>>>> +int board_ehci_hcd_init(int port) {
>>>>>>>>>>> +	u32 *usbnc_usb_ctrl;
>>>>>>>>>>> +
>>>>>>>>>>> +	if (port > 1)
>>>>>>>>>>> +		return -EINVAL;
>>>>>>>>>>> +
>>>>>>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>>>>>> USB_OTHERREGS_OFFSET
>>>>>>>>>> +
>>>>>>>>>>> +				 port * 4);
>>>>>>>>>>> +
>>>>>>>>>>> +	/* Set Power polarity */
>>>>>>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>>>>>>>>>> +	return 0;
>>>>>>>>>>> +}
>>>>>>>>>>
>>>>>>>>>> Is this function similar to what usb_oc_config() does ?
>>>>>>>>>
>>>>>>>>> No, this bit is not for overcurrent. According to RM, this is
>>>>>>>>> OTG Power Polarity This bit should be set according to power
>>>>>>>>> switch's enable
>>>>>>>> polarity.
>>>>>>>>> 1 Power switch has an active-high enable input
>>>>>>>>> 0 Power switch has an active-low enable input
>>>>>>>>>
>>>>>>>>> This is board specific.
>>>>>>>>
>>>>>>>> Great, except it should also be part of the driver , the same way
>>>>>>>> as polarity is configured, yes ?
>>>>>>>
>>>>>>> I just found that there is code for mx7,
>>>>>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>>>>>>>
>>>>>>> Then do you agree I add such piece code in usb_power_config for mx6?
>>>>>>> "
>>>>>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>>>>>>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
>>>>>>>         clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
>>>>>>
>>>>>> Didn't you add DT support in a patch sent like ... yesterday ? :)
>>>>>> Just use DT property instead of ifdef.
>>>>>
>>>>> There is no property for otg power polatiry in upstream Linux. I
>>>>> would not like to introduce one in U-Boot. We can drop the ifdef
>>>>> when there is an
>>>> property in upstream. What do you think?
>>>>
>>>> So uh, how can the USB work in mainline Linux on that board ?
>>>
>>> Confirmed with IC, if using gpio to control vbus power, no need to configure
>> this bit.
>>> Since in dts, we are using gpio regulator to control vbus, this piece
>>> code in patch 3/3 can be discarded. I'll drop it in V2.
>>
>> Hum, OK. Let's revisit this when we need this then ?
> 
> Take i.MX6SLL as an example,
> The pin key_col4 can be configured with the two functions.
> [1] MX6_PAD_KEY_COL4__USB_OTG1_PWR 
> [2] MX6_PAD_KEY_COL4__GPIO4_IO00
> 
> In fsl i.mx uboot code, the common way is to use [1] and configure the polarity to enable the vbus power.
> But in kernel, it is configured as [2] and use gpio regulator to enable the vbus power.
> 
> This is board specific. There is no upstream property for this bit. When there is an upstream property for this,
> We could add it in probe function for those that use [1] in dts pinmux.
> 
> Now I am using the same dts from kernel which this pinmux configured as [2], so this piece code is not needed for now.

Well, can you submit a proposal with the new prop for devicetree@ ?
It can then be improved in linux too ...

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
  2016-12-22  8:58                       ` Marek Vasut
@ 2016-12-22  9:04                         ` Peng Fan
  0 siblings, 0 replies; 19+ messages in thread
From: Peng Fan @ 2016-12-22  9:04 UTC (permalink / raw)
  To: u-boot


Hi Marek,
> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, December 22, 2016 4:58 PM
> To: Peng Fan <peng.fan@nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 09:55 AM, Peng Fan wrote:
> >
> >
> >>>>>>>>>>> +#define USB_OTHERREGS_OFFSET   0x800
> >>>>>>>>>>> +#define UCTRL_PWR_POL          (1 << 9)
> >>>>>>>>>>> +
> >>>>>>>>>>> +int board_ehci_hcd_init(int port) {
> >>>>>>>>>>> +	u32 *usbnc_usb_ctrl;
> >>>>>>>>>>> +
> >>>>>>>>>>> +	if (port > 1)
> >>>>>>>>>>> +		return -EINVAL;
> >>>>>>>>>>> +
> >>>>>>>>>>> +	usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> >>>>>> USB_OTHERREGS_OFFSET
> >>>>>>>>>> +
> >>>>>>>>>>> +				 port * 4);
> >>>>>>>>>>> +
> >>>>>>>>>>> +	/* Set Power polarity */
> >>>>>>>>>>> +	setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>>>>>>>>>> +	return 0;
> >>>>>>>>>>> +}
> >>>>>>>>>>
> >>>>>>>>>> Is this function similar to what usb_oc_config() does ?
> >>>>>>>>>
> >>>>>>>>> No, this bit is not for overcurrent. According to RM, this is
> >>>>>>>>> OTG Power Polarity This bit should be set according to power
> >>>>>>>>> switch's enable
> >>>>>>>> polarity.
> >>>>>>>>> 1 Power switch has an active-high enable input
> >>>>>>>>> 0 Power switch has an active-low enable input
> >>>>>>>>>
> >>>>>>>>> This is board specific.
> >>>>>>>>
> >>>>>>>> Great, except it should also be part of the driver , the same
> >>>>>>>> way as polarity is configured, yes ?
> >>>>>>>
> >>>>>>> I just found that there is code for mx7,
> >>>>>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >>>>>>>
> >>>>>>> Then do you agree I add such piece code in usb_power_config for
> mx6?
> >>>>>>> "
> >>>>>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >>>>>>>         setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >>>>>>>         clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> >>>>>>
> >>>>>> Didn't you add DT support in a patch sent like ... yesterday ? :)
> >>>>>> Just use DT property instead of ifdef.
> >>>>>
> >>>>> There is no property for otg power polatiry in upstream Linux. I
> >>>>> would not like to introduce one in U-Boot. We can drop the ifdef
> >>>>> when there is an
> >>>> property in upstream. What do you think?
> >>>>
> >>>> So uh, how can the USB work in mainline Linux on that board ?
> >>>
> >>> Confirmed with IC, if using gpio to control vbus power, no need to
> >>> configure
> >> this bit.
> >>> Since in dts, we are using gpio regulator to control vbus, this
> >>> piece code in patch 3/3 can be discarded. I'll drop it in V2.
> >>
> >> Hum, OK. Let's revisit this when we need this then ?
> >
> > Take i.MX6SLL as an example,
> > The pin key_col4 can be configured with the two functions.
> > [1] MX6_PAD_KEY_COL4__USB_OTG1_PWR
> > [2] MX6_PAD_KEY_COL4__GPIO4_IO00
> >
> > In fsl i.mx uboot code, the common way is to use [1] and configure the
> polarity to enable the vbus power.
> > But in kernel, it is configured as [2] and use gpio regulator to enable the vbus
> power.
> >
> > This is board specific. There is no upstream property for this bit.
> > When there is an upstream property for this, We could add it in probe
> function for those that use [1] in dts pinmux.
> >
> > Now I am using the same dts from kernel which this pinmux configured as [2],
> so this piece code is not needed for now.
> 
> Well, can you submit a proposal with the new prop for devicetree@ ?
> It can then be improved in linux too ...

I'll talk with our internal Linux usb maintainer to see whether he has plan to do this.

Thanks,
Peng.
> 
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-12-22  9:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-21  8:14 [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Peng Fan
2016-12-21  8:14 ` [U-Boot] [PATCH 2/3] usb: ehci-mx6: handle vbus-supply Peng Fan
2016-12-21  8:14 ` [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support Peng Fan
2016-12-21 14:01   ` Jagan Teki
2016-12-22  1:16     ` Peng Fan
2016-12-21 14:09   ` Marek Vasut
2016-12-22  1:13     ` Peng Fan
2016-12-22  4:39       ` Marek Vasut
2016-12-22  6:03         ` Peng Fan
2016-12-22  6:11           ` Marek Vasut
2016-12-22  6:38             ` Peng Fan
2016-12-22  7:11               ` Marek Vasut
2016-12-22  8:38                 ` Peng Fan
2016-12-22  8:46                   ` Marek Vasut
2016-12-22  8:55                     ` Peng Fan
2016-12-22  8:58                       ` Marek Vasut
2016-12-22  9:04                         ` Peng Fan
2016-12-21 14:07 ` [U-Boot] [PATCH 1/3] usb: ehci-mx6: implement ofdata_to_platdata Marek Vasut
2016-12-22  1:20   ` Peng Fan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.