All of lore.kernel.org
 help / color / mirror / Atom feed
* [v2,1/3] usb: chipidea: imx: support configuring for active low oc signal
@ 2018-12-02 21:33 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, linux-arm-kernel, kernel, Matthew Starr

The status quo on i.MX6 is that if "over-current-active-high" is
specified in the device tree this is configured as expected. If
the property is missing polarity isn't changed and so the
polarity is kept as setup by the bootloader. Reset default is
active high, so active low can only be used with help by the
bootloader. On i.MX7 it is similar, but there disabling of
over current detection has a similar inconsistency.

This patch introduces a new property that allows to explicitly
configure for active low over current detection and consistently
sets this up. In the absence of an explicit configuration the
bit is kept as is. On i.MX7 over current detection is used unless
disabled in the device tree.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
 drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
 drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
 drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 529e51879fb2..c32f6e983cf6 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -87,8 +87,9 @@ i.mx specific properties
 - fsl,usbmisc: phandler of non-core register device, with one
   argument that indicate usb controller index
 - disable-over-current: disable over current detect
-- over-current-active-high: over current signal polarity is high active,
-  typically over current signal polarity is low active.
+- over-current-active-low: over current signal polarity is active low.
+- over-current-active-high: over current signal polarity is active high.
+  It's recommended to specify the over current polarity.
 - external-vbus-divider: enables off-chip resistor divider for Vbus
 
 Example:
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 09b37c0d075d..80b4e4ef9b68 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	if (of_find_property(np, "disable-over-current", NULL))
 		data->disable_oc = 1;
 
-	if (of_find_property(np, "over-current-active-high", NULL))
-		data->oc_polarity = 1;
+	if (of_find_property(np, "over-current-active-high", NULL)) {
+		data->oc_pol_active_low = 0;
+		data->oc_pol_configured = 1;
+	} else if (of_find_property(np, "over-current-active-low", NULL)) {
+		data->oc_pol_active_low = 1;
+		data->oc_pol_configured = 1;
+	}
 
 	if (of_find_property(np, "external-vbus-divider", NULL))
 		data->evdo = 1;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 204275f47573..640721fef0d7 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -11,7 +11,13 @@ struct imx_usbmisc_data {
 	int index;
 
 	unsigned int disable_oc:1; /* over current detect disabled */
-	unsigned int oc_polarity:1; /* over current polarity if oc enabled */
+
+	/* true if over-current polarity is active low */
+	unsigned int oc_pol_active_low:1;
+
+	/* true if dt specifies polarity */
+	unsigned int oc_pol_configured:1;
+
 	unsigned int evdo:1; /* set external vbus divider option */
 	unsigned int ulpi:1; /* connected to an ULPI phy */
 };
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index def80ff547e4..4a8de1b37f67 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -340,11 +340,17 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
 	reg = readl(usbmisc->base + data->index * 4);
 	if (data->disable_oc) {
 		reg |= MX6_BM_OVER_CUR_DIS;
-	} else if (data->oc_polarity == 1) {
-		/* High active */
-		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
 	} else {
-		reg &= ~(MX6_BM_OVER_CUR_DIS);
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+
+		/*
+		 * If the polarity is not configured keep it as setup by the
+		 * bootloader.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			reg |= MX6_BM_OVER_CUR_POLARITY;
+		else if (data->oc_pol_configured)
+			reg &= ~MX6_BM_OVER_CUR_POLARITY;
 	}
 	writel(reg, usbmisc->base + data->index * 4);
 
@@ -444,9 +450,17 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 	reg = readl(usbmisc->base);
 	if (data->disable_oc) {
 		reg |= MX6_BM_OVER_CUR_DIS;
-	} else if (data->oc_polarity == 1) {
-		/* High active */
-		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
+	} else {
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+
+		/*
+		 * If the polarity is not configured keep it as setup by the
+		 * bootloader.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			reg |= MX6_BM_OVER_CUR_POLARITY;
+		else if (data->oc_pol_configured)
+			reg &= ~MX6_BM_OVER_CUR_POLARITY;
 	}
 	writel(reg, usbmisc->base);
 

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

* [PATCH v2 1/3] usb: chipidea: imx: support configuring for active low oc signal
@ 2018-12-02 21:33 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, Matthew Starr, kernel, linux-arm-kernel

The status quo on i.MX6 is that if "over-current-active-high" is
specified in the device tree this is configured as expected. If
the property is missing polarity isn't changed and so the
polarity is kept as setup by the bootloader. Reset default is
active high, so active low can only be used with help by the
bootloader. On i.MX7 it is similar, but there disabling of
over current detection has a similar inconsistency.

This patch introduces a new property that allows to explicitly
configure for active low over current detection and consistently
sets this up. In the absence of an explicit configuration the
bit is kept as is. On i.MX7 over current detection is used unless
disabled in the device tree.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
 drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
 drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
 drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 529e51879fb2..c32f6e983cf6 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -87,8 +87,9 @@ i.mx specific properties
 - fsl,usbmisc: phandler of non-core register device, with one
   argument that indicate usb controller index
 - disable-over-current: disable over current detect
-- over-current-active-high: over current signal polarity is high active,
-  typically over current signal polarity is low active.
+- over-current-active-low: over current signal polarity is active low.
+- over-current-active-high: over current signal polarity is active high.
+  It's recommended to specify the over current polarity.
 - external-vbus-divider: enables off-chip resistor divider for Vbus
 
 Example:
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 09b37c0d075d..80b4e4ef9b68 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	if (of_find_property(np, "disable-over-current", NULL))
 		data->disable_oc = 1;
 
-	if (of_find_property(np, "over-current-active-high", NULL))
-		data->oc_polarity = 1;
+	if (of_find_property(np, "over-current-active-high", NULL)) {
+		data->oc_pol_active_low = 0;
+		data->oc_pol_configured = 1;
+	} else if (of_find_property(np, "over-current-active-low", NULL)) {
+		data->oc_pol_active_low = 1;
+		data->oc_pol_configured = 1;
+	}
 
 	if (of_find_property(np, "external-vbus-divider", NULL))
 		data->evdo = 1;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 204275f47573..640721fef0d7 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -11,7 +11,13 @@ struct imx_usbmisc_data {
 	int index;
 
 	unsigned int disable_oc:1; /* over current detect disabled */
-	unsigned int oc_polarity:1; /* over current polarity if oc enabled */
+
+	/* true if over-current polarity is active low */
+	unsigned int oc_pol_active_low:1;
+
+	/* true if dt specifies polarity */
+	unsigned int oc_pol_configured:1;
+
 	unsigned int evdo:1; /* set external vbus divider option */
 	unsigned int ulpi:1; /* connected to an ULPI phy */
 };
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index def80ff547e4..4a8de1b37f67 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -340,11 +340,17 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
 	reg = readl(usbmisc->base + data->index * 4);
 	if (data->disable_oc) {
 		reg |= MX6_BM_OVER_CUR_DIS;
-	} else if (data->oc_polarity == 1) {
-		/* High active */
-		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
 	} else {
-		reg &= ~(MX6_BM_OVER_CUR_DIS);
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+
+		/*
+		 * If the polarity is not configured keep it as setup by the
+		 * bootloader.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			reg |= MX6_BM_OVER_CUR_POLARITY;
+		else if (data->oc_pol_configured)
+			reg &= ~MX6_BM_OVER_CUR_POLARITY;
 	}
 	writel(reg, usbmisc->base + data->index * 4);
 
@@ -444,9 +450,17 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 	reg = readl(usbmisc->base);
 	if (data->disable_oc) {
 		reg |= MX6_BM_OVER_CUR_DIS;
-	} else if (data->oc_polarity == 1) {
-		/* High active */
-		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
+	} else {
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+
+		/*
+		 * If the polarity is not configured keep it as setup by the
+		 * bootloader.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			reg |= MX6_BM_OVER_CUR_POLARITY;
+		else if (data->oc_pol_configured)
+			reg &= ~MX6_BM_OVER_CUR_POLARITY;
 	}
 	writel(reg, usbmisc->base);
 
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
  2018-12-02 21:33 ` [PATCH v2 1/3] " Uwe Kleine-König
@ 2018-12-02 21:33 ` Uwe Kleine-König
  -1 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, linux-arm-kernel, kernel, Matthew Starr

The polarity of the over current detection pin isn't configured on i.MX6/7
if it's unspecified in the device tree. So the actual configuration depends
on bootloader behavior which is bad.

So encourage users to fix their device tree by issuing a warning in this
case.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 80b4e4ef9b68..3dcfd0d97f94 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -141,6 +141,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	} else if (of_find_property(np, "over-current-active-low", NULL)) {
 		data->oc_pol_active_low = 1;
 		data->oc_pol_configured = 1;
+	} else {
+		dev_warn(dev, "No over current polarity defined\n");
 	}
 
 	if (of_find_property(np, "external-vbus-divider", NULL))

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

* [PATCH v2 2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
@ 2018-12-02 21:33 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, Matthew Starr, kernel, linux-arm-kernel

The polarity of the over current detection pin isn't configured on i.MX6/7
if it's unspecified in the device tree. So the actual configuration depends
on bootloader behavior which is bad.

So encourage users to fix their device tree by issuing a warning in this
case.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 80b4e4ef9b68..3dcfd0d97f94 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -141,6 +141,8 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
 	} else if (of_find_property(np, "over-current-active-low", NULL)) {
 		data->oc_pol_active_low = 1;
 		data->oc_pol_configured = 1;
+	} else {
+		dev_warn(dev, "No over current polarity defined\n");
 	}
 
 	if (of_find_property(np, "external-vbus-divider", NULL))
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,3/3] usb: chipidea: imx: allow to configure oc polarity on i.MX25
  2018-12-02 21:33 ` [PATCH v2 1/3] " Uwe Kleine-König
@ 2018-12-02 21:33 ` Uwe Kleine-König
  -1 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, linux-arm-kernel, kernel, Matthew Starr

Up to now the polarity of the over current pin was hard coded to active
high. Use the already defined device tree properties to configure polarity
on i.MX25, too. In difference to i.MX6/7 use active high behavior if the
polarity is unspecified to keep compatibility to existing device trees.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 4a8de1b37f67..47eee5cade84 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -120,6 +120,14 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
 		val &= ~(MX25_OTG_SIC_MASK | MX25_OTG_PP_BIT);
 		val |= (MX25_EHCI_INTERFACE_DIFF_UNI & MX25_EHCI_INTERFACE_MASK) << MX25_OTG_SIC_SHIFT;
 		val |= (MX25_OTG_PM_BIT | MX25_OTG_OCPOL_BIT);
+
+		/*
+		 * If the polarity is not configured assume active high for
+		 * historical reasons.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			val &= ~MX25_OTG_OCPOL_BIT;
+
 		writel(val, usbmisc->base);
 		break;
 	case 1:
@@ -129,6 +137,13 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
 		val |= (MX25_H1_PM_BIT | MX25_H1_OCPOL_BIT | MX25_H1_TLL_BIT |
 			MX25_H1_USBTE_BIT | MX25_H1_IPPUE_DOWN_BIT);
 
+		/*
+		 * If the polarity is not configured assume active high for
+		 * historical reasons.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			val &= ~MX25_H1_OCPOL_BIT;
+
 		writel(val, usbmisc->base);
 
 		break;

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

* [PATCH v2 3/3] usb: chipidea: imx: allow to configure oc polarity on i.MX25
@ 2018-12-02 21:33 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-02 21:33 UTC (permalink / raw)
  To: Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, Matthew Starr, kernel, linux-arm-kernel

Up to now the polarity of the over current pin was hard coded to active
high. Use the already defined device tree properties to configure polarity
on i.MX25, too. In difference to i.MX6/7 use active high behavior if the
polarity is unspecified to keep compatibility to existing device trees.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/usb/chipidea/usbmisc_imx.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 4a8de1b37f67..47eee5cade84 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -120,6 +120,14 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
 		val &= ~(MX25_OTG_SIC_MASK | MX25_OTG_PP_BIT);
 		val |= (MX25_EHCI_INTERFACE_DIFF_UNI & MX25_EHCI_INTERFACE_MASK) << MX25_OTG_SIC_SHIFT;
 		val |= (MX25_OTG_PM_BIT | MX25_OTG_OCPOL_BIT);
+
+		/*
+		 * If the polarity is not configured assume active high for
+		 * historical reasons.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			val &= ~MX25_OTG_OCPOL_BIT;
+
 		writel(val, usbmisc->base);
 		break;
 	case 1:
@@ -129,6 +137,13 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
 		val |= (MX25_H1_PM_BIT | MX25_H1_OCPOL_BIT | MX25_H1_TLL_BIT |
 			MX25_H1_USBTE_BIT | MX25_H1_IPPUE_DOWN_BIT);
 
+		/*
+		 * If the polarity is not configured assume active high for
+		 * historical reasons.
+		 */
+		if (data->oc_pol_configured && data->oc_pol_active_low)
+			val &= ~MX25_H1_OCPOL_BIT;
+
 		writel(val, usbmisc->base);
 
 		break;
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,1/3] usb: chipidea: imx: support configuring for active low oc signal
  2018-12-02 21:33 ` [PATCH v2 1/3] " Uwe Kleine-König
@ 2018-12-03 16:10 ` Matthew Starr
  -1 siblings, 0 replies; 14+ messages in thread
From: Matthew Starr @ 2018-12-03 16:10 UTC (permalink / raw)
  To: Uwe Kleine-König, Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, linux-arm-kernel, kernel

> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> Sent: Sunday, December 02, 2018 3:33 PM
> 
> The status quo on i.MX6 is that if "over-current-active-high" is
> specified in the device tree this is configured as expected. If
> the property is missing polarity isn't changed and so the
> polarity is kept as setup by the bootloader. Reset default is
> active high, so active low can only be used with help by the
> bootloader. On i.MX7 it is similar, but there disabling of
> over current detection has a similar inconsistency.
> 
> This patch introduces a new property that allows to explicitly
> configure for active low over current detection and consistently
> sets this up. In the absence of an explicit configuration the
> bit is kept as is. On i.MX7 over current detection is used unless
> disabled in the device tree.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
>  drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
>  drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
>  drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
>  4 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..c32f6e983cf6 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -87,8 +87,9 @@ i.mx specific properties
>  - fsl,usbmisc: phandler of non-core register device, with one
>    argument that indicate usb controller index
>  - disable-over-current: disable over current detect
> -- over-current-active-high: over current signal polarity is high active,
> -  typically over current signal polarity is low active.
> +- over-current-active-low: over current signal polarity is active low.
> +- over-current-active-high: over current signal polarity is active high.
> +  It's recommended to specify the over current polarity.
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
> 
>  Example:
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 09b37c0d075d..80b4e4ef9b68 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>  	if (of_find_property(np, "disable-over-current", NULL))
>  		data->disable_oc = 1;
> 
> -	if (of_find_property(np, "over-current-active-high", NULL))
> -		data->oc_polarity = 1;
> +	if (of_find_property(np, "over-current-active-high", NULL)) {
> +		data->oc_pol_active_low = 0;
> +		data->oc_pol_configured = 1;
> +	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> +		data->oc_pol_active_low = 1;
> +		data->oc_pol_configured = 1;
> +	}

