Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Ping.

Hi Rob,

I add some comment about gpio-keys and extcon-gpio.
I'm waiting for your reply.

On 2016년 05월 31일 23:34, Chanwoo Choi wrote:
> Hi Rob,
> 
> On Tue, May 31, 2016 at 10:35 PM, Rob Herring  wrote:
>> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
>>> Hi Rob,
>>>
>>> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
 On 2016년 05월 28일 00:29, Rob Herring wrote:
> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>> Add the support for Device tree bindings of extcon-gpio driver.
>> The extcon-gpio device tree node must include the both 'extcon-id' and
>> 'gpios' property.
>
> I think extcon bindings are a mess in general...
>
>> For example:
>> usb_cable: extcon-gpio-0 {
>> compatible = "extcon-gpio";
>> extcon-id = ;
>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>> }
>> ta_cable: extcon-gpio-1 {
>> compatible = "extcon-gpio";
>> extcon-id = ;
>> gpios = < 2 GPIO_ACTIVE_LOW>;
>> debounce-ms = <50>; /* 50 millisecond */
>> wakeup-source;
>> }
>
> This is all 1 logical connector, the USB connector. Why are you
> describing cables? Those are not part of the h/w and are dynamic.
> Describe this as a connector which is one thing (i.e. node). Use a
> compatible string that reflects the type of connector
> (usb-microab-connector), not the driver you want to use. Then define
> GPIO lines needed to provide state information like VBus, ID, charger
> modes and control lines like soft connect (D+ pullup enable), VBus
> enable, etc.

 You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
 compatible.
 As you commented[1], the each connector will have the unique name to use 
 the extcon-gpio.c driver.
 [1] https://lkml.org/lkml/2015/10/21/906


 For example,
 The extcon-gpio.c driver may have the different name including the h/w 
 information
 according to the kind of external connector.

 static const struct of_device_id gpio_extcon_of_match[] = {
   {
   .compatible = "extcon-chg-sdp", /* SDP charger connector */
   .data = EXTCON_CHG_SDP_DATA,
   }, {
   .compatible = "extcon-chg-dcp", /* DCP charger connector */
   .data = EXTCON_CHG_DCP_DATA,
   }, {
   .compatible = "extcon-jack-microphone", /* Microphone jack 
 connector */
   .data = EXTCON_JACK_MICROPHONE_DATA,
   }, {
   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
   .data = EXTCON_DISP_HDMI_DATA,
   },
   ..
 };
>>>
>>> I reply it again.
>>>
>>> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
>>> [1] drivers/input/keyboard/gpio_keys.c
>>
>> There is a big difference in that each gpio-key is independent. The
>> only state is pressed or not. A USB connector has multiple pieces of
>> state information. You may be treating them independently, but I don't
>> think they should be.
> 
> I think that is it not different because the EXTCON can only notify the 
> whether
> external connector is attached or detached such as gpio-key (pressed or not).
> 
> EXTCON don't handle the any additional state except for attached or detached.
> 
>>
>>> The gpio_keys.c driver use the following style to support the device-tree.
>>> It use the "gpio-keys" compatible and this dt node include the specific
>>> 'key code' such as 'extcon-id = ;'
>>
>> This is state information about what is currently attached. The
>> analogy with gpio-keys would be multiple key codes on one gpio which
>> would be broken...
> 
> I compared between 'gpis-keys'  and ' extcon-gpio' driver as following:
> 
> name   | gpio-keys | extcon-gpio
> -
> gpio   | gpios = <>| extcon-gpio = <>
> -
> type   | linux,code = <> | extcon-id = <>
> -
> key &  | KEY_POWER  | EXTCON_USB
> extcon id  | KEY_VOLUME_UP | EXTCON_CHG_USB_SDP
>| KEY_VOLUME_DOWN   | EXTCON_JACK_MICROPHONE
>| etc| etc
> -
> state  | pressed or not  | attached or detached
> -

> 
>>
>>>
>>> gpio_keys {
>>> compatible = 

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Ping.

Hi Rob,

I add some comment about gpio-keys and extcon-gpio.
I'm waiting for your reply.

On 2016년 05월 31일 23:34, Chanwoo Choi wrote:
> Hi Rob,
> 
> On Tue, May 31, 2016 at 10:35 PM, Rob Herring  wrote:
>> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
>>> Hi Rob,
>>>
>>> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
 On 2016년 05월 28일 00:29, Rob Herring wrote:
> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>> Add the support for Device tree bindings of extcon-gpio driver.
>> The extcon-gpio device tree node must include the both 'extcon-id' and
>> 'gpios' property.
>
> I think extcon bindings are a mess in general...
>
>> For example:
>> usb_cable: extcon-gpio-0 {
>> compatible = "extcon-gpio";
>> extcon-id = ;
>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>> }
>> ta_cable: extcon-gpio-1 {
>> compatible = "extcon-gpio";
>> extcon-id = ;
>> gpios = < 2 GPIO_ACTIVE_LOW>;
>> debounce-ms = <50>; /* 50 millisecond */
>> wakeup-source;
>> }
>
> This is all 1 logical connector, the USB connector. Why are you
> describing cables? Those are not part of the h/w and are dynamic.
> Describe this as a connector which is one thing (i.e. node). Use a
> compatible string that reflects the type of connector
> (usb-microab-connector), not the driver you want to use. Then define
> GPIO lines needed to provide state information like VBus, ID, charger
> modes and control lines like soft connect (D+ pullup enable), VBus
> enable, etc.

 You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
 compatible.
 As you commented[1], the each connector will have the unique name to use 
 the extcon-gpio.c driver.
 [1] https://lkml.org/lkml/2015/10/21/906


 For example,
 The extcon-gpio.c driver may have the different name including the h/w 
 information
 according to the kind of external connector.

 static const struct of_device_id gpio_extcon_of_match[] = {
   {
   .compatible = "extcon-chg-sdp", /* SDP charger connector */
   .data = EXTCON_CHG_SDP_DATA,
   }, {
   .compatible = "extcon-chg-dcp", /* DCP charger connector */
   .data = EXTCON_CHG_DCP_DATA,
   }, {
   .compatible = "extcon-jack-microphone", /* Microphone jack 
 connector */
   .data = EXTCON_JACK_MICROPHONE_DATA,
   }, {
   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
   .data = EXTCON_DISP_HDMI_DATA,
   },
   ..
 };
>>>
>>> I reply it again.
>>>
>>> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
>>> [1] drivers/input/keyboard/gpio_keys.c
>>
>> There is a big difference in that each gpio-key is independent. The
>> only state is pressed or not. A USB connector has multiple pieces of
>> state information. You may be treating them independently, but I don't
>> think they should be.
> 
> I think that is it not different because the EXTCON can only notify the 
> whether
> external connector is attached or detached such as gpio-key (pressed or not).
> 
> EXTCON don't handle the any additional state except for attached or detached.
> 
>>
>>> The gpio_keys.c driver use the following style to support the device-tree.
>>> It use the "gpio-keys" compatible and this dt node include the specific
>>> 'key code' such as 'extcon-id = ;'
>>
>> This is state information about what is currently attached. The
>> analogy with gpio-keys would be multiple key codes on one gpio which
>> would be broken...
> 
> I compared between 'gpis-keys'  and ' extcon-gpio' driver as following:
> 
> name   | gpio-keys | extcon-gpio
> -
> gpio   | gpios = <>| extcon-gpio = <>
> -
> type   | linux,code = <> | extcon-id = <>
> -
> key &  | KEY_POWER  | EXTCON_USB
> extcon id  | KEY_VOLUME_UP | EXTCON_CHG_USB_SDP
>| KEY_VOLUME_DOWN   | EXTCON_JACK_MICROPHONE
>| etc| etc
> -
> state  | pressed or not  | attached or detached
> -

> 
>>
>>>
>>> gpio_keys {
>>> compatible = "gpio-keys";
>>>
>>> 

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Wed, Jun 1, 2016 at 9:49 AM, Laxman Dewangan  wrote:
>
> On Tuesday 31 May 2016 09:18 PM, Rob Herring wrote:
>>
>> On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan 
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

 On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
 wrote:
>
> The extcon-gpio.c driver can separate the kind of external connector
> by using the 'extcon-id' property.

 This use of DT is just broken. Come up with another way.


>>>
>>> Can we have the DT binding very similar to IIO, clock, reset etc?
>>
>> In what way? I'm guessing you mean to describe what controller a
>> connector is associated with. Certainly, some sort of phandle
>> reference will be needed. However. the node it points to is what needs
>> a lot of work first.
>
>
> Here, we just need to define that controller support how many different
> cable? It does not say anything that which cable is what and all its on
> platform dependent.
>
> So for gpio-extcon, it will just say 1 cable. the name is not decided by
> driver but it is by platform.

Why the fascination with cables? You need to describe the connector
and circuitry between the connector and phy/controller, and its
capabilities. Maybe those properties are implied or explicit. If you
have some USB chip for Vbus control and charger detect, then it's all
going to be implied by that chip's compatible. If you have gpio lines
then the properties and capabilities are explicit.

>>> Here is details for extcon-jack DT binding and its client:
>>>
>>> The client can get the cable information through its node or extcon name
>>> and once cable information is available, it can register for notification
>>> when state gets changed.
>>>
>>> The typical dt nodes are:
>>>
>>>  Extcon-driver node:
>>>
>>>  extcon: arizona-extcon {
>>>  compatible = "wlf,arizona-extcon";
>>>  #extcon-cells = <1>;
>>>  };
>>>
>>>
>>>
>>>  Driver need to specify the cable ID as
>>
>> Unless you have cables hardwired to a board, please stop describing
>> cables in DT. Connectors!
>
>
> If some controller only support cable with names then we need to do it. Like
> palmas extcon, it detect VBUS, ID_GND, RIDA, RIDB, RIDC. So for these cases,
> driver need to define cables.

The only thing the DT should have is the palmas chip and any
connections to it like regulators or interrupts. What the chip can
control/detect doesn't belong in DT.

> Driver can specify that cable index 0 is for VBUS, 1 is for ID-GND etc.

You are still just making up indexes.

>>>   Cable  ID
>>>   
>>>Mechanical0
>>>Microphone   1
>>>Headphone   2
>>>Line-out3
>>
>> No, please don't create some made up some number space. I don't see
>> why you need this. You should just need the phandle to "the microphone
>> jack" node.
>
>
> I am thinking that I will need the phandle of extcon nodes with index for
> cable ID.
>
>
>>>  Here #extcon-cells is must and specifies the size of extcon cells.
>>> The
>>> client need to provide the driver specific information as argument along
>>> with handle.
>>>
>>>
>>> Extcon Client node:
>>>
>>>  audio-controller@b {
>>> 
>>>  extcon-cables = < 1>, < 3>;
>>>  extcon-cable-names = "Microphone", "Line-out";
>>>  };
>>>
>>> and client driver can register the cable by passing the cable name
>>> as above along with its node.
>>>
>>> struct extcon_cable {
>>>  struct extcon_dev *edev,
>>>  int cable_id;
>>> };
>>
>> If you are showing driver details to explain the binding, something is
>> wrong.
>
>
>
> I have USB OTG driver which need the notification when VBUS or ID detected.

That's a kernel architectural detail.

> For this, it can get the phandle of extcon and cable ID from that driver for
> "id" and "vbus".

You only need to describe the connection of the connector to the
controller. These details don't need to be in DT.

> I have platform like follows:
> I have 2 gpios which detect the ID and VBUS.
> Both are independent. My USB driver get the extcon handle from the DT:
>
> extcon_vbus: extcon@0 {
> comptaible = "extcon-gpio";
> gpio = < JETSON_VBUS_GPIO 0>;
> #extcon-cells = <1>;
> };
>
>
>
> extcon_id: extcon@1 {
> comptaible = "extcon-gpio";
> gpio = < JETSON_ID_GPIO 0>;
> #extcon-cells = <1>;
> };

Sigh. These are all properties of one entity: a connector.

>
> usb-otg {
> ::
> extcon-cable = <_vbus 0 _d 0>;
> extcon-cable-names = "id", "vbus";
> ::
> };

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Wed, Jun 1, 2016 at 9:49 AM, Laxman Dewangan  wrote:
>
> On Tuesday 31 May 2016 09:18 PM, Rob Herring wrote:
>>
>> On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan 
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

 On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
 wrote:
>
> The extcon-gpio.c driver can separate the kind of external connector
> by using the 'extcon-id' property.

 This use of DT is just broken. Come up with another way.


>>>
>>> Can we have the DT binding very similar to IIO, clock, reset etc?
>>
>> In what way? I'm guessing you mean to describe what controller a
>> connector is associated with. Certainly, some sort of phandle
>> reference will be needed. However. the node it points to is what needs
>> a lot of work first.
>
>
> Here, we just need to define that controller support how many different
> cable? It does not say anything that which cable is what and all its on
> platform dependent.
>
> So for gpio-extcon, it will just say 1 cable. the name is not decided by
> driver but it is by platform.

Why the fascination with cables? You need to describe the connector
and circuitry between the connector and phy/controller, and its
capabilities. Maybe those properties are implied or explicit. If you
have some USB chip for Vbus control and charger detect, then it's all
going to be implied by that chip's compatible. If you have gpio lines
then the properties and capabilities are explicit.

>>> Here is details for extcon-jack DT binding and its client:
>>>
>>> The client can get the cable information through its node or extcon name
>>> and once cable information is available, it can register for notification
>>> when state gets changed.
>>>
>>> The typical dt nodes are:
>>>
>>>  Extcon-driver node:
>>>
>>>  extcon: arizona-extcon {
>>>  compatible = "wlf,arizona-extcon";
>>>  #extcon-cells = <1>;
>>>  };
>>>
>>>
>>>
>>>  Driver need to specify the cable ID as
>>
>> Unless you have cables hardwired to a board, please stop describing
>> cables in DT. Connectors!
>
>
> If some controller only support cable with names then we need to do it. Like
> palmas extcon, it detect VBUS, ID_GND, RIDA, RIDB, RIDC. So for these cases,
> driver need to define cables.

The only thing the DT should have is the palmas chip and any
connections to it like regulators or interrupts. What the chip can
control/detect doesn't belong in DT.

> Driver can specify that cable index 0 is for VBUS, 1 is for ID-GND etc.

You are still just making up indexes.

>>>   Cable  ID
>>>   
>>>Mechanical0
>>>Microphone   1
>>>Headphone   2
>>>Line-out3
>>
>> No, please don't create some made up some number space. I don't see
>> why you need this. You should just need the phandle to "the microphone
>> jack" node.
>
>
> I am thinking that I will need the phandle of extcon nodes with index for
> cable ID.
>
>
>>>  Here #extcon-cells is must and specifies the size of extcon cells.
>>> The
>>> client need to provide the driver specific information as argument along
>>> with handle.
>>>
>>>
>>> Extcon Client node:
>>>
>>>  audio-controller@b {
>>> 
>>>  extcon-cables = < 1>, < 3>;
>>>  extcon-cable-names = "Microphone", "Line-out";
>>>  };
>>>
>>> and client driver can register the cable by passing the cable name
>>> as above along with its node.
>>>
>>> struct extcon_cable {
>>>  struct extcon_dev *edev,
>>>  int cable_id;
>>> };
>>
>> If you are showing driver details to explain the binding, something is
>> wrong.
>
>
>
> I have USB OTG driver which need the notification when VBUS or ID detected.

That's a kernel architectural detail.

> For this, it can get the phandle of extcon and cable ID from that driver for
> "id" and "vbus".

You only need to describe the connection of the connector to the
controller. These details don't need to be in DT.

> I have platform like follows:
> I have 2 gpios which detect the ID and VBUS.
> Both are independent. My USB driver get the extcon handle from the DT:
>
> extcon_vbus: extcon@0 {
> comptaible = "extcon-gpio";
> gpio = < JETSON_VBUS_GPIO 0>;
> #extcon-cells = <1>;
> };
>
>
>
> extcon_id: extcon@1 {
> comptaible = "extcon-gpio";
> gpio = < JETSON_ID_GPIO 0>;
> #extcon-cells = <1>;
> };

Sigh. These are all properties of one entity: a connector.

>
> usb-otg {
> ::
> extcon-cable = <_vbus 0 _d 0>;
> extcon-cable-names = "id", "vbus";
> ::
> };

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Laxman Dewangan]

On Tuesday 31 May 2016 09:18 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan  wrote:

Hi Rob,

On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
wrote:

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.




Can we have the DT binding very similar to IIO, clock, reset etc?

In what way? I'm guessing you mean to describe what controller a
connector is associated with. Certainly, some sort of phandle
reference will be needed. However. the node it points to is what needs
a lot of work first.


Here, we just need to define that controller support how many different 
cable? It does not say anything that which cable is what and all its on 
platform dependent.


So for gpio-extcon, it will just say 1 cable. the name is not decided by 
driver but it is by platform.




Here is details for extcon-jack DT binding and its client:

The client can get the cable information through its node or extcon name
and once cable information is available, it can register for notification
when state gets changed.

The typical dt nodes are:

 Extcon-driver node:

 extcon: arizona-extcon {
 compatible = "wlf,arizona-extcon";
 #extcon-cells = <1>;
 };



 Driver need to specify the cable ID as

Unless you have cables hardwired to a board, please stop describing
cables in DT. Connectors!


If some controller only support cable with names then we need to do it. 
Like palmas extcon, it detect VBUS, ID_GND, RIDA, RIDB, RIDC. So for 
these cases, driver need to define cables.

Driver can specify that cable index 0 is for VBUS, 1 is for ID-GND etc.





  Cable  ID
  
   Mechanical0
   Microphone   1
   Headphone   2
   Line-out3

No, please don't create some made up some number space. I don't see
why you need this. You should just need the phandle to "the microphone
jack" node.


I am thinking that I will need the phandle of extcon nodes with index 
for cable ID.




 Here #extcon-cells is must and specifies the size of extcon cells. The
client need to provide the driver specific information as argument along
with handle.


Extcon Client node:

 audio-controller@b {

 extcon-cables = < 1>, < 3>;
 extcon-cable-names = "Microphone", "Line-out";
 };

and client driver can register the cable by passing the cable name
as above along with its node.

struct extcon_cable {
 struct extcon_dev *edev,
 int cable_id;
};

If you are showing driver details to explain the binding, something is wrong.



I have USB OTG driver which need the notification when VBUS or ID 
detected. For this, it can get the phandle of extcon and cable ID from 
that driver for  "id" and "vbus".



I have platform like follows:
I have 2 gpios which detect the ID and VBUS.
Both are independent. My USB driver get the extcon handle from the DT:

extcon_vbus: extcon@0 {
comptaible = "extcon-gpio";
gpio = < JETSON_VBUS_GPIO 0>;
#extcon-cells = <1>;
};



extcon_id: extcon@1 {
comptaible = "extcon-gpio";
gpio = < JETSON_ID_GPIO 0>;
#extcon-cells = <1>;
};

usb-otg {
::
extcon-cable = <_vbus 0 _d 0>;
extcon-cable-names = "id", "vbus";
::
};





edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
extcon_register_notification(edev_mic_cable, notifier);

edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
extcon_register_notification(edev_line_out_cable, notifier);

Just remember that "*-names" should be optional (nor am I a fan of
adding -names everywhere). This can be defined as first entry is mic
and 2nd entry is line-out.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Laxman Dewangan]

On Tuesday 31 May 2016 09:18 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan  wrote:

Hi Rob,

On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
wrote:

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.




Can we have the DT binding very similar to IIO, clock, reset etc?

In what way? I'm guessing you mean to describe what controller a
connector is associated with. Certainly, some sort of phandle
reference will be needed. However. the node it points to is what needs
a lot of work first.


Here, we just need to define that controller support how many different 
cable? It does not say anything that which cable is what and all its on 
platform dependent.


So for gpio-extcon, it will just say 1 cable. the name is not decided by 
driver but it is by platform.




Here is details for extcon-jack DT binding and its client:

The client can get the cable information through its node or extcon name
and once cable information is available, it can register for notification
when state gets changed.

The typical dt nodes are:

 Extcon-driver node:

 extcon: arizona-extcon {
 compatible = "wlf,arizona-extcon";
 #extcon-cells = <1>;
 };



 Driver need to specify the cable ID as

Unless you have cables hardwired to a board, please stop describing
cables in DT. Connectors!


If some controller only support cable with names then we need to do it. 
Like palmas extcon, it detect VBUS, ID_GND, RIDA, RIDB, RIDC. So for 
these cases, driver need to define cables.

Driver can specify that cable index 0 is for VBUS, 1 is for ID-GND etc.





  Cable  ID
  
   Mechanical0
   Microphone   1
   Headphone   2
   Line-out3

No, please don't create some made up some number space. I don't see
why you need this. You should just need the phandle to "the microphone
jack" node.


I am thinking that I will need the phandle of extcon nodes with index 
for cable ID.




 Here #extcon-cells is must and specifies the size of extcon cells. The
client need to provide the driver specific information as argument along
with handle.


Extcon Client node:

 audio-controller@b {

 extcon-cables = < 1>, < 3>;
 extcon-cable-names = "Microphone", "Line-out";
 };

and client driver can register the cable by passing the cable name
as above along with its node.

struct extcon_cable {
 struct extcon_dev *edev,
 int cable_id;
};

If you are showing driver details to explain the binding, something is wrong.



I have USB OTG driver which need the notification when VBUS or ID 
detected. For this, it can get the phandle of extcon and cable ID from 
that driver for  "id" and "vbus".



I have platform like follows:
I have 2 gpios which detect the ID and VBUS.
Both are independent. My USB driver get the extcon handle from the DT:

extcon_vbus: extcon@0 {
comptaible = "extcon-gpio";
gpio = < JETSON_VBUS_GPIO 0>;
#extcon-cells = <1>;
};



extcon_id: extcon@1 {
comptaible = "extcon-gpio";
gpio = < JETSON_ID_GPIO 0>;
#extcon-cells = <1>;
};

usb-otg {
::
extcon-cable = <_vbus 0 _d 0>;
extcon-cable-names = "id", "vbus";
::
};





edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
extcon_register_notification(edev_mic_cable, notifier);

edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
extcon_register_notification(edev_line_out_cable, notifier);

Just remember that "*-names" should be optional (nor am I a fan of
adding -names everywhere). This can be defined as first entry is mic
and 2nd entry is line-out.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan  wrote:
> Hi Rob,
>
> On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:
>>
>> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
>> wrote:
>>>
>>> The extcon-gpio.c driver can separate the kind of external connector
>>> by using the 'extcon-id' property.
>>
>> This use of DT is just broken. Come up with another way.
>>
>>
>
>
> Can we have the DT binding very similar to IIO, clock, reset etc?

In what way? I'm guessing you mean to describe what controller a
connector is associated with. Certainly, some sort of phandle
reference will be needed. However. the node it points to is what needs
a lot of work first.

>
> Here is details for extcon-jack DT binding and its client:
>
> The client can get the cable information through its node or extcon name
> and once cable information is available, it can register for notification
> when state gets changed.
>
> The typical dt nodes are:
>
> Extcon-driver node:
>
> extcon: arizona-extcon {
> compatible = "wlf,arizona-extcon";
> #extcon-cells = <1>;
> };
>
>
>
> Driver need to specify the cable ID as

Unless you have cables hardwired to a board, please stop describing
cables in DT. Connectors!

>  Cable  ID
>  
>   Mechanical0
>   Microphone   1
>   Headphone   2
>   Line-out3

No, please don't create some made up some number space. I don't see
why you need this. You should just need the phandle to "the microphone
jack" node.

> Here #extcon-cells is must and specifies the size of extcon cells. The
> client need to provide the driver specific information as argument along
> with handle.
>
>
> Extcon Client node:
>
> audio-controller@b {
>
> extcon-cables = < 1>, < 3>;
> extcon-cable-names = "Microphone", "Line-out";
> };
>
> and client driver can register the cable by passing the cable name
> as above along with its node.
>
> struct extcon_cable {
> struct extcon_dev *edev,
> int cable_id;
> };

If you are showing driver details to explain the binding, something is wrong.

>
> edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
> extcon_register_notification(edev_mic_cable, notifier);
>
> edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
> extcon_register_notification(edev_line_out_cable, notifier);

Just remember that "*-names" should be optional (nor am I a fan of
adding -names everywhere). This can be defined as first entry is mic
and 2nd entry is line-out.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Tue, May 31, 2016 at 8:44 AM, Laxman Dewangan  wrote:
> Hi Rob,
>
> On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:
>>
>> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi 
>> wrote:
>>>
>>> The extcon-gpio.c driver can separate the kind of external connector
>>> by using the 'extcon-id' property.
>>
>> This use of DT is just broken. Come up with another way.
>>
>>
>
>
> Can we have the DT binding very similar to IIO, clock, reset etc?

In what way? I'm guessing you mean to describe what controller a
connector is associated with. Certainly, some sort of phandle
reference will be needed. However. the node it points to is what needs
a lot of work first.

>
> Here is details for extcon-jack DT binding and its client:
>
> The client can get the cable information through its node or extcon name
> and once cable information is available, it can register for notification
> when state gets changed.
>
> The typical dt nodes are:
>
> Extcon-driver node:
>
> extcon: arizona-extcon {
> compatible = "wlf,arizona-extcon";
> #extcon-cells = <1>;
> };
>
>
>
> Driver need to specify the cable ID as

Unless you have cables hardwired to a board, please stop describing
cables in DT. Connectors!

>  Cable  ID
>  
>   Mechanical0
>   Microphone   1
>   Headphone   2
>   Line-out3

No, please don't create some made up some number space. I don't see
why you need this. You should just need the phandle to "the microphone
jack" node.

> Here #extcon-cells is must and specifies the size of extcon cells. The
> client need to provide the driver specific information as argument along
> with handle.
>
>
> Extcon Client node:
>
> audio-controller@b {
>
> extcon-cables = < 1>, < 3>;
> extcon-cable-names = "Microphone", "Line-out";
> };
>
> and client driver can register the cable by passing the cable name
> as above along with its node.
>
> struct extcon_cable {
> struct extcon_dev *edev,
> int cable_id;
> };

If you are showing driver details to explain the binding, something is wrong.

>
> edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
> extcon_register_notification(edev_mic_cable, notifier);
>
> edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
> extcon_register_notification(edev_line_out_cable, notifier);

Just remember that "*-names" should be optional (nor am I a fan of
adding -names everywhere). This can be defined as first entry is mic
and 2nd entry is line-out.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Rob,

On Tue, May 31, 2016 at 10:35 PM, Rob Herring  wrote:
> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
>> Hi Rob,
>>
>> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
>>> On 2016년 05월 28일 00:29, Rob Herring wrote:
 On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.

 I think extcon bindings are a mess in general...

> For example:
> usb_cable: extcon-gpio-0 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 1 GPIO_ACTIVE_HIGH>;
> }
> ta_cable: extcon-gpio-1 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 2 GPIO_ACTIVE_LOW>;
> debounce-ms = <50>; /* 50 millisecond */
> wakeup-source;
> }

 This is all 1 logical connector, the USB connector. Why are you
 describing cables? Those are not part of the h/w and are dynamic.
 Describe this as a connector which is one thing (i.e. node). Use a
 compatible string that reflects the type of connector
 (usb-microab-connector), not the driver you want to use. Then define
 GPIO lines needed to provide state information like VBus, ID, charger
 modes and control lines like soft connect (D+ pullup enable), VBus
 enable, etc.
>>>
>>> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
>>> compatible.
>>> As you commented[1], the each connector will have the unique name to use 
>>> the extcon-gpio.c driver.
>>> [1] https://lkml.org/lkml/2015/10/21/906
>>>
>>>
>>> For example,
>>> The extcon-gpio.c driver may have the different name including the h/w 
>>> information
>>> according to the kind of external connector.
>>>
>>> static const struct of_device_id gpio_extcon_of_match[] = {
>>>   {
>>>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>>>   .data = EXTCON_CHG_SDP_DATA,
>>>   }, {
>>>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>>>   .data = EXTCON_CHG_DCP_DATA,
>>>   }, {
>>>   .compatible = "extcon-jack-microphone", /* Microphone jack 
>>> connector */
>>>   .data = EXTCON_JACK_MICROPHONE_DATA,
>>>   }, {
>>>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>>>   .data = EXTCON_DISP_HDMI_DATA,
>>>   },
>>>   ..
>>> };
>>
>> I reply it again.
>>
>> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
>> [1] drivers/input/keyboard/gpio_keys.c
>
> There is a big difference in that each gpio-key is independent. The
> only state is pressed or not. A USB connector has multiple pieces of
> state information. You may be treating them independently, but I don't
> think they should be.

I think that is it not different because the EXTCON can only notify the whether
external connector is attached or detached such as gpio-key (pressed or not).

EXTCON don't handle the any additional state except for attached or detached.

>
>> The gpio_keys.c driver use the following style to support the device-tree.
>> It use the "gpio-keys" compatible and this dt node include the specific
>> 'key code' such as 'extcon-id = ;'
>
> This is state information about what is currently attached. The
> analogy with gpio-keys would be multiple key codes on one gpio which
> would be broken...

I compared between 'gpis-keys'  and ' extcon-gpio' driver as following:

name  | gpio-keys | extcon-gpio
-
gpio| gpios = <>| extcon-gpio = <>
-
type| linux,code = <> | extcon-id = <>
-
key &  | KEY_POWER  | EXTCON_USB
extcon id | KEY_VOLUME_UP | EXTCON_CHG_USB_SDP
   | KEY_VOLUME_DOWN   | EXTCON_JACK_MICROPHONE
   | etc| etc
-
state   | pressed or not  | attached or detached
-

