On Mon, Dec 03, 2018 at 04:10:37PM +0000, Matthew Starr wrote: > > -----Original Message----- > > From: Uwe Kleine-König [mailto:u.kleine-koenig@xxxxxxxxxxxxxx] > > 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@xxxxxxxxxxxxxx> > > --- > > .../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/ |