Since both over-current-active-high and over-current-active-low can be enabled, the error case of when both are in the device tree is not covered here.  An error or warning should be given.  Also in this case the safest thing to do would be to leave the OC polarity bit alone and leave it the way it was set by the bootloader.

Matt Starr

> 
>  	if (of_find_property(np, "external-vbus-divider", NULL))
>  		data->evdo = 1;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 204275f47573..640721fef0d7 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -11,7 +11,13 @@ struct imx_usbmisc_data {
>  	int index;
> 
>  	unsigned int disable_oc:1; /* over current detect disabled */
> -	unsigned int oc_polarity:1; /* over current polarity if oc enabled */
> +
> +	/* true if over-current polarity is active low */
> +	unsigned int oc_pol_active_low:1;
> +
> +	/* true if dt specifies polarity */
> +	unsigned int oc_pol_configured:1;
> +
>  	unsigned int evdo:1; /* set external vbus divider option */
>  	unsigned int ulpi:1; /* connected to an ULPI phy */
>  };
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index def80ff547e4..4a8de1b37f67 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -340,11 +340,17 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
>  	reg = readl(usbmisc->base + data->index * 4);
>  	if (data->disable_oc) {
>  		reg |= MX6_BM_OVER_CUR_DIS;
> -	} else if (data->oc_polarity == 1) {
> -		/* High active */
> -		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
>  	} else {
> -		reg &= ~(MX6_BM_OVER_CUR_DIS);
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +
> +		/*
> +		 * If the polarity is not configured keep it as setup by the
> +		 * bootloader.
> +		 */
> +		if (data->oc_pol_configured && data->oc_pol_active_low)
> +			reg |= MX6_BM_OVER_CUR_POLARITY;
> +		else if (data->oc_pol_configured)
> +			reg &= ~MX6_BM_OVER_CUR_POLARITY;
>  	}
>  	writel(reg, usbmisc->base + data->index * 4);
> 
> @@ -444,9 +450,17 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>  	reg = readl(usbmisc->base);
>  	if (data->disable_oc) {
>  		reg |= MX6_BM_OVER_CUR_DIS;
> -	} else if (data->oc_polarity == 1) {
> -		/* High active */
> -		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
> +	} else {
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +
> +		/*
> +		 * If the polarity is not configured keep it as setup by the
> +		 * bootloader.
> +		 */
> +		if (data->oc_pol_configured && data->oc_pol_active_low)
> +			reg |= MX6_BM_OVER_CUR_POLARITY;
> +		else if (data->oc_pol_configured)
> +			reg &= ~MX6_BM_OVER_CUR_POLARITY;
>  	}
>  	writel(reg, usbmisc->base);
> 
> --
> 2.19.2

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

* RE: [PATCH v2 1/3] usb: chipidea: imx: support configuring for active low oc signal
@ 2018-12-03 16:10 ` Matthew Starr
  0 siblings, 0 replies; 14+ messages in thread
From: Matthew Starr @ 2018-12-03 16:10 UTC (permalink / raw)
  To: Uwe Kleine-König, Peter Chen, Shawn Guo
  Cc: Fabio Estevam, linux-usb, kernel, linux-arm-kernel

> -----Original Message-----
> From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> Sent: Sunday, December 02, 2018 3:33 PM
> 
> The status quo on i.MX6 is that if "over-current-active-high" is
> specified in the device tree this is configured as expected. If
> the property is missing polarity isn't changed and so the
> polarity is kept as setup by the bootloader. Reset default is
> active high, so active low can only be used with help by the
> bootloader. On i.MX7 it is similar, but there disabling of
> over current detection has a similar inconsistency.
> 
> This patch introduces a new property that allows to explicitly
> configure for active low over current detection and consistently
> sets this up. In the absence of an explicit configuration the
> bit is kept as is. On i.MX7 over current detection is used unless
> disabled in the device tree.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
>  drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
>  drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
>  drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
>  4 files changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 529e51879fb2..c32f6e983cf6 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -87,8 +87,9 @@ i.mx specific properties
>  - fsl,usbmisc: phandler of non-core register device, with one
>    argument that indicate usb controller index
>  - disable-over-current: disable over current detect
> -- over-current-active-high: over current signal polarity is high active,
> -  typically over current signal polarity is low active.
> +- over-current-active-low: over current signal polarity is active low.
> +- over-current-active-high: over current signal polarity is active high.
> +  It's recommended to specify the over current polarity.
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
> 
>  Example:
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 09b37c0d075d..80b4e4ef9b68 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>  	if (of_find_property(np, "disable-over-current", NULL))
>  		data->disable_oc = 1;
> 
> -	if (of_find_property(np, "over-current-active-high", NULL))
> -		data->oc_polarity = 1;
> +	if (of_find_property(np, "over-current-active-high", NULL)) {
> +		data->oc_pol_active_low = 0;
> +		data->oc_pol_configured = 1;
> +	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> +		data->oc_pol_active_low = 1;
> +		data->oc_pol_configured = 1;
> +	}

Since both over-current-active-high and over-current-active-low can be enabled, the error case of when both are in the device tree is not covered here.  An error or warning should be given.  Also in this case the safest thing to do would be to leave the OC polarity bit alone and leave it the way it was set by the bootloader.

Matt Starr

> 
>  	if (of_find_property(np, "external-vbus-divider", NULL))
>  		data->evdo = 1;
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
> index 204275f47573..640721fef0d7 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -11,7 +11,13 @@ struct imx_usbmisc_data {
>  	int index;
> 
>  	unsigned int disable_oc:1; /* over current detect disabled */
> -	unsigned int oc_polarity:1; /* over current polarity if oc enabled */
> +
> +	/* true if over-current polarity is active low */
> +	unsigned int oc_pol_active_low:1;
> +
> +	/* true if dt specifies polarity */
> +	unsigned int oc_pol_configured:1;
> +
>  	unsigned int evdo:1; /* set external vbus divider option */
>  	unsigned int ulpi:1; /* connected to an ULPI phy */
>  };
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index def80ff547e4..4a8de1b37f67 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -340,11 +340,17 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
>  	reg = readl(usbmisc->base + data->index * 4);
>  	if (data->disable_oc) {
>  		reg |= MX6_BM_OVER_CUR_DIS;
> -	} else if (data->oc_polarity == 1) {
> -		/* High active */
> -		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
>  	} else {
> -		reg &= ~(MX6_BM_OVER_CUR_DIS);
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +
> +		/*
> +		 * If the polarity is not configured keep it as setup by the
> +		 * bootloader.
> +		 */
> +		if (data->oc_pol_configured && data->oc_pol_active_low)
> +			reg |= MX6_BM_OVER_CUR_POLARITY;
> +		else if (data->oc_pol_configured)
> +			reg &= ~MX6_BM_OVER_CUR_POLARITY;
>  	}
>  	writel(reg, usbmisc->base + data->index * 4);
> 
> @@ -444,9 +450,17 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>  	reg = readl(usbmisc->base);
>  	if (data->disable_oc) {
>  		reg |= MX6_BM_OVER_CUR_DIS;
> -	} else if (data->oc_polarity == 1) {
> -		/* High active */
> -		reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY);
> +	} else {
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +
> +		/*
> +		 * If the polarity is not configured keep it as setup by the
> +		 * bootloader.
> +		 */
> +		if (data->oc_pol_configured && data->oc_pol_active_low)
> +			reg |= MX6_BM_OVER_CUR_POLARITY;
> +		else if (data->oc_pol_configured)
> +			reg &= ~MX6_BM_OVER_CUR_POLARITY;
>  	}
>  	writel(reg, usbmisc->base);
> 
> --
> 2.19.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,1/3] usb: chipidea: imx: support configuring for active low oc signal
  2018-12-03 16:10 ` [PATCH v2 1/3] " Matthew Starr