>
>>
>> gpio_keys {
>> compatible = "gpio-keys";
>>
>> power_key {
>> gpios = < 7 GPIO_ACTIVE_LOW>;
>> linux,code = ;
>> label = "power key";
>> debounce-interval = <10>;
>> 

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Rob,

On Tue, May 31, 2016 at 10:35 PM, Rob Herring  wrote:
> On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
>> Hi Rob,
>>
>> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
>>> On 2016년 05월 28일 00:29, Rob Herring wrote:
 On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.

 I think extcon bindings are a mess in general...

> For example:
> usb_cable: extcon-gpio-0 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 1 GPIO_ACTIVE_HIGH>;
> }
> ta_cable: extcon-gpio-1 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 2 GPIO_ACTIVE_LOW>;
> debounce-ms = <50>; /* 50 millisecond */
> wakeup-source;
> }

 This is all 1 logical connector, the USB connector. Why are you
 describing cables? Those are not part of the h/w and are dynamic.
 Describe this as a connector which is one thing (i.e. node). Use a
 compatible string that reflects the type of connector
 (usb-microab-connector), not the driver you want to use. Then define
 GPIO lines needed to provide state information like VBus, ID, charger
 modes and control lines like soft connect (D+ pullup enable), VBus
 enable, etc.
>>>
>>> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
>>> compatible.
>>> As you commented[1], the each connector will have the unique name to use 
>>> the extcon-gpio.c driver.
>>> [1] https://lkml.org/lkml/2015/10/21/906
>>>
>>>
>>> For example,
>>> The extcon-gpio.c driver may have the different name including the h/w 
>>> information
>>> according to the kind of external connector.
>>>
>>> static const struct of_device_id gpio_extcon_of_match[] = {
>>>   {
>>>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>>>   .data = EXTCON_CHG_SDP_DATA,
>>>   }, {
>>>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>>>   .data = EXTCON_CHG_DCP_DATA,
>>>   }, {
>>>   .compatible = "extcon-jack-microphone", /* Microphone jack 
>>> connector */
>>>   .data = EXTCON_JACK_MICROPHONE_DATA,
>>>   }, {
>>>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>>>   .data = EXTCON_DISP_HDMI_DATA,
>>>   },
>>>   ..
>>> };
>>
>> I reply it again.
>>
>> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
>> [1] drivers/input/keyboard/gpio_keys.c
>
> There is a big difference in that each gpio-key is independent. The
> only state is pressed or not. A USB connector has multiple pieces of
> state information. You may be treating them independently, but I don't
> think they should be.

I think that is it not different because the EXTCON can only notify the whether
external connector is attached or detached such as gpio-key (pressed or not).

EXTCON don't handle the any additional state except for attached or detached.

>
>> The gpio_keys.c driver use the following style to support the device-tree.
>> It use the "gpio-keys" compatible and this dt node include the specific
>> 'key code' such as 'extcon-id = ;'
>
> This is state information about what is currently attached. The
> analogy with gpio-keys would be multiple key codes on one gpio which
> would be broken...

I compared between 'gpis-keys'  and ' extcon-gpio' driver as following:

name  | gpio-keys | extcon-gpio
-
gpio| gpios = <>| extcon-gpio = <>
-
type| linux,code = <> | extcon-id = <>
-
key &  | KEY_POWER  | EXTCON_USB
extcon id | KEY_VOLUME_UP | EXTCON_CHG_USB_SDP
   | KEY_VOLUME_DOWN   | EXTCON_JACK_MICROPHONE
   | etc| etc
-
state   | pressed or not  | attached or detached
-

>
>>
>> gpio_keys {
>> compatible = "gpio-keys";
>>
>> power_key {
>> gpios = < 7 GPIO_ACTIVE_LOW>;
>> linux,code = ;
>> label = "power key";
>> debounce-interval = <10>;
>> wakeup-source;
>> };
>>

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Laxman Dewangan]

Hi Rob,

On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.





Can we have the DT binding very similar to IIO, clock, reset etc?

Here is details for extcon-jack DT binding and its client:

The client can get the cable information through its node or extcon name
and once cable information is available, it can register for 
notification when state gets changed.


The typical dt nodes are:

Extcon-driver node:

extcon: arizona-extcon {
compatible = "wlf,arizona-extcon";
#extcon-cells = <1>;
};



Driver need to specify the cable ID as
 Cable  ID
 
  Mechanical0
  Microphone   1
  Headphone   2
  Line-out3

Here #extcon-cells is must and specifies the size of extcon cells. 
The client need to provide the driver specific information as argument 
along with handle.



Extcon Client node:

audio-controller@b {
   
extcon-cables = < 1>, < 3>;
extcon-cable-names = "Microphone", "Line-out";
};

and client driver can register the cable by passing the cable name
as above along with its node.

struct extcon_cable {
struct extcon_dev *edev,
int cable_id;
};

edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
extcon_register_notification(edev_mic_cable, notifier);

edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
extcon_register_notification(edev_line_out_cable, notifier);


Prototype:
struct extcon_cable *extcon_get_extcon_cable(struct device *dev, const 
char *cable_name)

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Laxman Dewangan]

Hi Rob,

On Tuesday 31 May 2016 07:05 PM, Rob Herring wrote:

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.





Can we have the DT binding very similar to IIO, clock, reset etc?

Here is details for extcon-jack DT binding and its client:

The client can get the cable information through its node or extcon name
and once cable information is available, it can register for 
notification when state gets changed.


The typical dt nodes are:

Extcon-driver node:

extcon: arizona-extcon {
compatible = "wlf,arizona-extcon";
#extcon-cells = <1>;
};



Driver need to specify the cable ID as
 Cable  ID
 
  Mechanical0
  Microphone   1
  Headphone   2
  Line-out3

Here #extcon-cells is must and specifies the size of extcon cells. 
The client need to provide the driver specific information as argument 
along with handle.



Extcon Client node:

audio-controller@b {
   
extcon-cables = < 1>, < 3>;
extcon-cable-names = "Microphone", "Line-out";
};

and client driver can register the cable by passing the cable name
as above along with its node.

struct extcon_cable {
struct extcon_dev *edev,
int cable_id;
};

edev_mic_cable = extcon_get_extcon_cable(dev, "Microphone");
extcon_register_notification(edev_mic_cable, notifier);

edev_line_out_cable = extcon_get_extcon_cable(dev, "Line-out");
extcon_register_notification(edev_line_out_cable, notifier);


Prototype:
struct extcon_cable *extcon_get_extcon_cable(struct device *dev, const 
char *cable_name)

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
> Hi Rob,
>
> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
>> On 2016년 05월 28일 00:29, Rob Herring wrote:
>>> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
 Add the support for Device tree bindings of extcon-gpio driver.
 The extcon-gpio device tree node must include the both 'extcon-id' and
 'gpios' property.
>>>
>>> I think extcon bindings are a mess in general...
>>>
 For example:
 usb_cable: extcon-gpio-0 {
 compatible = "extcon-gpio";
 extcon-id = ;
 gpios = < 1 GPIO_ACTIVE_HIGH>;
 }
 ta_cable: extcon-gpio-1 {
 compatible = "extcon-gpio";
 extcon-id = ;
 gpios = < 2 GPIO_ACTIVE_LOW>;
 debounce-ms = <50>; /* 50 millisecond */
 wakeup-source;
 }
>>>
>>> This is all 1 logical connector, the USB connector. Why are you
>>> describing cables? Those are not part of the h/w and are dynamic.
>>> Describe this as a connector which is one thing (i.e. node). Use a
>>> compatible string that reflects the type of connector
>>> (usb-microab-connector), not the driver you want to use. Then define
>>> GPIO lines needed to provide state information like VBus, ID, charger
>>> modes and control lines like soft connect (D+ pullup enable), VBus
>>> enable, etc.
>>
>> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
>> compatible.
>> As you commented[1], the each connector will have the unique name to use the 
>> extcon-gpio.c driver.
>> [1] https://lkml.org/lkml/2015/10/21/906
>>
>>
>> For example,
>> The extcon-gpio.c driver may have the different name including the h/w 
>> information
>> according to the kind of external connector.
>>
>> static const struct of_device_id gpio_extcon_of_match[] = {
>>   {
>>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>>   .data = EXTCON_CHG_SDP_DATA,
>>   }, {
>>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>>   .data = EXTCON_CHG_DCP_DATA,
>>   }, {
>>   .compatible = "extcon-jack-microphone", /* Microphone jack 
>> connector */
>>   .data = EXTCON_JACK_MICROPHONE_DATA,
>>   }, {
>>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>>   .data = EXTCON_DISP_HDMI_DATA,
>>   },
>>   ..
>> };
>
> I reply it again.
>
> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
> [1] drivers/input/keyboard/gpio_keys.c

There is a big difference in that each gpio-key is independent. The
only state is pressed or not. A USB connector has multiple pieces of
state information. You may be treating them independently, but I don't
think they should be.

> The gpio_keys.c driver use the following style to support the device-tree.
> It use the "gpio-keys" compatible and this dt node include the specific
> 'key code' such as 'extcon-id = ;'

This is state information about what is currently attached. The
analogy with gpio-keys would be multiple key codes on one gpio which
would be broken...

>
> gpio_keys {
> compatible = "gpio-keys";
>
> power_key {
> gpios = < 7 GPIO_ACTIVE_LOW>;
> linux,code = ;
> label = "power key";
> debounce-interval = <10>;
> wakeup-source;
> };
> };
>
> If the extcon-gpio.c driver should have the separate compatible according to
> the kind of external connector, the list of compatible name of extcon-gpio.c 
> driver
> will be increased when new external connector is attached.

So? Different h/w needs different compatible strings.

But again, you are mixing describing the connector (only what is
soldered on a board) and state information (what is attached). Do not
put state information into DT (describe the gpio signals or chip that
provides the state information).

> The extcon-gpio.c driver can separate the kind of external connector
> by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Tue, May 31, 2016 at 2:35 AM, Chanwoo Choi  wrote:
> Hi Rob,
>
> On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
>> On 2016년 05월 28일 00:29, Rob Herring wrote:
>>> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
 Add the support for Device tree bindings of extcon-gpio driver.
 The extcon-gpio device tree node must include the both 'extcon-id' and
 'gpios' property.
