>
> On Mon, Dec 03, 2018 at 03:13:01AM +0000, PETER CHEN wrote:
> > The current OC (Over Current) handling does not consider the default
> > and bootloader OC setting well, in this commit, we reset OC setting
> > according to dts value:
> > - If property "disable-over-current" is set, we will disable OC at
> > code; otherwise, we will enable OC.
> > - Since most of USB power control chips are low active for OC, we keep
> > "over-current-active-high" property unchanging to reduce users effort.
> > If this property is set, we set OC polarity as high explicitly;
> > otherwise, we set it as low explicitly.
> >
> > Cc: stable <stable@xxxxxxxxxxxxxxx>
> > Cc: Peter Chen <peter.chen@xxxxxxx>
> > Cc: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > Cc: Matthew Starr <mstarr@xxxxxxxxxxxxx>
> > Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
> > ---
> > drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
> > drivers/usb/chipidea/ci_hdrc_imx.h | 3 ++-
> > drivers/usb/chipidea/usbmisc_imx.c | 27 ++++++++++++++++++++++-----
> > 3 files changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 56781c329db0..058468f2de8d 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -140,7 +140,7 @@ static struct imx_usbmisc_data
> *usbmisc_get_init_data(struct device *dev)
> > data->disable_oc = 1;
> >
> > if (of_find_property(np, "over-current-active-high", NULL))
> > - data->oc_polarity = 1;
> > + data->oc_polarity_high = 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 fcecab478934..5ea8bb239b38 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.h
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
> > @@ -11,7 +11,8 @@ 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 */
> > + /* over current polarity high if oc enabled */
> > + unsigned int oc_polarity_high:1;
> > unsigned int evdo:1; /* set external vbus divider option */
> > unsigned int ulpi:1; /* connected to an ULPI phy */
> > unsigned int hsic:1; /* HSIC controlller */ diff --git
> > a/drivers/usb/chipidea/usbmisc_imx.c
> > b/drivers/usb/chipidea/usbmisc_imx.c
> > index 43a15a6e86f5..30d1ada952e1 100644
> > --- a/drivers/usb/chipidea/usbmisc_imx.c
> > +++ b/drivers/usb/chipidea/usbmisc_imx.c
> > @@ -135,15 +135,26 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data
> *data)
> > val = readl(usbmisc->base);
> > 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);
> > + val |= MX25_OTG_PM_BIT;
> > + if (data->oc_polarity_high)
> > + /* High active */
> > + val |= MX25_OTG_OCPOL_BIT;
> > + else
> > + val &= ~MX25_OTG_OCPOL_BIT;
> > +
>
> I think we shouldn't change the i.MX25 behaviour in a patch that is backported to
> stable. Even without this I'd put the change to i.MX25 in a separate patch. (As I did
> in my series.)
>
> Also the change is bad, because you're going from hard-coded active high to active
> low for all users that don't specify "over-current-active-high".
> This breaks (I think) most i.MX25 machines.
>
> Affected in mainline are:
>
> imx25-pdk.dts (&usbotg + &usbhost1)
> imx25-eukrea-mbimxsd25-baseboard.dts (&usbotg + &usbhost1)
>
> and out of tree probably more. (But maybe these use really active low signalling? I
> don't know and didn't check.)
Are you sure the boards you listed works well for OC now? Both you and Matthew posted
patch for OC issue, I think you both probably find the OC function works not well. I have
checked the boards (from imx35) at Freescale/NXP internal, all boards are active low for OC.
My intention is to let most of boards work well without adding dts property since most of
USB power control chip is active low for OC, the OC function should not test well before.
Your patches do not break current setting, but need the users to add dts property
to let OC function work (Since most of chip are active low for OC), almost all i.mx
boards will see the warning message for without setting OC polarity, and must
add dts property to fix this warning message.
I am OK to accept your patch if both Matthew and Jun are ok with it.
Peter