@ 2018-12-03 17:22 ` Uwe Kleine-König
  -1 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-03 17:22 UTC (permalink / raw)
  To: Matthew Starr
  Cc: Peter Chen, Shawn Guo, Fabio Estevam, linux-usb,
	linux-arm-kernel, kernel, Rob Herring

On Mon, Dec 03, 2018 at 04:10:37PM +0000, Matthew Starr wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> > Sent: Sunday, December 02, 2018 3:33 PM
> > 
> > The status quo on i.MX6 is that if "over-current-active-high" is
> > specified in the device tree this is configured as expected. If
> > the property is missing polarity isn't changed and so the
> > polarity is kept as setup by the bootloader. Reset default is
> > active high, so active low can only be used with help by the
> > bootloader. On i.MX7 it is similar, but there disabling of
> > over current detection has a similar inconsistency.
> > 
> > This patch introduces a new property that allows to explicitly
> > configure for active low over current detection and consistently
> > sets this up. In the absence of an explicit configuration the
> > bit is kept as is. On i.MX7 over current detection is used unless
> > disabled in the device tree.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
> >  drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
> >  drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
> >  drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
> >  4 files changed, 38 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index 529e51879fb2..c32f6e983cf6 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -87,8 +87,9 @@ i.mx specific properties
> >  - fsl,usbmisc: phandler of non-core register device, with one
> >    argument that indicate usb controller index
> >  - disable-over-current: disable over current detect
> > -- over-current-active-high: over current signal polarity is high active,
> > -  typically over current signal polarity is low active.
> > +- over-current-active-low: over current signal polarity is active low.
> > +- over-current-active-high: over current signal polarity is active high.
> > +  It's recommended to specify the over current polarity.
> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > 
> >  Example:
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 09b37c0d075d..80b4e4ef9b68 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
> >  	if (of_find_property(np, "disable-over-current", NULL))
> >  		data->disable_oc = 1;
> > 
> > -	if (of_find_property(np, "over-current-active-high", NULL))
> > -		data->oc_polarity = 1;
> > +	if (of_find_property(np, "over-current-active-high", NULL)) {
> > +		data->oc_pol_active_low = 0;
> > +		data->oc_pol_configured = 1;
> > +	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> > +		data->oc_pol_active_low = 1;
> > +		data->oc_pol_configured = 1;
> > +	}
> 
> Since both over-current-active-high and over-current-active-low can be
> enabled, the error case of when both are in the device tree is not
> covered here.  An error or warning should be given.  Also in this case
> the safest thing to do would be to leave the OC polarity bit alone and
> leave it the way it was set by the bootloader.

Given that it obviously cannot describe the hardware correctly when both
(over-current-active-high and over-current-active-low) are given it is
IMHO ok to do anything in this case. In my eyes that's similar to the C
language where the compiler is free to do whatever it seems fit when the
programmer does something unspecified.

Added Rob to Cc, maybe he can comment.

Best regards
Uwe

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

* Re: [PATCH v2 1/3] usb: chipidea: imx: support configuring for active low oc signal
@ 2018-12-03 17:22 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-03 17:22 UTC (permalink / raw)
  To: Matthew Starr
  Cc: Peter Chen, Rob Herring, linux-usb, kernel, Fabio Estevam,
	Shawn Guo, linux-arm-kernel

On Mon, Dec 03, 2018 at 04:10:37PM +0000, Matthew Starr wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de]
> > Sent: Sunday, December 02, 2018 3:33 PM
> > 
> > The status quo on i.MX6 is that if "over-current-active-high" is
> > specified in the device tree this is configured as expected. If
> > the property is missing polarity isn't changed and so the
> > polarity is kept as setup by the bootloader. Reset default is
> > active high, so active low can only be used with help by the
> > bootloader. On i.MX7 it is similar, but there disabling of
> > over current detection has a similar inconsistency.
> > 
> > This patch introduces a new property that allows to explicitly
> > configure for active low over current detection and consistently
> > sets this up. In the absence of an explicit configuration the
> > bit is kept as is. On i.MX7 over current detection is used unless
> > disabled in the device tree.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  .../devicetree/bindings/usb/ci-hdrc-usb2.txt  |  5 ++--
> >  drivers/usb/chipidea/ci_hdrc_imx.c            |  9 ++++--
> >  drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
> >  drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
> >  4 files changed, 38 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index 529e51879fb2..c32f6e983cf6 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -87,8 +87,9 @@ i.mx specific properties
> >  - fsl,usbmisc: phandler of non-core register device, with one
> >    argument that indicate usb controller index
> >  - disable-over-current: disable over current detect
> > -- over-current-active-high: over current signal polarity is high active,
> > -  typically over current signal polarity is low active.
> > +- over-current-active-low: over current signal polarity is active low.
> > +- over-current-active-high: over current signal polarity is active high.
> > +  It's recommended to specify the over current polarity.
> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > 
> >  Example:
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 09b37c0d075d..80b4e4ef9b68 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -135,8 +135,13 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
> >  	if (of_find_property(np, "disable-over-current", NULL))
> >  		data->disable_oc = 1;
> > 
> > -	if (of_find_property(np, "over-current-active-high", NULL))
> > -		data->oc_polarity = 1;
> > +	if (of_find_property(np, "over-current-active-high", NULL)) {
> > +		data->oc_pol_active_low = 0;
> > +		data->oc_pol_configured = 1;
> > +	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> > +		data->oc_pol_active_low = 1;
> > +		data->oc_pol_configured = 1;
> > +	}
> 
> Since both over-current-active-high and over-current-active-low can be
> enabled, the error case of when both are in the device tree is not
> covered here.  An error or warning should be given.  Also in this case
> the safest thing to do would be to leave the OC polarity bit alone and
> leave it the way it was set by the bootloader.

Given that it obviously cannot describe the hardware correctly when both
(over-current-active-high and over-current-active-low) are given it is
IMHO ok to do anything in this case. In my eyes that's similar to the C
language where the compiler is free to do whatever it seems fit when the
programmer does something unspecified.

Added Rob to Cc, maybe he can comment.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
  2018-12-02 21:33 ` [PATCH v2 2/3] " Uwe Kleine-König
@ 2018-12-04  6:19 ` PETER CHEN
  -1 siblings, 0 replies; 14+ messages in thread
From: Peter Chen @ 2018-12-04  6:19 UTC (permalink / raw)
  To: Uwe Kleine-König, Shawn Guo
  Cc: Fabio Estevam, linux-usb, linux-arm-kernel, kernel, mstarr

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 80b4e4ef9b68..3dcfd0d97f94 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -141,6 +141,8 @@ static struct imx_usbmisc_data
> *usbmisc_get_init_data(struct device *dev)
>  	} else if (of_find_property(np, "over-current-active-low", NULL)) {
>  		data->oc_pol_active_low = 1;
>  		data->oc_pol_configured = 1;
> +	} else {
> +		dev_warn(dev, "No over current polarity defined\n");
>  	}

If the platform doesn't support OC, the polarity setting is not needed. You could consider
getting polarity property only "disable-over-current" is not found at patch 1.

Peter

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

* RE: [PATCH v2 2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
@ 2018-12-04  6:19 ` PETER CHEN
  0 siblings, 0 replies; 14+ messages in thread
From: PETER CHEN @ 2018-12-04  6:19 UTC (permalink / raw)
  To: Uwe Kleine-König, Shawn Guo
  Cc: Fabio Estevam, linux-usb, mstarr, kernel, linux-arm-kernel

 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 80b4e4ef9b68..3dcfd0d97f94 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -141,6 +141,8 @@ static struct imx_usbmisc_data
> *usbmisc_get_init_data(struct device *dev)
>  	} else if (of_find_property(np, "over-current-active-low", NULL)) {
>  		data->oc_pol_active_low = 1;
>  		data->oc_pol_configured = 1;
> +	} else {
> +		dev_warn(dev, "No over current polarity defined\n");
>  	}

If the platform doesn't support OC, the polarity setting is not needed. You could consider
getting polarity property only "disable-over-current" is not found at patch 1.

Peter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [v2,2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
  2018-12-04  6:19 ` [PATCH v2 2/3] " PETER CHEN
@ 2018-12-04  8:20 ` Uwe Kleine-König
  -1 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-04  8:20 UTC (permalink / raw)
  To: PETER CHEN
  Cc: Shawn Guo, Fabio Estevam, linux-usb, linux-arm-kernel, kernel, mstarr

Hello Peter,

On Tue, Dec 04, 2018 at 06:19:11AM +0000, PETER CHEN wrote:
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 80b4e4ef9b68..3dcfd0d97f94 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -141,6 +141,8 @@ static struct imx_usbmisc_data
> > *usbmisc_get_init_data(struct device *dev)
> >  	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> >  		data->oc_pol_active_low = 1;
> >  		data->oc_pol_configured = 1;
> > +	} else {
> > +		dev_warn(dev, "No over current polarity defined\n");
> >  	}
> 
> If the platform doesn't support OC, the polarity setting is not needed. You could consider
> getting polarity property only "disable-over-current" is not found at patch 1.

Right, just fixed that up and will send out v3 in a moment.

Best regards
Uwe

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

* Re: [PATCH v2 2/3] usb: chipidea: imx: Warn if oc polarity isn't specified
@ 2018-12-04  8:20 ` Uwe Kleine-König
  0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2018-12-04  8:20 UTC (permalink / raw)
  To: PETER CHEN
  Cc: linux-usb, mstarr, kernel, Fabio Estevam, Shawn Guo, linux-arm-kernel

Hello Peter,

On Tue, Dec 04, 2018 at 06:19:11AM +0000, PETER CHEN wrote:
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 80b4e4ef9b68..3dcfd0d97f94 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -141,6 +141,8 @@ static struct imx_usbmisc_data
> > *usbmisc_get_init_data(struct device *dev)
> >  	} else if (of_find_property(np, "over-current-active-low", NULL)) {
> >  		data->oc_pol_active_low = 1;
> >  		data->oc_pol_configured = 1;
> > +	} else {
> > +		dev_warn(dev, "No over current polarity defined\n");
> >  	}
> 
> If the platform doesn't support OC, the polarity setting is not needed. You could consider
> getting polarity property only "disable-over-current" is not found at patch 1.

Right, just fixed that up and will send out v3 in a moment.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2018-12-04  8:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-02 21:33 [v2,3/3] usb: chipidea: imx: allow to configure oc polarity on i.MX25 Uwe Kleine-König
2018-12-02 21:33 ` [PATCH v2 3/3] " Uwe Kleine-König
  -- strict thread matches above, loose matches on Subject: below --
2018-12-04  8:20 [v2,2/3] usb: chipidea: imx: Warn if oc polarity isn't specified Uwe Kleine-König
2018-12-04  8:20 ` [PATCH v2 2/3] " Uwe Kleine-König
2018-12-04  6:19 [v2,2/3] " Peter Chen
2018-12-04  6:19 ` [PATCH v2 2/3] " PETER CHEN
2018-12-03 17:22 [v2,1/3] usb: chipidea: imx: support configuring for active low oc signal Uwe Kleine-König
2018-12-03 17:22 ` [PATCH v2 1/3] " Uwe Kleine-König
2018-12-03 16:10 [v2,1/3] " Matthew Starr
2018-12-03 16:10 ` [PATCH v2 1/3] " Matthew Starr
2018-12-02 21:33 [v2,2/3] usb: chipidea: imx: Warn if oc polarity isn't specified Uwe Kleine-König
2018-12-02 21:33 ` [PATCH v2 2/3] " Uwe Kleine-König
2018-12-02 21:33 [v2,1/3] usb: chipidea: imx: support configuring for active low oc signal Uwe Kleine-König
2018-12-02 21:33 ` [PATCH v2 1/3] " Uwe Kleine-König

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.