>>>
>>> I think extcon bindings are a mess in general...
>>>
 For example:
 usb_cable: extcon-gpio-0 {
 compatible = "extcon-gpio";
 extcon-id = ;
 gpios = < 1 GPIO_ACTIVE_HIGH>;
 }
 ta_cable: extcon-gpio-1 {
 compatible = "extcon-gpio";
 extcon-id = ;
 gpios = < 2 GPIO_ACTIVE_LOW>;
 debounce-ms = <50>; /* 50 millisecond */
 wakeup-source;
 }
>>>
>>> This is all 1 logical connector, the USB connector. Why are you
>>> describing cables? Those are not part of the h/w and are dynamic.
>>> Describe this as a connector which is one thing (i.e. node). Use a
>>> compatible string that reflects the type of connector
>>> (usb-microab-connector), not the driver you want to use. Then define
>>> GPIO lines needed to provide state information like VBus, ID, charger
>>> modes and control lines like soft connect (D+ pullup enable), VBus
>>> enable, etc.
>>
>> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
>> compatible.
>> As you commented[1], the each connector will have the unique name to use the 
>> extcon-gpio.c driver.
>> [1] https://lkml.org/lkml/2015/10/21/906
>>
>>
>> For example,
>> The extcon-gpio.c driver may have the different name including the h/w 
>> information
>> according to the kind of external connector.
>>
>> static const struct of_device_id gpio_extcon_of_match[] = {
>>   {
>>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>>   .data = EXTCON_CHG_SDP_DATA,
>>   }, {
>>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>>   .data = EXTCON_CHG_DCP_DATA,
>>   }, {
>>   .compatible = "extcon-jack-microphone", /* Microphone jack 
>> connector */
>>   .data = EXTCON_JACK_MICROPHONE_DATA,
>>   }, {
>>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>>   .data = EXTCON_DISP_HDMI_DATA,
>>   },
>>   ..
>> };
>
> I reply it again.
>
> The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
> [1] drivers/input/keyboard/gpio_keys.c

There is a big difference in that each gpio-key is independent. The
only state is pressed or not. A USB connector has multiple pieces of
state information. You may be treating them independently, but I don't
think they should be.

> The gpio_keys.c driver use the following style to support the device-tree.
> It use the "gpio-keys" compatible and this dt node include the specific
> 'key code' such as 'extcon-id = ;'

This is state information about what is currently attached. The
analogy with gpio-keys would be multiple key codes on one gpio which
would be broken...

>
> gpio_keys {
> compatible = "gpio-keys";
>
> power_key {
> gpios = < 7 GPIO_ACTIVE_LOW>;
> linux,code = ;
> label = "power key";
> debounce-interval = <10>;
> wakeup-source;
> };
> };
>
> If the extcon-gpio.c driver should have the separate compatible according to
> the kind of external connector, the list of compatible name of extcon-gpio.c 
> driver
> will be increased when new external connector is attached.

So? Different h/w needs different compatible strings.

But again, you are mixing describing the connector (only what is
soldered on a board) and state information (what is attached). Do not
put state information into DT (describe the gpio signals or chip that
provides the state information).

> The extcon-gpio.c driver can separate the kind of external connector
> by using the 'extcon-id' property.

This use of DT is just broken. Come up with another way.

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Rob,

On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
> On 2016년 05월 28일 00:29, Rob Herring wrote:
>> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>>> Add the support for Device tree bindings of extcon-gpio driver.
>>> The extcon-gpio device tree node must include the both 'extcon-id' and
>>> 'gpios' property.
>>
>> I think extcon bindings are a mess in general...
>>
>>> For example:
>>> usb_cable: extcon-gpio-0 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>>> }
>>> ta_cable: extcon-gpio-1 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 2 GPIO_ACTIVE_LOW>;
>>> debounce-ms = <50>; /* 50 millisecond */
>>> wakeup-source;
>>> }
>>
>> This is all 1 logical connector, the USB connector. Why are you 
>> describing cables? Those are not part of the h/w and are dynamic. 
>> Describe this as a connector which is one thing (i.e. node). Use a 
>> compatible string that reflects the type of connector 
>> (usb-microab-connector), not the driver you want to use. Then define 
>> GPIO lines needed to provide state information like VBus, ID, charger 
>> modes and control lines like soft connect (D+ pullup enable), VBus 
>> enable, etc.
> 
> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
> compatible.
> As you commented[1], the each connector will have the unique name to use the 
> extcon-gpio.c driver.
> [1] https://lkml.org/lkml/2015/10/21/906
> 
> 
> For example,
> The extcon-gpio.c driver may have the different name including the h/w 
> information
> according to the kind of external connector.
> 
> static const struct of_device_id gpio_extcon_of_match[] = {
>   {
>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>   .data = EXTCON_CHG_SDP_DATA,
>   }, {
>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>   .data = EXTCON_CHG_DCP_DATA,
>   }, {
>   .compatible = "extcon-jack-microphone", /* Microphone jack 
> connector */
>   .data = EXTCON_JACK_MICROPHONE_DATA,
>   }, {
>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>   .data = EXTCON_DISP_HDMI_DATA,
>   },
>   ..
> };

I reply it again.

The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
[1] drivers/input/keyboard/gpio_keys.c

The gpio_keys.c driver use the following style to support the device-tree.
It use the "gpio-keys" compatible and this dt node include the specific
'key code' such as 'extcon-id = ;'

gpio_keys {
compatible = "gpio-keys";

power_key {
gpios = < 7 GPIO_ACTIVE_LOW>;
linux,code = ;
label = "power key";
debounce-interval = <10>;
wakeup-source;
};
};

If the extcon-gpio.c driver should have the separate compatible according to
the kind of external connector, the list of compatible name of extcon-gpio.c 
driver
will be increased when new external connector is attached.

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property. 

Thanks,
Chanwoo Choi

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Rob,

On 2016년 05월 31일 15:44, Chanwoo Choi wrote:
> On 2016년 05월 28일 00:29, Rob Herring wrote:
>> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>>> Add the support for Device tree bindings of extcon-gpio driver.
>>> The extcon-gpio device tree node must include the both 'extcon-id' and
>>> 'gpios' property.
>>
>> I think extcon bindings are a mess in general...
>>
>>> For example:
>>> usb_cable: extcon-gpio-0 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>>> }
>>> ta_cable: extcon-gpio-1 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 2 GPIO_ACTIVE_LOW>;
>>> debounce-ms = <50>; /* 50 millisecond */
>>> wakeup-source;
>>> }
>>
>> This is all 1 logical connector, the USB connector. Why are you 
>> describing cables? Those are not part of the h/w and are dynamic. 
>> Describe this as a connector which is one thing (i.e. node). Use a 
>> compatible string that reflects the type of connector 
>> (usb-microab-connector), not the driver you want to use. Then define 
>> GPIO lines needed to provide state information like VBus, ID, charger 
>> modes and control lines like soft connect (D+ pullup enable), VBus 
>> enable, etc.
> 
> You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
> compatible.
> As you commented[1], the each connector will have the unique name to use the 
> extcon-gpio.c driver.
> [1] https://lkml.org/lkml/2015/10/21/906
> 
> 
> For example,
> The extcon-gpio.c driver may have the different name including the h/w 
> information
> according to the kind of external connector.
> 
> static const struct of_device_id gpio_extcon_of_match[] = {
>   {
>   .compatible = "extcon-chg-sdp", /* SDP charger connector */
>   .data = EXTCON_CHG_SDP_DATA,
>   }, {
>   .compatible = "extcon-chg-dcp", /* DCP charger connector */
>   .data = EXTCON_CHG_DCP_DATA,
>   }, {
>   .compatible = "extcon-jack-microphone", /* Microphone jack 
> connector */
>   .data = EXTCON_JACK_MICROPHONE_DATA,
>   }, {
>   .compatible = "extcon-disp-hdmi", /* HDMI connector*/
>   .data = EXTCON_DISP_HDMI_DATA,
>   },
>   ..
> };

I reply it again.

The extcon-gpio.c is very similar with existing gpio_keys.c driver[1]
[1] drivers/input/keyboard/gpio_keys.c

The gpio_keys.c driver use the following style to support the device-tree.
It use the "gpio-keys" compatible and this dt node include the specific
'key code' such as 'extcon-id = ;'

gpio_keys {
compatible = "gpio-keys";

power_key {
gpios = < 7 GPIO_ACTIVE_LOW>;
linux,code = ;
label = "power key";
debounce-interval = <10>;
wakeup-source;
};
};

If the extcon-gpio.c driver should have the separate compatible according to
the kind of external connector, the list of compatible name of extcon-gpio.c 
driver
will be increased when new external connector is attached.

The extcon-gpio.c driver can separate the kind of external connector
by using the 'extcon-id' property. 

Thanks,
Chanwoo Choi

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Venkat,

On 2016년 05월 27일 14:13, Venkat Reddy Talla wrote:
> Hi Choi,
> 
> Sorry for changing author, will update author field with your name.
> 
> Regarding Rob Herring comments, You had already replied.

Rob gave some comment which the compatible string of extcon-gpio.c should
include the type of external connector. I replied about it [1].
[1] https://lkml.org/lkml/2016/5/31/109

> 
> I felt separate compatible for each external connector is not required,
> as client driver can detect the type of external cable(sdp,dcp, microphone) 
> on receiving notification from extcon provider,

You're right about the operation point of view.
But, As Rob gave some comment, The device-tree should include the device 
information
instead of driver information. The 'extcon-gpio' compatible mean the only driver
without h/w information. 

I think that there is more discussion how to
decide the compatible name of extcon-gpio.c driver.  

> I have also added more description for wakeup-source.

OK.

> 
> Do you see any other changes required on top of v4 patch?

Regards,
Chanwoo Choi

