[PATCH 1/4] doc: usb: usbmisc-imx: add imx7d compatible string

[PATCH 1/4] doc: usb: usbmisc-imx: add imx7d compatible string

[Li Jun]

Add compatible string for imx7d-usbmisc.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 Documentation/devicetree/bindings/usb/usbmisc-imx.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
index 3539d4e..f1e27fa 100644
--- a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
+++ b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
@@ -6,6 +6,7 @@ Required properties:
 	"fsl,imx6q-usbmisc" for imx6q
 	"fsl,vf610-usbmisc" for Vybrid vf610
 	"fsl,imx6sx-usbmisc" for imx6sx
+	"fsl,imx7d-usbmisc" for imx7d
 - reg: Should contain registers location and length
 
 Examples:
-- 
1.9.1

[PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-current-polarity

[Li Jun]

Adding over-current-polarity to indicate the over current flag
is low active or high active.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
index 341dc67..c5d35f4 100644
--- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
+++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
@@ -81,6 +81,8 @@ 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-polarity: 0 if the over current signal polarity is low active,
+  1 if the over current signal polarity is high active.
 - external-vbus-divider: enables off-chip resistor divider for Vbus
 
 Example:
-- 
1.9.1

[PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-current-polarity

[Joshua Clayton]

Greetings,
On 07/18/2016 04:15 AM, Li Jun wrote:
> Adding over-current-polarity to indicate the over current flag
> is low active or high active.
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 341dc67..c5d35f4 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -81,6 +81,8 @@ 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-polarity: 0 if the over current signal polarity is low active,
> +  1 if the over current signal polarity is high active.
>  - external-vbus-divider: enables off-chip resistor divider for Vbus
>  
>  Example:
The gpio device bindings already have active low/ active high.
Could that be used here?

In the imx SPI subsystem, for instance, the developers used gpio rather than
MUXED spi chipselect lines for exactly this kind of flexibility.

If there is no magic happening in silicon (which would seem to be the case if we can
handle inverting polarity in the driver), I would suggest going the gpiod route.

Thanks,
Joshua

[PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-current-polarity

[Peter Chen]

On Mon, Jul 18, 2016 at 08:24:50AM -0700, Joshua Clayton wrote:
> Greetings,
> On 07/18/2016 04:15 AM, Li Jun wrote:
> > Adding over-current-polarity to indicate the over current flag
> > is low active or high active.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > index 341dc67..c5d35f4 100644
> > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > @@ -81,6 +81,8 @@ 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-polarity: 0 if the over current signal polarity is low active,
> > +  1 if the over current signal polarity is high active.
> >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> >  
> >  Example:
> The gpio device bindings already have active low/ active high.
> Could that be used here?
> 

No, the over current pin is a dedicated pin for USB, you need
to configure it through pinmux. If we are using a gpio (even
the same pin as OC pin), the USB subsystem will not know
OC event. The polarity is for OC pin and configured through
USB register.

> In the imx SPI subsystem, for instance, the developers used gpio rather than
> MUXED spi chipselect lines for exactly this kind of flexibility.
> 
> If there is no magic happening in silicon (which would seem to be the case if we can
> handle inverting polarity in the driver), I would suggest going the gpiod route.
> 

-- 

Best Regards,
Peter Chen

[PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-current-polarity

[Jun Li]

Hi,
> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Tuesday, July 19, 2016 9:47 AM
> To: Joshua Clayton <stillcompiling@gmail.com>
> Cc: Jun Li <jun.li@nxp.com>; Peter Chen <peter.chen@nxp.com>;
> robh+dt at kernel.org; shawnguo at kernel.org; devicetree at vger.kernel.org;
> linux-usb at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-
> current-polarity
> 
> On Mon, Jul 18, 2016 at 08:24:50AM -0700, Joshua Clayton wrote:
> > Greetings,
> > On 07/18/2016 04:15 AM, Li Jun wrote:
> > > Adding over-current-polarity to indicate the over current flag is
> > > low active or high active.
> > >
> > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > > index 341dc67..c5d35f4 100644
> > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> > > @@ -81,6 +81,8 @@ 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-polarity: 0 if the over current signal polarity is
> > > +low active,
> > > +  1 if the over current signal polarity is high active.
> > >  - external-vbus-divider: enables off-chip resistor divider for Vbus
> > >
> > >  Example:
> > The gpio device bindings already have active low/ active high.
> > Could that be used here?
> >
> 
> No, the over current pin is a dedicated pin for USB, you need to configure
> it through pinmux. If we are using a gpio (even the same pin as OC pin),
> the USB subsystem will not know OC event. The polarity is for OC pin and
> configured through USB register.

Agree with Peter, there is do have magic in silicon of usb controller,
which is part of ehci spec so ehci host driver can take care of it as
a standard usb event.

> 
> > In the imx SPI subsystem, for instance, the developers used gpio
> > rather than MUXED spi chipselect lines for exactly this kind of
> flexibility.

I guess your SPI example is not a normal usage, but by that way to
workaround some silicon issue.

Li Jun
  
> >
> > If there is no magic happening in silicon (which would seem to be the
> > case if we can handle inverting polarity in the driver), I would suggest
> going the gpiod route.
> >
> 
> --
> 
> Best Regards,
> Peter Chen

[PATCH 2/4] doc: usb: ci-hdrc-usb2: add property over-current-polarity

[Rob Herring]

On Mon, Jul 18, 2016 at 07:15:45PM +0800, Li Jun wrote:
> Adding over-current-polarity to indicate the over current flag
> is low active or high active.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> index 341dc67..c5d35f4 100644
> --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt
> @@ -81,6 +81,8 @@ 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-polarity: 0 if the over current signal polarity is low active,
> +  1 if the over current signal polarity is high active.

Make the common case be no property and make this bool. So if typically 
active high, the property is "over-current-active-low".

Rob

[PATCH 3/4] ARM: dts: imx7d-sdb: Set OTG2 oc polarity to be active low

[Li Jun]

Set OTG2 over current flag to be active low, OTG1 OC is not used
by default.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 arch/arm/boot/dts/imx7d-sdb.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts
index b267f79..44ed399 100644
--- a/arch/arm/boot/dts/imx7d-sdb.dts
+++ b/arch/arm/boot/dts/imx7d-sdb.dts
@@ -287,7 +287,9 @@
 
 &usbotg2 {
 	vbus-supply = <&reg_usb_otg2_vbus>;
+	pinctrl-0 = <&pinctrl_usbotg2>;
 	dr_mode = "host";
+	over-current-polarity = <0>;
 	status = "okay";
 };
 
@@ -415,6 +417,12 @@
 			>;
 		};
 
+		pinctrl_usbotg2: usbotg2grp {
+			fsl,pins = <
+				MX7D_PAD_GPIO1_IO06__USB_OTG2_OC	0x59
+			>;
+		};
+
 		pinctrl_usdhc1: usdhc1grp {
 			fsl,pins = <
 				MX7D_PAD_SD1_CMD__SD1_CMD		0x59
-- 
1.9.1

[PATCH 4/4] usb: chipidea: imx: set over current polarity per dts setting

[Li Jun]

With over-current-polarity property added, imx usb over current
polarity can be configed to be low or high active, since the default
setting value(0) is for active high, so keep this setting for those
legacy platforms without this property specified.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/chipidea/ci_hdrc_imx.c |  9 +++++++++
 drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
 drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++-----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index dedc33e..61e712b 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -140,6 +140,15 @@ 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_property_read_u32(np, "over-current-polarity", &ret))
+		data->oc_polarity = ret ? 1 : 0;
+	else
+		/*
+		 * Keep the oc polarity setting of legacy
+		 * platforms unchanged.
+		 */
+		data->oc_polarity = 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 635717e..409aa5ca8 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -17,6 +17,7 @@ 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 */
 	unsigned int evdo:1; /* set external vbus divider option */
 };
 
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index ab8b027..193dbe4 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -56,6 +56,7 @@
 
 #define MX6_BM_NON_BURST_SETTING	BIT(1)
 #define MX6_BM_OVER_CUR_DIS		BIT(7)
+#define MX6_BM_OVER_CUR_POLARITY	BIT(8)
 #define MX6_BM_WAKEUP_ENABLE		BIT(10)
 #define MX6_BM_ID_WAKEUP		BIT(16)
 #define MX6_BM_VBUS_WAKEUP		BIT(17)
@@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
 
 	spin_lock_irqsave(&usbmisc->lock, flags);
 
+	reg = readl(usbmisc->base + data->index * 4);
 	if (data->disable_oc) {
-		reg = readl(usbmisc->base + data->index * 4);
-		writel(reg | MX6_BM_OVER_CUR_DIS,
-			usbmisc->base + data->index * 4);
+		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 {
+		/* Low active */
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+		reg |= MX6_BM_OVER_CUR_POLARITY;
 	}
+	writel(reg, usbmisc->base + data->index * 4);
 
 	/* SoC non-burst setting */
 	reg = readl(usbmisc->base + data->index * 4);
@@ -365,10 +373,18 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
 		return -EINVAL;
 
 	spin_lock_irqsave(&usbmisc->lock, flags);
+	reg = readl(usbmisc->base);
 	if (data->disable_oc) {
-		reg = readl(usbmisc->base);
-		writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
+		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 {
+		/* Low active */
+		reg &= ~MX6_BM_OVER_CUR_DIS;
+		reg |= MX6_BM_OVER_CUR_POLARITY;
 	}
+	writel(reg, usbmisc->base);
 
 	reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
 	reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
@@ -492,6 +508,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
 		.compatible = "fsl,imx6ul-usbmisc",
 		.data = &imx6sx_usbmisc_ops,
 	},
+	{
+		.compatible = "fsl,imx7d-usbmisc",
+		.data = &imx7d_usbmisc_ops,
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
-- 
1.9.1

[PATCH 4/4] usb: chipidea: imx: set over current polarity per dts setting

[Peter Chen]

On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote:
> With over-current-polarity property added, imx usb over current
> polarity can be configed to be low or high active, since the default
> setting value(0) is for active high, so keep this setting for those
> legacy platforms without this property specified.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c |  9 +++++++++
>  drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
>  drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++-----
>  3 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index dedc33e..61e712b 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -140,6 +140,15 @@ 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_property_read_u32(np, "over-current-polarity", &ret))
> +		data->oc_polarity = ret ? 1 : 0;
> +	else
> +		/*
> +		 * Keep the oc polarity setting of legacy
> +		 * platforms unchanged.
> +		 */
> +		data->oc_polarity = 1;

We may can't ensure that, since there are lots of i.mx platforms, eg
from imx27 to imx7.
   
If the user does not assign oc polarity at DT, we need to read it from
register.

Peter
> +
>  	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 635717e..409aa5ca8 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> @@ -17,6 +17,7 @@ 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 */
>  	unsigned int evdo:1; /* set external vbus divider option */
>  };
>  
> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
> index ab8b027..193dbe4 100644
> --- a/drivers/usb/chipidea/usbmisc_imx.c
> +++ b/drivers/usb/chipidea/usbmisc_imx.c
> @@ -56,6 +56,7 @@
>  
>  #define MX6_BM_NON_BURST_SETTING	BIT(1)
>  #define MX6_BM_OVER_CUR_DIS		BIT(7)
> +#define MX6_BM_OVER_CUR_POLARITY	BIT(8)
>  #define MX6_BM_WAKEUP_ENABLE		BIT(10)
>  #define MX6_BM_ID_WAKEUP		BIT(16)
>  #define MX6_BM_VBUS_WAKEUP		BIT(17)
> @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
>  
>  	spin_lock_irqsave(&usbmisc->lock, flags);
>  
> +	reg = readl(usbmisc->base + data->index * 4);
>  	if (data->disable_oc) {
> -		reg = readl(usbmisc->base + data->index * 4);
> -		writel(reg | MX6_BM_OVER_CUR_DIS,
> -			usbmisc->base + data->index * 4);
> +		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 {
> +		/* Low active */
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +		reg |= MX6_BM_OVER_CUR_POLARITY;
>  	}
> +	writel(reg, usbmisc->base + data->index * 4);
>  
>  	/* SoC non-burst setting */
>  	reg = readl(usbmisc->base + data->index * 4);
> @@ -365,10 +373,18 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(&usbmisc->lock, flags);
> +	reg = readl(usbmisc->base);
>  	if (data->disable_oc) {
> -		reg = readl(usbmisc->base);
> -		writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
> +		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 {
> +		/* Low active */
> +		reg &= ~MX6_BM_OVER_CUR_DIS;
> +		reg |= MX6_BM_OVER_CUR_POLARITY;
>  	}
> +	writel(reg, usbmisc->base);
>  
>  	reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
>  	reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
> @@ -492,6 +508,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = {
>  		.compatible = "fsl,imx6ul-usbmisc",
>  		.data = &imx6sx_usbmisc_ops,
>  	},
> +	{
> +		.compatible = "fsl,imx7d-usbmisc",
> +		.data = &imx7d_usbmisc_ops,
> +	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 

Best Regards,
Peter Chen

[PATCH 4/4] usb: chipidea: imx: set over current polarity per dts setting

[Jun Li]

Hi

> -----Original Message-----
> From: Peter Chen [mailto:hzpeterchen at gmail.com]
> Sent: Tuesday, July 19, 2016 9:57 AM
> To: Jun Li <jun.li@nxp.com>
> Cc: Peter Chen <peter.chen@nxp.com>; robh+dt at kernel.org;
> shawnguo at kernel.org; devicetree at vger.kernel.org; linux-usb at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 4/4] usb: chipidea: imx: set over current polarity per
> dts setting
> 
> On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote:
> > With over-current-polarity property added, imx usb over current
> > polarity can be configed to be low or high active, since the default
> > setting value(0) is for active high, so keep this setting for those
> > legacy platforms without this property specified.
> >
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c |  9 +++++++++
> > drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
> > drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++-----
> >  3 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index dedc33e..61e712b 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -140,6 +140,15 @@ 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_property_read_u32(np, "over-current-polarity", &ret))
> > +		data->oc_polarity = ret ? 1 : 0;
> > +	else
> > +		/*
> > +		 * Keep the oc polarity setting of legacy
> > +		 * platforms unchanged.
> > +		 */
> > +		data->oc_polarity = 1;
> 
> We may can't ensure that, since there are lots of i.mx platforms, eg from
> imx27 to imx7.
> 
> If the user does not assign oc polarity at DT, we need to read it from
> register.

I suppose old platforms (other than i.mx6&7) should set either
"over-current-polarity" or "disable-over-current" if want to use
"data->oc_polarity".
 
Do you mean read the register value before set "data->oc_polarity" here?
Even with that, as I still can't ensure all i.mx platforms have the same
mapping:
0 <--> active high.
1 <--> active low.
How should I set "data->oc_polarity" accordingly?

Li Jun
> 
> Peter
> > +
> >  	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 635717e..409aa5ca8 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> > @@ -17,6 +17,7 @@ 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
> > +*/
> >  	unsigned int evdo:1; /* set external vbus divider option */  };
> >
> > diff --git a/drivers/usb/chipidea/usbmisc_imx.c
> > b/drivers/usb/chipidea/usbmisc_imx.c
> > index ab8b027..193dbe4 100644
> > --- a/drivers/usb/chipidea/usbmisc_imx.c
> > +++ b/drivers/usb/chipidea/usbmisc_imx.c
> > @@ -56,6 +56,7 @@
> >
> >  #define MX6_BM_NON_BURST_SETTING	BIT(1)
> >  #define MX6_BM_OVER_CUR_DIS		BIT(7)
> > +#define MX6_BM_OVER_CUR_POLARITY	BIT(8)
> >  #define MX6_BM_WAKEUP_ENABLE		BIT(10)
> >  #define MX6_BM_ID_WAKEUP		BIT(16)
> >  #define MX6_BM_VBUS_WAKEUP		BIT(17)
> > @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct
> > imx_usbmisc_data *data)
> >
> >  	spin_lock_irqsave(&usbmisc->lock, flags);
> >
> > +	reg = readl(usbmisc->base + data->index * 4);
> >  	if (data->disable_oc) {
> > -		reg = readl(usbmisc->base + data->index * 4);
> > -		writel(reg | MX6_BM_OVER_CUR_DIS,
> > -			usbmisc->base + data->index * 4);
> > +		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 {
> > +		/* Low active */
> > +		reg &= ~MX6_BM_OVER_CUR_DIS;
> > +		reg |= MX6_BM_OVER_CUR_POLARITY;
> >  	}
> > +	writel(reg, usbmisc->base + data->index * 4);
> >
> >  	/* SoC non-burst setting */
> >  	reg = readl(usbmisc->base + data->index * 4); @@ -365,10 +373,18 @@
> > static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
> >  		return -EINVAL;
> >
> >  	spin_lock_irqsave(&usbmisc->lock, flags);
> > +	reg = readl(usbmisc->base);
> >  	if (data->disable_oc) {
> > -		reg = readl(usbmisc->base);
> > -		writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
> > +		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 {
> > +		/* Low active */
> > +		reg &= ~MX6_BM_OVER_CUR_DIS;
> > +		reg |= MX6_BM_OVER_CUR_POLARITY;
> >  	}
> > +	writel(reg, usbmisc->base);
> >
> >  	reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
> >  	reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
> > @@ -492,6 +508,10 @@ static const struct of_device_id
> usbmisc_imx_dt_ids[] = {
> >  		.compatible = "fsl,imx6ul-usbmisc",
> >  		.data = &imx6sx_usbmisc_ops,
> >  	},
> > +	{
> > +		.compatible = "fsl,imx7d-usbmisc",
> > +		.data = &imx7d_usbmisc_ops,
> > +	},
> >  	{ /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
> > --
> > 1.9.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> --
> 
> Best Regards,
> Peter Chen

[PATCH 4/4] usb: chipidea: imx: set over current polarity per dts setting

[Peter Chen]

On Tue, Jul 19, 2016 at 02:31:41AM +0000, Jun Li wrote:
> Hi
> 
> > -----Original Message-----
> > From: Peter Chen [mailto:hzpeterchen at gmail.com]
> > Sent: Tuesday, July 19, 2016 9:57 AM
> > To: Jun Li <jun.li@nxp.com>
> > Cc: Peter Chen <peter.chen@nxp.com>; robh+dt at kernel.org;
> > shawnguo at kernel.org; devicetree at vger.kernel.org; linux-usb at vger.kernel.org;
> > linux-arm-kernel at lists.infradead.org
> > Subject: Re: [PATCH 4/4] usb: chipidea: imx: set over current polarity per
> > dts setting
> > 
> > On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote:
> > > With over-current-polarity property added, imx usb over current
> > > polarity can be configed to be low or high active, since the default
> > > setting value(0) is for active high, so keep this setting for those
> > > legacy platforms without this property specified.
> > >
> > > Signed-off-by: Li Jun <jun.li@nxp.com>
> > > ---
> > >  drivers/usb/chipidea/ci_hdrc_imx.c |  9 +++++++++
> > > drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
> > > drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++-----
> > >  3 files changed, 35 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > index dedc33e..61e712b 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > > @@ -140,6 +140,15 @@ 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_property_read_u32(np, "over-current-polarity", &ret))
> > > +		data->oc_polarity = ret ? 1 : 0;
> > > +	else
> > > +		/*
> > > +		 * Keep the oc polarity setting of legacy
> > > +		 * platforms unchanged.
> > > +		 */
> > > +		data->oc_polarity = 1;
> > 
> > We may can't ensure that, since there are lots of i.mx platforms, eg from
> > imx27 to imx7.
> > 
> > If the user does not assign oc polarity at DT, we need to read it from
> > register.
> 
> I suppose old platforms (other than i.mx6&7) should set either
> "over-current-polarity" or "disable-over-current" if want to use
> "data->oc_polarity".
>  

The user may use old dtb and the latest kernel, so the introduced
DT property should not affect old dtb platforms.

> Do you mean read the register value before set "data->oc_polarity" here?

yes, if there is a dts property, read it from DT; else, read it from the register.

if (!of_property_read_u32(np, "over-current-polarity", &ret))
	data->oc_polarity = ret ? 1 : 0;
else
	/*
	 * Keep the oc polarity setting of legacy
	 * platforms unchanged.
	 */
	data->oc_polarity = readl(oc_polarity);

Then, when you set register for oc polarity, you can depend on the
flag.
	reg = readl(usbmisc->base + data->index * 4);
	value = data->oc_polarity ? MX6_BM_OVER_CUR_POLARITY : 0;
	writel(reg | value, usbmisc->base + data->index * 4);
	

> Even with that, as I still can't ensure all i.mx platforms have the same
> mapping:
> 0 <--> active high.
> 1 <--> active low.
> How should I set "data->oc_polarity" accordingly?

Just keep current register value unchanging if there is no dts property.

Peter

> Li Jun
> > 
> > Peter
> > > +
> > >  	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 635717e..409aa5ca8 100644
> > > --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> > > @@ -17,6 +17,7 @@ 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
> > > +*/
> > >  	unsigned int evdo:1; /* set external vbus divider option */  };
> > >
> > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c
> > > b/drivers/usb/chipidea/usbmisc_imx.c
> > > index ab8b027..193dbe4 100644
> > > --- a/drivers/usb/chipidea/usbmisc_imx.c
> > > +++ b/drivers/usb/chipidea/usbmisc_imx.c
> > > @@ -56,6 +56,7 @@
> > >
> > >  #define MX6_BM_NON_BURST_SETTING	BIT(1)
> > >  #define MX6_BM_OVER_CUR_DIS		BIT(7)
> > > +#define MX6_BM_OVER_CUR_POLARITY	BIT(8)
> > >  #define MX6_BM_WAKEUP_ENABLE		BIT(10)
> > >  #define MX6_BM_ID_WAKEUP		BIT(16)
> > >  #define MX6_BM_VBUS_WAKEUP		BIT(17)
> > > @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct
> > > imx_usbmisc_data *data)
> > >
> > >  	spin_lock_irqsave(&usbmisc->lock, flags);
> > >
> > > +	reg = readl(usbmisc->base + data->index * 4);
> > >  	if (data->disable_oc) {
> > > -		reg = readl(usbmisc->base + data->index * 4);
> > > -		writel(reg | MX6_BM_OVER_CUR_DIS,
> > > -			usbmisc->base + data->index * 4);
> > > +		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 {
> > > +		/* Low active */
> > > +		reg &= ~MX6_BM_OVER_CUR_DIS;
> > > +		reg |= MX6_BM_OVER_CUR_POLARITY;
> > >  	}
> > > +	writel(reg, usbmisc->base + data->index * 4);
> > >
> > >  	/* SoC non-burst setting */
> > >  	reg = readl(usbmisc->base + data->index * 4); @@ -365,10 +373,18 @@
> > > static int usbmisc_imx7d_init(struct imx_usbmisc_data *data)
> > >  		return -EINVAL;
> > >
> > >  	spin_lock_irqsave(&usbmisc->lock, flags);
> > > +	reg = readl(usbmisc->base);
> > >  	if (data->disable_oc) {
> > > -		reg = readl(usbmisc->base);
> > > -		writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base);
> > > +		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 {
> > > +		/* Low active */
> > > +		reg &= ~MX6_BM_OVER_CUR_DIS;
> > > +		reg |= MX6_BM_OVER_CUR_POLARITY;
> > >  	}
> > > +	writel(reg, usbmisc->base);
> > >
> > >  	reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2);
> > >  	reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK;
> > > @@ -492,6 +508,10 @@ static const struct of_device_id
> > usbmisc_imx_dt_ids[] = {
> > >  		.compatible = "fsl,imx6ul-usbmisc",
> > >  		.data = &imx6sx_usbmisc_ops,
> > >  	},
> > > +	{
> > > +		.compatible = "fsl,imx7d-usbmisc",
> > > +		.data = &imx7d_usbmisc_ops,
> > > +	},
> > >  	{ /* sentinel */ }
> > >  };
> > >  MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids);
> > > --
> > > 1.9.1
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> > --
> > 
> > Best Regards,
> > Peter Chen

-- 

Best Regards,
Peter Chen

[PATCH 1/4] doc: usb: usbmisc-imx: add imx7d compatible string

[Rob Herring]

On Mon, Jul 18, 2016 at 07:15:44PM +0800, Li Jun wrote:
> Add compatible string for imx7d-usbmisc.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  Documentation/devicetree/bindings/usb/usbmisc-imx.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>