Re: [PATCH 1/2] extcon: usb-gpio: Add level trigger support

[Baolin Wang]

Hi,

On 24 March 2017 at 20:41, Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> On Fri, Mar 24, 2017 at 6:56 AM, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>> Hi,
>>
>> On 24 March 2017 at 19:08, Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote:
>>> Hi,
>>>
>>> On 2017ë 03ì 20ì 16:59, Baolin Wang wrote:
>>>> Now extcon-usb-gpio only supports for GPIO egdge trigger, but VBUS/ID
>>>> gpios' detection can be triggered by the level trigger on some platforms.
>>>> Thus intoduce one property 'extcon-gpio,level-trigger' to identify this
>>>> situation.
>>>>
>>>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
>>>> ---
>>>>  .../devicetree/bindings/extcon/extcon-usb-gpio.txt |    3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> index dfc14f7..191504b 100644
>>>> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> @@ -9,6 +9,9 @@ Required properties:
>>>>  Either one of id-gpio or vbus-gpio must be present. Both can be present as well.
>>>>  - id-gpio: gpio for USB ID pin. See gpio binding.
>>>>  - vbus-gpio: gpio for USB VBUS pin.
>>>> +- extcon-gpio,level-trigger: Boolean, set this gpio's interrupt flag to
>>>
>>> It is 'extcon-usb-gpio' instead of 'extcon-gpio'.
>>
>> Sure.
>
> "extcon-gpio" is not a vendor, so don't make it a vendor prefix.

OK.

>
>>>> +level trigger. If not specified defaults to false, gpio's interrupt flag
>>>> +defaults to edge trigger.
>>>
>>> I understand why you need the new flag.
>>> But, I'm not sure it is right way to add the new flag for interrupt flag.
>>>
>>> IMHO, I think that we need to find the more proper way to get the interrupt flag
>>> or maybe, we may need to implement the new helper api to get the interrupt
>>> flag for gpio pin when we use the gpio as the interrupt source,
>>
>> Yes, I agree with you. We already have helper functions to get
>> interrupt flag from device tree, but we can not get the irq number of
>> GPIO to configure in device tree, then I introduce one new flag to
>> indicate the trigger type. But I like to change the patch if there are
>> any good suggestion. Thanks.
>
> The extcon binding needs an overhaul in general. I'm not going to take
> extensions to a broken binding. What's needed is a USB connector
> binding like we have for HDMI and other video connectors. And for
> USB-C, those need to be combined.

I am not sure I understand your points correctly, could you elaborate
on how to modify for extcon? Thanks.

-- 
Baolin.wang
Best Regards