> 
> Regards,
> Venkat
> 
>> -Original Message-
>> From: Chanwoo Choi [mailto:cwcho...@gmail.com]
>> Sent: Thursday, May 26, 2016 6:52 PM
>> To: Venkat Reddy Talla
>> Cc: MyungJoo Ham; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
>> devicetree; Kumar Gala; linux-kernel; Laxman Dewangan
>> Subject: Re: [PATCH v4] extcon: gpio: Add the support for Device tree
>> bindings
>>
>> Hi Venkat,
>>
>> There is some miscommunication. On previous my reply, I don't mean that
>> the author of the patch[1] is changed from me to you.
>>
>> I'd like you to remain the original author(me) for the patch[1] without
>> changing the author.
>> [1] https://lkml.org/lkml/2015/10/21/8
>> - [PATCH v3] extcon: gpio: Add the support for Device tree bindings
>>
>> You can use the patch[1] as based patch and you can add new feature on
>> base patch[1]. Also, If you ok, you can modify the extccon-gpio.c driver as
>> Rob comment[2].
>>
>> But, Rob Herring gave me the some comment[2].
>> [2] https://lkml.org/lkml/2015/10/21/906
>>
>>
>> Thanks,
>> Chanwoo Choi
>>
>>
>> On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
>> <vreddyta...@nvidia.com> wrote:
>>> Add the support for Device tree bindings of extcon-gpio driver.
>>> The extcon-gpio device tree node must include the both 'extcon-id' and
>>> 'gpios' property.
>>>
>>> For example:
>>> usb_cable: extcon-gpio-0 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>>> }
>>> ta_cable: extcon-gpio-1 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 2 GPIO_ACTIVE_LOW>;
>>> debounce-ms = <50>; /* 50 millisecond */
>>> wakeup-source;
>>> }
>>> _usb {
>>> extcon = <_cable>;
>>> };
>>>  {
>>> extcon = <_cable>;
>>> };
>>>
>>> Signed-off-by: Venkat Reddy Talla <vreddyta...@nvidia.com>
>>> Signed-off-by: Chanwoo Choi <cw00.c...@samsung.com>
>>> Signed-off-by: MyungJoo Ham <myungjoo@samsung.com>
>>>
>>> ---
>>> changes from v3:
>>> - add description for wakeup-source in documentation
>>> - change dts property extcon-gpio name to gpios
>>> - use of_get_named_gpio_flags to get gpio number and flags Changes
>>> from v2:
>>> - Add the more description for 'extcon-id' property in documentation
>>> Changes from v1:
>>> - Create the include/dt-bindings/extcon/extcon.h including the
>>> identification of external connector. These definitions are used in dts 
>>> file.
>>> - Fix error if CONFIG_OF is disabled.
>>> - Add signed-off tag by Myungjoo Ham
>>> ---
>>> ---
>>>  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
>>>  drivers/extcon/extcon-gpio.c   | 109 
>>> +
>>>  include/dt-bindings/extcon/extcon.h|  47 +
>>>  include/linux/extcon/extcon-gpio.h |   8 +-
>>>  4 files changed, 189 insertions(+), 23 deletions(-)  create mode
>>> 100644 Documentation/devicetree/bindings/extcon/extcon-

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Venkat,

On 2016년 05월 27일 14:13, Venkat Reddy Talla wrote:
> Hi Choi,
> 
> Sorry for changing author, will update author field with your name.
> 
> Regarding Rob Herring comments, You had already replied.

Rob gave some comment which the compatible string of extcon-gpio.c should
include the type of external connector. I replied about it [1].
[1] https://lkml.org/lkml/2016/5/31/109

> 
> I felt separate compatible for each external connector is not required,
> as client driver can detect the type of external cable(sdp,dcp, microphone) 
> on receiving notification from extcon provider,

You're right about the operation point of view.
But, As Rob gave some comment, The device-tree should include the device 
information
instead of driver information. The 'extcon-gpio' compatible mean the only driver
without h/w information. 

I think that there is more discussion how to
decide the compatible name of extcon-gpio.c driver.  

> I have also added more description for wakeup-source.

OK.

> 
> Do you see any other changes required on top of v4 patch?

Regards,
Chanwoo Choi

> 
> Regards,
> Venkat
> 
>> -Original Message-
>> From: Chanwoo Choi [mailto:cwcho...@gmail.com]
>> Sent: Thursday, May 26, 2016 6:52 PM
>> To: Venkat Reddy Talla
>> Cc: MyungJoo Ham; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
>> devicetree; Kumar Gala; linux-kernel; Laxman Dewangan
>> Subject: Re: [PATCH v4] extcon: gpio: Add the support for Device tree
>> bindings
>>
>> Hi Venkat,
>>
>> There is some miscommunication. On previous my reply, I don't mean that
>> the author of the patch[1] is changed from me to you.
>>
>> I'd like you to remain the original author(me) for the patch[1] without
>> changing the author.
>> [1] https://lkml.org/lkml/2015/10/21/8
>> - [PATCH v3] extcon: gpio: Add the support for Device tree bindings
>>
>> You can use the patch[1] as based patch and you can add new feature on
>> base patch[1]. Also, If you ok, you can modify the extccon-gpio.c driver as
>> Rob comment[2].
>>
>> But, Rob Herring gave me the some comment[2].
>> [2] https://lkml.org/lkml/2015/10/21/906
>>
>>
>> Thanks,
>> Chanwoo Choi
>>
>>
>> On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
>>  wrote:
>>> Add the support for Device tree bindings of extcon-gpio driver.
>>> The extcon-gpio device tree node must include the both 'extcon-id' and
>>> 'gpios' property.
>>>
>>> For example:
>>> usb_cable: extcon-gpio-0 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 1 GPIO_ACTIVE_HIGH>;
>>> }
>>> ta_cable: extcon-gpio-1 {
>>> compatible = "extcon-gpio";
>>> extcon-id = ;
>>> gpios = < 2 GPIO_ACTIVE_LOW>;
>>> debounce-ms = <50>; /* 50 millisecond */
>>> wakeup-source;
>>> }
>>> _usb {
>>> extcon = <_cable>;
>>> };
>>>  {
>>> extcon = <_cable>;
>>> };
>>>
>>> Signed-off-by: Venkat Reddy Talla 
>>> Signed-off-by: Chanwoo Choi 
>>> Signed-off-by: MyungJoo Ham 
>>>
>>> ---
>>> changes from v3:
>>> - add description for wakeup-source in documentation
>>> - change dts property extcon-gpio name to gpios
>>> - use of_get_named_gpio_flags to get gpio number and flags Changes
>>> from v2:
>>> - Add the more description for 'extcon-id' property in documentation
>>> Changes from v1:
>>> - Create the include/dt-bindings/extcon/extcon.h including the
>>> identification of external connector. These definitions are used in dts 
>>> file.
>>> - Fix error if CONFIG_OF is disabled.
>>> - Add signed-off tag by Myungjoo Ham
>>> ---
>>> ---
>>>  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
>>>  drivers/extcon/extcon-gpio.c   | 109 
>>> +
>>>  include/dt-bindings/extcon/extcon.h|  47 +
>>>  include/linux/extcon/extcon-gpio.h |   8 +-
>>>  4 files changed, 189 insertions(+), 23 deletions(-)  create mode
>>> 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>>>  create mode 100644 include/dt-bindings/extcon/extcon.h
>>>
>>> diff --git a/Document

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

On 2016년 05월 28일 00:29, Rob Herring wrote:
> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>> Add the support for Device tree bindings of extcon-gpio driver.
>> The extcon-gpio device tree node must include the both 'extcon-id' and
>> 'gpios' property.
> 
> I think extcon bindings are a mess in general...
> 
>> For example:
>>  usb_cable: extcon-gpio-0 {
>>  compatible = "extcon-gpio";
>>  extcon-id = ;
>>  gpios = < 1 GPIO_ACTIVE_HIGH>;
>>  }
>>  ta_cable: extcon-gpio-1 {
>>  compatible = "extcon-gpio";
>>  extcon-id = ;
>>  gpios = < 2 GPIO_ACTIVE_LOW>;
>>  debounce-ms = <50>; /* 50 millisecond */
>>  wakeup-source;
>>  }
> 
> This is all 1 logical connector, the USB connector. Why are you 
> describing cables? Those are not part of the h/w and are dynamic. 
> Describe this as a connector which is one thing (i.e. node). Use a 
> compatible string that reflects the type of connector 
> (usb-microab-connector), not the driver you want to use. Then define 
> GPIO lines needed to provide state information like VBus, ID, charger 
> modes and control lines like soft connect (D+ pullup enable), VBus 
> enable, etc.

You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
compatible.
As you commented[1], the each connector will have the unique name to use the 
extcon-gpio.c driver.
[1] https://lkml.org/lkml/2015/10/21/906


For example,
The extcon-gpio.c driver may have the different name including the h/w 
information
according to the kind of external connector.

static const struct of_device_id gpio_extcon_of_match[] = {
{
.compatible = "extcon-chg-sdp", /* SDP charger connector */
.data = EXTCON_CHG_SDP_DATA,
}, {
.compatible = "extcon-chg-dcp", /* DCP charger connector */
.data = EXTCON_CHG_DCP_DATA,
}, {
.compatible = "extcon-jack-microphone", /* Microphone jack 
connector */
.data = EXTCON_JACK_MICROPHONE_DATA,
}, {
.compatible = "extcon-disp-hdmi", /* HDMI connector*/
.data = EXTCON_DISP_HDMI_DATA,
},
..
};

Thanks,
Chanwoo Choi

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

On 2016년 05월 28일 00:29, Rob Herring wrote:
> On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
>> Add the support for Device tree bindings of extcon-gpio driver.
>> The extcon-gpio device tree node must include the both 'extcon-id' and
>> 'gpios' property.
> 
> I think extcon bindings are a mess in general...
> 
>> For example:
>>  usb_cable: extcon-gpio-0 {
>>  compatible = "extcon-gpio";
>>  extcon-id = ;
>>  gpios = < 1 GPIO_ACTIVE_HIGH>;
>>  }
>>  ta_cable: extcon-gpio-1 {
>>  compatible = "extcon-gpio";
>>  extcon-id = ;
>>  gpios = < 2 GPIO_ACTIVE_LOW>;
>>  debounce-ms = <50>; /* 50 millisecond */
>>  wakeup-source;
>>  }
> 
> This is all 1 logical connector, the USB connector. Why are you 
> describing cables? Those are not part of the h/w and are dynamic. 
> Describe this as a connector which is one thing (i.e. node). Use a 
> compatible string that reflects the type of connector 
> (usb-microab-connector), not the driver you want to use. Then define 
> GPIO lines needed to provide state information like VBus, ID, charger 
> modes and control lines like soft connect (D+ pullup enable), VBus 
> enable, etc.

