Re: [PATCH v3 1/3] usb: chipidea: imx: support configuring for active low oc signal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 04.12.18 um 09:52 schrieb Uwe Kleine-König:
> Hello Stefan,
>
> On Tue, Dec 04, 2018 at 09:40:26AM +0100, Stefan Wahren wrote:
>> Am 04.12.18 um 09:31 schrieb Uwe Kleine-König:
>>> 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            | 16 ++++++++---
>>>  drivers/usb/chipidea/ci_hdrc_imx.h            |  8 +++++-
>>>  drivers/usb/chipidea/usbmisc_imx.c            | 28 ++++++++++++++-----
>>>  4 files changed, 43 insertions(+), 14 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:
>> thanks for making this configurable. But shouldn't this be a separate
>> patch and reviewed by the devicetree guys?
> I'm not sure about the separate patch part. My impression is it depends
> on the maintainer if a change to the binding (opposed to the
> introduction of a binding) should be separate or not.
>
> But for the review you are right, I added the dt people to Cc for them
> to comment. In v2 Matthew also noted that he would prefer to handle the
> situation when both over-current-active-low and over-current-active-high
> were given differently. I think we don't need that as this is a case of
> "broken dt" and it doesn't matter much what is done then. (With my patch
> we're configuring for active high in that case.) A feedback here would
> be great, too.

AFAIR such invalid settings should be catched with a proper error
message and abort of the probing.

Regards
Stefan

>
> Best regards
> Uwe
>



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux