> -----Original Message----- > From: PETER CHEN [mailto:peter.chen@xxxxxxx] > Sent: Thursday, November 29, 2018 9:30 PM > > > > > The implementation for setting the over current polarity has always been the over- > > current-active-high property. The problem with this is there is no way to enable > > over current active low polarity since the default state of the register bit that controls > > the over current polarity is a 0 which means active high. This value never actually > > did anything since active high is already the default state. Also it is not used in any > > device tree source in the kernel. The default register bit state was verified by > > checking the i.MX6Q and i.MX7Dual reference manuals. > > > > The change replaces the over-current-active-high property with the over- current- > > active-low property. Without this property the over current polarity will be active > > high by default like it always has been. > > With the property the over current polarity will be active low. > > > > Would you please check it? Why it is opposite for your patch and Matthew's? The i.MX6DQ Reference Manual, the i.MX6UL Reference Manual, and the i.MX7D Reference Manual all show that the default reset value of the OVER_CUR_POL bit in the USBNC_USB_OTG_CTRL register is 0. Here is the reference manual's description for the OVER_CUR_POL bit: OTG Polarity of Overcurrent The polarity of OTG port overcurrent event 1 Low active (low on this signal represents an overcurrent condition) 0 High active (high on this signal represents an overcurrent condition) I verified this on a custom made product using an i.MX6UL. The overcurrent detection on this product is active low triggered, and only through the modification of the driver enabling active low triggering of the overcurrent functionality was I able to see the USB detect the overcurrent and reset itself. I think this has been missed up till now because the bit next to the OVER_CUR_POL bit is the PWR_POL which has the exact inverse logic with a value of 1 meaning high enable and 0 meaning low enable. Also I rarely have seen the USB interface go into an overcurrent situation so maybe no one ever caught this, but while forcing it during hardware testing the issue was discovered. Matt Starr > > > Signed-off-by: Matthew Starr <mstarr@xxxxxxxxxxxxx> > > --- > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 4 ++-- > > drivers/usb/chipidea/ci_hdrc_imx.c | 2 +- > > drivers/usb/chipidea/usbmisc_imx.c | 10 ++++++---- > > 3 files changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > index 529e51879fb2..3dee58037839 100644 > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > @@ -87,8 +87,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-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 low active, > > + the default signal polarity is high active. > > - 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..f7069544fb42 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > > @@ -135,7 +135,7 @@ 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)) > > + if (of_find_property(np, "over-current-active-low", NULL)) > > data->oc_polarity = 1; > > > > if (of_find_property(np, "external-vbus-divider", NULL)) > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > > index def80ff547e4..39223708e859 100644 > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > @@ -341,8 +341,9 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data) > > 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); > > + /* Low active */ > > + reg &= ~(MX6_BM_OVER_CUR_DIS); > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > } else { > > reg &= ~(MX6_BM_OVER_CUR_DIS); > > } > > @@ -445,8 +446,9 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data) > > 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); > > + /* Low active */ > > + reg &= ~(MX6_BM_OVER_CUR_DIS); > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > } > > writel(reg, usbmisc->base); > > > > -- > > 2.17.1