You're right. The extcon-gpio driver will not use the "extcon-gpio" raw 
compatible.
As you commented[1], the each connector will have the unique name to use the 
extcon-gpio.c driver.
[1] https://lkml.org/lkml/2015/10/21/906


For example,
The extcon-gpio.c driver may have the different name including the h/w 
information
according to the kind of external connector.

static const struct of_device_id gpio_extcon_of_match[] = {
{
.compatible = "extcon-chg-sdp", /* SDP charger connector */
.data = EXTCON_CHG_SDP_DATA,
}, {
.compatible = "extcon-chg-dcp", /* DCP charger connector */
.data = EXTCON_CHG_DCP_DATA,
}, {
.compatible = "extcon-jack-microphone", /* Microphone jack 
connector */
.data = EXTCON_JACK_MICROPHONE_DATA,
}, {
.compatible = "extcon-disp-hdmi", /* HDMI connector*/
.data = EXTCON_DISP_HDMI_DATA,
},
..
};

Thanks,
Chanwoo Choi

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.

I think extcon bindings are a mess in general...

> For example:
>   usb_cable: extcon-gpio-0 {
>   compatible = "extcon-gpio";
>   extcon-id = ;
>   gpios = < 1 GPIO_ACTIVE_HIGH>;
>   }
>   ta_cable: extcon-gpio-1 {
>   compatible = "extcon-gpio";
>   extcon-id = ;
>   gpios = < 2 GPIO_ACTIVE_LOW>;
>   debounce-ms = <50>; /* 50 millisecond */
>   wakeup-source;
>   }

This is all 1 logical connector, the USB connector. Why are you 
describing cables? Those are not part of the h/w and are dynamic. 
Describe this as a connector which is one thing (i.e. node). Use a 
compatible string that reflects the type of connector 
(usb-microab-connector), not the driver you want to use. Then define 
GPIO lines needed to provide state information like VBus, ID, charger 
modes and control lines like soft connect (D+ pullup enable), VBus 
enable, etc.

>   _usb {
>   extcon = <_cable>;
>   };
>{
>   extcon = <_cable>;
>   };

Rob

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Rob Herring]

On Thu, May 26, 2016 at 05:17:45PM +0530, Venkat Reddy Talla wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.

I think extcon bindings are a mess in general...

> For example:
>   usb_cable: extcon-gpio-0 {
>   compatible = "extcon-gpio";
>   extcon-id = ;
>   gpios = < 1 GPIO_ACTIVE_HIGH>;
>   }
>   ta_cable: extcon-gpio-1 {
>   compatible = "extcon-gpio";
>   extcon-id = ;
>   gpios = < 2 GPIO_ACTIVE_LOW>;
>   debounce-ms = <50>; /* 50 millisecond */
>   wakeup-source;
>   }

This is all 1 logical connector, the USB connector. Why are you 
describing cables? Those are not part of the h/w and are dynamic. 
Describe this as a connector which is one thing (i.e. node). Use a 
compatible string that reflects the type of connector 
(usb-microab-connector), not the driver you want to use. Then define 
GPIO lines needed to provide state information like VBus, ID, charger 
modes and control lines like soft connect (D+ pullup enable), VBus 
enable, etc.

>   _usb {
>   extcon = <_cable>;
>   };
>{
>   extcon = <_cable>;
>   };

Rob

RE: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Venkat Reddy Talla]

Hi Choi,

Sorry for changing author, will update author field with your name.

Regarding Rob Herring comments, You had already replied.

I felt separate compatible for each external connector is not required,
as client driver can detect the type of external cable(sdp,dcp, microphone) on 
receiving notification from extcon provider,
I have also added more description for wakeup-source.

Do you see any other changes required on top of v4 patch?

Regards,
Venkat

> -Original Message-
> From: Chanwoo Choi [mailto:cwcho...@gmail.com]
> Sent: Thursday, May 26, 2016 6:52 PM
> To: Venkat Reddy Talla
> Cc: MyungJoo Ham; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
> devicetree; Kumar Gala; linux-kernel; Laxman Dewangan
> Subject: Re: [PATCH v4] extcon: gpio: Add the support for Device tree
> bindings
> 
> Hi Venkat,
> 
> There is some miscommunication. On previous my reply, I don't mean that
> the author of the patch[1] is changed from me to you.
> 
> I'd like you to remain the original author(me) for the patch[1] without
> changing the author.
> [1] https://lkml.org/lkml/2015/10/21/8
> - [PATCH v3] extcon: gpio: Add the support for Device tree bindings
> 
> You can use the patch[1] as based patch and you can add new feature on
> base patch[1]. Also, If you ok, you can modify the extccon-gpio.c driver as
> Rob comment[2].
> 
> But, Rob Herring gave me the some comment[2].
> [2] https://lkml.org/lkml/2015/10/21/906
> 
> 
> Thanks,
> Chanwoo Choi
> 
> 
> On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
> <vreddyta...@nvidia.com> wrote:
> > Add the support for Device tree bindings of extcon-gpio driver.
> > The extcon-gpio device tree node must include the both 'extcon-id' and
> > 'gpios' property.
> >
> > For example:
> > usb_cable: extcon-gpio-0 {
> > compatible = "extcon-gpio";
> > extcon-id = ;
> > gpios = < 1 GPIO_ACTIVE_HIGH>;
> > }
> > ta_cable: extcon-gpio-1 {
> > compatible = "extcon-gpio";
> > extcon-id = ;
> > gpios = < 2 GPIO_ACTIVE_LOW>;
> > debounce-ms = <50>; /* 50 millisecond */
> > wakeup-source;
> > }
> > _usb {
> > extcon = <_cable>;
> > };
> >  {
> > extcon = <_cable>;
> > };
> >
> > Signed-off-by: Venkat Reddy Talla <vreddyta...@nvidia.com>
> > Signed-off-by: Chanwoo Choi <cw00.c...@samsung.com>
> > Signed-off-by: MyungJoo Ham <myungjoo@samsung.com>
> >
> > ---
> > changes from v3:
> > - add description for wakeup-source in documentation
> > - change dts property extcon-gpio name to gpios
> > - use of_get_named_gpio_flags to get gpio number and flags Changes
> > from v2:
> > - Add the more description for 'extcon-id' property in documentation
> > Changes from v1:
> > - Create the include/dt-bindings/extcon/extcon.h including the
> > identification of external connector. These definitions are used in dts 
> > file.
> > - Fix error if CONFIG_OF is disabled.
> > - Add signed-off tag by Myungjoo Ham
> > ---
> > ---
> >  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
> >  drivers/extcon/extcon-gpio.c   | 109 
> > +
> >  include/dt-bindings/extcon/extcon.h|  47 +
> >  include/linux/extcon/extcon-gpio.h |   8 +-
> >  4 files changed, 189 insertions(+), 23 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> >  create mode 100644 include/dt-bindings/extcon/extcon.h
> >
> > diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > new file mode 100644
> > index 000..81f7932
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > @@ -0,0 +1,48 @@
> > +GPIO Extcon device
> > +
> > +This is a virtual device used to generate the specific cable states
> > +from the GPIO pin.
> > +
> > +Required properties:
> > +- compatible: Must be "extcon-gpio".
> > +- extcon-id: The extcon support the various type of external
> > +connector to check
> > +  whether connector is attached or detached. The each external
> > +connector has
> > +  the unique number to identify it. So this property includes the
> > +un

RE: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Venkat Reddy Talla]

Hi Choi,

Sorry for changing author, will update author field with your name.

Regarding Rob Herring comments, You had already replied.

I felt separate compatible for each external connector is not required,
as client driver can detect the type of external cable(sdp,dcp, microphone) on 
receiving notification from extcon provider,
I have also added more description for wakeup-source.

Do you see any other changes required on top of v4 patch?

Regards,
Venkat

> -Original Message-
> From: Chanwoo Choi [mailto:cwcho...@gmail.com]
> Sent: Thursday, May 26, 2016 6:52 PM
> To: Venkat Reddy Talla
> Cc: MyungJoo Ham; Rob Herring; Pawel Moll; Mark Rutland; Ian Campbell;
> devicetree; Kumar Gala; linux-kernel; Laxman Dewangan
> Subject: Re: [PATCH v4] extcon: gpio: Add the support for Device tree
> bindings
> 
> Hi Venkat,
> 
> There is some miscommunication. On previous my reply, I don't mean that
> the author of the patch[1] is changed from me to you.
> 
> I'd like you to remain the original author(me) for the patch[1] without
> changing the author.
> [1] https://lkml.org/lkml/2015/10/21/8
> - [PATCH v3] extcon: gpio: Add the support for Device tree bindings
> 
> You can use the patch[1] as based patch and you can add new feature on
> base patch[1]. Also, If you ok, you can modify the extccon-gpio.c driver as
> Rob comment[2].
> 
> But, Rob Herring gave me the some comment[2].
> [2] https://lkml.org/lkml/2015/10/21/906
> 
> 
> Thanks,
> Chanwoo Choi
> 
> 
> On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
>  wrote:
> > Add the support for Device tree bindings of extcon-gpio driver.
> > The extcon-gpio device tree node must include the both 'extcon-id' and
> > 'gpios' property.
> >
> > For example:
> > usb_cable: extcon-gpio-0 {
> > compatible = "extcon-gpio";
> > extcon-id = ;
> > gpios = < 1 GPIO_ACTIVE_HIGH>;
> > }
> > ta_cable: extcon-gpio-1 {
> > compatible = "extcon-gpio";
> > extcon-id = ;
> > gpios = < 2 GPIO_ACTIVE_LOW>;
> > debounce-ms = <50>; /* 50 millisecond */
> > wakeup-source;
> > }
> > _usb {
> > extcon = <_cable>;
> > };
> >  {
> > extcon = <_cable>;
> > };
> >
> > Signed-off-by: Venkat Reddy Talla 
> > Signed-off-by: Chanwoo Choi 
> > Signed-off-by: MyungJoo Ham 
> >
> > ---
> > changes from v3:
> > - add description for wakeup-source in documentation
> > - change dts property extcon-gpio name to gpios
> > - use of_get_named_gpio_flags to get gpio number and flags Changes
> > from v2:
> > - Add the more description for 'extcon-id' property in documentation
> > Changes from v1:
> > - Create the include/dt-bindings/extcon/extcon.h including the
> > identification of external connector. These definitions are used in dts 
> > file.
> > - Fix error if CONFIG_OF is disabled.
> > - Add signed-off tag by Myungjoo Ham
> > ---
> > ---
> >  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
> >  drivers/extcon/extcon-gpio.c   | 109 
> > +
> >  include/dt-bindings/extcon/extcon.h|  47 +
> >  include/linux/extcon/extcon-gpio.h |   8 +-
> >  4 files changed, 189 insertions(+), 23 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> >  create mode 100644 include/dt-bindings/extcon/extcon.h
> >
> > diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > new file mode 100644
> > index 000..81f7932
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> > @@ -0,0 +1,48 @@
> > +GPIO Extcon device
> > +
> > +This is a virtual device used to generate the specific cable states
> > +from the GPIO pin.
> > +
> > +Required properties:
> > +- compatible: Must be "extcon-gpio".
> > +- extcon-id: The extcon support the various type of external
> > +connector to check
> > +  whether connector is attached or detached. The each external
> > +connector has
> > +  the unique number to identify it. So this property includes the
> > +unique number
> > +  which indicates the specific external connector. When external
> > +connector is
> > + 

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Venkat,

There is some miscommunication. On previous my reply, I don't mean
that the author of the patch[1] is changed from me to you.

I'd like you to remain the original author(me) for the patch[1]
without changing the author.
[1] https://lkml.org/lkml/2015/10/21/8
- [PATCH v3] extcon: gpio: Add the support for Device tree bindings

You can use the patch[1] as based patch and you can add new feature on
base patch[1]. Also, If you ok, you can modify the extccon-gpio.c
driver as Rob comment[2].

But, Rob Herring gave me the some comment[2].
[2] https://lkml.org/lkml/2015/10/21/906


Thanks,
Chanwoo Choi


On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
 wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.
>
> For example:
> usb_cable: extcon-gpio-0 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 1 GPIO_ACTIVE_HIGH>;
> }
> ta_cable: extcon-gpio-1 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 2 GPIO_ACTIVE_LOW>;
> debounce-ms = <50>; /* 50 millisecond */
> wakeup-source;
> }
> _usb {
> extcon = <_cable>;
> };
>  {
> extcon = <_cable>;
> };
>
> Signed-off-by: Venkat Reddy Talla 
> Signed-off-by: Chanwoo Choi 
> Signed-off-by: MyungJoo Ham 
>
> ---
> changes from v3:
> - add description for wakeup-source in documentation
> - change dts property extcon-gpio name to gpios
> - use of_get_named_gpio_flags to get gpio number and flags
> Changes from v2:
> - Add the more description for 'extcon-id' property in documentation
> Changes from v1:
> - Create the include/dt-bindings/extcon/extcon.h including the identification
> of external connector. These definitions are used in dts file.
> - Fix error if CONFIG_OF is disabled.
> - Add signed-off tag by Myungjoo Ham
> ---
> ---
>  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
>  drivers/extcon/extcon-gpio.c   | 109 
> +
>  include/dt-bindings/extcon/extcon.h|  47 +
>  include/linux/extcon/extcon-gpio.h |   8 +-
>  4 files changed, 189 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>  create mode 100644 include/dt-bindings/extcon/extcon.h
>
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> new file mode 100644
> index 000..81f7932
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> @@ -0,0 +1,48 @@
> +GPIO Extcon device
> +
> +This is a virtual device used to generate the specific cable states from the
> +GPIO pin.
> +
> +Required properties:
> +- compatible: Must be "extcon-gpio".
> +- extcon-id: The extcon support the various type of external connector to 
> check
> +  whether connector is attached or detached. The each external connector has
> +  the unique number to identify it. So this property includes the unique 
> number
> +  which indicates the specific external connector. When external connector is
> +  attached or detached, GPIO pin detect the changed state. See include/
> +  dt-bindings/extcon/extcon.h which defines the unique number for supported
> +  external connector from extcon framework.
> +- gpios: GPIO pin to detect the external connector. See gpio binding.
> +
> +Optional properties:
> +- debounce-ms: the debounce dealy for GPIO pin in millisecond.
> +- wakeup-source: Boolean, extcon can wake-up the system from suspend.
> +  if gpio provided in extcon-gpio DT node is registered as interrupt,
> +  then extcon can wake-up the system from suspend if wakeup-source property
> +  is available in DT node, if gpio registered as interrupt but wakeup-source
> +  is not available in DT node, then system wake-up due to extcon events
> +  not supported.
> +
> +Example: Examples of extcon-gpio node as listed below:
> +
> +   usb_cable: extcon-gpio-0 {
> +   compatible = "extcon-gpio";
> +   extcon-id = ;
> +   extcon-gpio = < 1 GPIO_ACTIVE_HIGH>;
> +   }
> +
> +   ta_cable: extcon-gpio-1 {
> +   compatible = "extcon-gpio";
> +   extcon-id = ;
> +   extcon-gpio = < 2 GPIO_ACTIVE_LOW>;
> +   debounce-ms = <50>; /* 50 millisecond */
> +   wakeup-source;
> +   }
> +
> +   _usb {
> +   extcon = <_cable>;
> +   };
> +
> +{
> +   extcon = <_cable>;
> +   };
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index d023789..592f395 100644
> --- 

Re: [PATCH v4] extcon: gpio: Add the support for Device tree bindings

[Chanwoo Choi]

Hi Venkat,

There is some miscommunication. On previous my reply, I don't mean
that the author of the patch[1] is changed from me to you.

I'd like you to remain the original author(me) for the patch[1]
without changing the author.
[1] https://lkml.org/lkml/2015/10/21/8
- [PATCH v3] extcon: gpio: Add the support for Device tree bindings

You can use the patch[1] as based patch and you can add new feature on
base patch[1]. Also, If you ok, you can modify the extccon-gpio.c
driver as Rob comment[2].

But, Rob Herring gave me the some comment[2].
[2] https://lkml.org/lkml/2015/10/21/906


Thanks,
Chanwoo Choi


On Thu, May 26, 2016 at 8:47 PM, Venkat Reddy Talla
 wrote:
> Add the support for Device tree bindings of extcon-gpio driver.
> The extcon-gpio device tree node must include the both 'extcon-id' and
> 'gpios' property.
>
> For example:
> usb_cable: extcon-gpio-0 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 1 GPIO_ACTIVE_HIGH>;
> }
> ta_cable: extcon-gpio-1 {
> compatible = "extcon-gpio";
> extcon-id = ;
> gpios = < 2 GPIO_ACTIVE_LOW>;
> debounce-ms = <50>; /* 50 millisecond */
> wakeup-source;
> }
> _usb {
> extcon = <_cable>;
> };
>  {
> extcon = <_cable>;
> };
>
> Signed-off-by: Venkat Reddy Talla 
> Signed-off-by: Chanwoo Choi 
> Signed-off-by: MyungJoo Ham 
>
> ---
> changes from v3:
> - add description for wakeup-source in documentation
> - change dts property extcon-gpio name to gpios
> - use of_get_named_gpio_flags to get gpio number and flags
> Changes from v2:
> - Add the more description for 'extcon-id' property in documentation
> Changes from v1:
> - Create the include/dt-bindings/extcon/extcon.h including the identification
> of external connector. These definitions are used in dts file.
> - Fix error if CONFIG_OF is disabled.
> - Add signed-off tag by Myungjoo Ham
> ---
> ---
>  .../devicetree/bindings/extcon/extcon-gpio.txt |  48 +
>  drivers/extcon/extcon-gpio.c   | 109 
> +
>  include/dt-bindings/extcon/extcon.h|  47 +
>  include/linux/extcon/extcon-gpio.h |   8 +-
>  4 files changed, 189 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio.txt
>  create mode 100644 include/dt-bindings/extcon/extcon.h
>
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> new file mode 100644
> index 000..81f7932
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio.txt
> @@ -0,0 +1,48 @@
> +GPIO Extcon device
> +
> +This is a virtual device used to generate the specific cable states from the
> +GPIO pin.
> +
> +Required properties:
> +- compatible: Must be "extcon-gpio".
> +- extcon-id: The extcon support the various type of external connector to 
> check
> +  whether connector is attached or detached. The each external connector has
> +  the unique number to identify it. So this property includes the unique 
> number
> +  which indicates the specific external connector. When external connector is
> +  attached or detached, GPIO pin detect the changed state. See include/
> +  dt-bindings/extcon/extcon.h which defines the unique number for supported
> +  external connector from extcon framework.
> +- gpios: GPIO pin to detect the external connector. See gpio binding.
> +
> +Optional properties:
> +- debounce-ms: the debounce dealy for GPIO pin in millisecond.
> +- wakeup-source: Boolean, extcon can wake-up the system from suspend.
> +  if gpio provided in extcon-gpio DT node is registered as interrupt,
> +  then extcon can wake-up the system from suspend if wakeup-source property
> +  is available in DT node, if gpio registered as interrupt but wakeup-source
> +  is not available in DT node, then system wake-up due to extcon events
> +  not supported.
> +
> +Example: Examples of extcon-gpio node as listed below:
> +
> +   usb_cable: extcon-gpio-0 {
> +   compatible = "extcon-gpio";
> +   extcon-id = ;
> +   extcon-gpio = < 1 GPIO_ACTIVE_HIGH>;
> +   }
> +
> +   ta_cable: extcon-gpio-1 {
> +   compatible = "extcon-gpio";
> +   extcon-id = ;
> +   extcon-gpio = < 2 GPIO_ACTIVE_LOW>;
> +   debounce-ms = <50>; /* 50 millisecond */
> +   wakeup-source;
> +   }
> +
> +   _usb {
> +   extcon = <_cable>;
> +   };
> +
> +{
> +   extcon = <_cable>;
> +   };
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index d023789..592f395 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -1,11 +1,9 @@
>  /*
>   *