Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Heikki Krogerus]

On Thu, Mar 08, 2018 at 01:41:23AM +, Jun Li wrote:
> > If you are asking my opinion, the data role must be handled as separate
> > capability of the port, i.e. you probable want to have separate properties 
> > for
> > the power role and data role, even if we did not support that yet in the 
> > class
> > code. Don't use "port-type" name, just call them "power-role" and
> > "data-role" from now on.
> > 
> 
> OK, I will introduce power-role and data-role, meanwhile, I think the class 
> code
> should be changed accordingly(looks like your attached patch is doing that).

True. I'll send the patch out now.


Thanks,

-- 
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

Hi
> -Original Message-
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: 2018年3月6日 20:02
> To: Jun Li <jun...@nxp.com>
> Cc: Andrzej Hajda <a.ha...@samsung.com>; gre...@linuxfoundation.org;
> robh...@kernel.org; li...@roeck-us.net; mark.rutl...@arm.com;
> yue...@google.com; Peter Chen <peter.c...@nxp.com>;
> garsi...@embeddedor.com; o_leve...@orange.fr;
> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> Hi guys,
> 
> On Tue, Mar 06, 2018 at 09:38:59AM +, Jun Li wrote:
> > > >>> diff --git
> > > >>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > >>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > >>> index e1463f1..242f6df 100644
> > > >>> ---
> > > >>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > >>> +++
> > > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > >>> @@ -15,6 +15,30 @@ Optional properties:
> > > >>>  - type: size of the connector, should be specified in case of
> > > >>> USB-A,
> > > USB-B
> > > >>>non-fullsize connectors: "mini", "micro".
> > > >>>
> > > >>> +Required properties for usb-c-connector with power delivery
> support:
> > > >>> +- port-type: should be one of "source", "sink" or "dual".
> > > >>> +- default-role: preferred power role if port-type is
> > > >>> +"dual"(drp), should be
> > > >>> +  "sink" or "source".
> > > >> Since port carries data and power, it would be better to
> > > >> explicitly mention it in names of properties which can be ambiguous.
> > > >> Maybe instead of 'port-type' you can use 'power-role', for example.
> > > > Port-type is align with the name of typec coding and ABI, 'power-role'
> > > > is more like for the current role rather than the port's ability.
> > >
> > > I am not sure what are you exactly mean by "coding and ABI", but I
> > > guess it is just about Power-Delivery part of USB-C. And if you want
> > > to put this property into USB node without mark it is related to PD
> > > part of USB connector, you risk confusion with possible USB data related
> properties.
> >
> > Understood your concern, I am following typec naming conventions, for
> > typec, port-type property has the same meaning as
> > /sys/class/typec/portx/port_type which is not only for power, also for
> > data, there are dedicated sys files power_role for power and data_role
> > for data.
> >
> > > Simple question, what if somebody wants to add property describing
> > > USB data capabilities (DFP, UFP, DRD), how should it be named?
> > >
> >
> > Then we may use data-role?
> >
> > > >
> > > >> Other thing is that default-role is required only in case of DRP,
> > > >> so maybe better would be to squash it in power-role, for example:
> > > >> ?? power-role: should be on of "source", "sink",
> > > >> "dual-source", "dual-sink", in case of dual roles suffix determines
> preferred role.
> > > > I don't know how much this squash can benefit, "dual-source/sink"
> > > > is not directly showing its meaning by name.
> > >
> > > Some benefit is simpler binding, no conditionally-required property.
> > >
> >
> > There is already string definition for port type and preferred role
> > parse static const char * const typec_port_types[] = {
> >  [TYPEC_PORT_DFP] = "source",
> >  [TYPEC_PORT_UFP] = "sink",
> >  [TYPEC_PORT_DRP] = "dual",
> > };
> > And I think there will be other conditionally-required properties to
> > be extended later, are we going to name all of them by this way?
> > Either way is OK for me, I am not sure if there is principle like we
> > should avoid conditionally-required property, if we should, maybe
> > "dual-preferred-source/sink" is better.
> >
> > Hi Heikki,
> > Do you have any comments/preference here?
> 
> In the first versions of USB Type-C specification the da

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Heikki Krogerus]

Hi guys,

On Tue, Mar 06, 2018 at 09:38:59AM +, Jun Li wrote:
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > >>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > >>> index e1463f1..242f6df 100644
> > >>> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > >>> +++
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > >>> @@ -15,6 +15,30 @@ Optional properties:
> > >>>  - type: size of the connector, should be specified in case of USB-A,
> > USB-B
> > >>>non-fullsize connectors: "mini", "micro".
> > >>>
> > >>> +Required properties for usb-c-connector with power delivery support:
> > >>> +- port-type: should be one of "source", "sink" or "dual".
> > >>> +- default-role: preferred power role if port-type is "dual"(drp),
> > >>> +should be
> > >>> +  "sink" or "source".
> > >> Since port carries data and power, it would be better to explicitly
> > >> mention it in names of properties which can be ambiguous.
> > >> Maybe instead of 'port-type' you can use 'power-role', for example.
> > > Port-type is align with the name of typec coding and ABI, 'power-role'
> > > is more like for the current role rather than the port's ability.
> > 
> > I am not sure what are you exactly mean by "coding and ABI", but I guess it 
> > is
> > just about Power-Delivery part of USB-C. And if you want to put this 
> > property
> > into USB node without mark it is related to PD part of USB connector, you
> > risk confusion with possible USB data related properties.
> 
> Understood your concern, I am following typec naming conventions,
> for typec, port-type property has the same meaning as
> /sys/class/typec/portx/port_type
> which is not only for power, also for data, there are dedicated
> sys files power_role for power and data_role for data.
> 
> > Simple question, what if somebody wants to add property describing USB
> > data capabilities (DFP, UFP, DRD), how should it be named?
> > 
> 
> Then we may use data-role?
> 
> > >
> > >> Other thing is that default-role is required only in case of DRP, so
> > >> maybe better would be to squash it in power-role, for example:
> > >> ?? power-role: should be on of "source", "sink", "dual-source",
> > >> "dual-sink", in case of dual roles suffix determines preferred role.
> > > I don't know how much this squash can benefit, "dual-source/sink" is
> > > not directly showing its meaning by name.
> > 
> > Some benefit is simpler binding, no conditionally-required property.
> > 
> 
> There is already string definition for port type and preferred role parse 
> static const char * const typec_port_types[] = {
>  [TYPEC_PORT_DFP] = "source",
>  [TYPEC_PORT_UFP] = "sink",
>  [TYPEC_PORT_DRP] = "dual",
> };
> And I think there will be other conditionally-required properties
> to be extended later, are we going to name all of them by this way?
> Either way is OK for me, I am not sure if there is principle like we
> should avoid conditionally-required property, if we should, maybe
> "dual-preferred-source/sink" is better.
> 
> Hi Heikki,
> Do you have any comments/preference here?

In the first versions of USB Type-C specification the data and power
roles were in practice coupled together. There were no data role
specific modes, just DFP, UFP and DRP. However, v1.2 of the spec.
introduced separate modes for the data roles as well, and I have a
patch for that (attached).

If you are asking my opinion, the data role must be handled as
separate capability of the port, i.e. you probable want to have
separate properties for the power role and data role, even if we did
not support that yet in the class code. Don't use "port-type" name,
just call them "power-role" and "data-role" from now on.

The default-role should tell the state machines whether Try.SNK or
Try.SRC states should be used or not. Perhaps you should have boolean
property for both: "try-snk" and "try-src" (note: both can not be true).

Final note. I think it would make sense to clearly separate the USB
Type-C specific properties from USB PD ones. Though it is unlikely
that we will see USB PD being used with other connector types besides
Type-C anymore, USB Type-C connectors will still definitely not
always support USB PD, but when USB PD is not supported, we still need
to know the data-role, power-role, default-role (or try-snk, try-src),
etc.


Br,

-- 
heikki
>From 4f7652ba8a3d4e8f7bd30cf7ca8beba6af5ad873 Mon Sep 17 00:00:00 2001
From: Heikki Krogerus 
Date: Tue, 2 Jan 2018 17:07:58 +0300
Subject: [PATCH] usb: typec: Separate the definitions for data and power roles

USB Type-C specification v1.2 separated the power and data
roles more clearly. Dual-Role-Data term was introduced, and
the meaning of DRP was changed from "Dual-Role-Port" to
"Dual-Role-Power".

In order to allow the port drivers to describe the
capabilities of the ports more clearly according to the
newest 

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Andrzej Hajda]

On 06.03.2018 10:38, Jun Li wrote:
> Hi
>> -Original Message-
>> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
>> Sent: 2018年3月5日 17:59
>> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
>> robh...@kernel.org; heikki.kroge...@linux.intel.com; li...@roeck-us.net
>> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
>> <peter.c...@nxp.com>; garsi...@embeddedor.com; o_leve...@orange.fr;
>> shufan_...@richtek.com; linux-usb@vger.kernel.org;
>> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
>> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
>> typec power delivery
>>
>> On 05.03.2018 08:00, Jun Li wrote:
>>>> -Original Message-
>>>> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
>>>> Sent: 2018年2月27日 16:41
>>>> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
>>>> robh...@kernel.org; heikki.kroge...@linux.intel.com;
>>>> robh+li...@roeck-us.net
>>>> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
>>>> <peter.c...@nxp.com>; garsi...@embeddedor.com;
>> o_leve...@orange.fr;
>>>> shufan_...@richtek.com; linux-usb@vger.kernel.org;
>>>> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
>>>> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties
>>>> for typec power delivery
>>>>
>>>> On 26.02.2018 12:49, Li Jun wrote:
>>>>> In case of usb-c-connector with power delivery support, add
>>>>> bingdings supported by current typec driver, so user can pass all
>>>>> those properties via dt.
>>>>>
>>>>> Signed-off-by: Li Jun <jun...@nxp.com>
>>>>> ---
>>>>> Changes for v2:
>>>>> - Added typec properties are based on general usb connector bindings[1]
>>>>>   proposed by Andrzej Hajda.
>>>>> - Use the standard unit suffixes as defined in property-units.txt.
>>>>>
>>>>> [1]
>>>>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
>>>> t
>>>>
>> chwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%40
>>>> nxp.co
>>>>
>> m%7Cccf14a36ca6445ee5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99
>>>> c5c30163
>>>>
>> 5%7C0%7C0%7C636553176880292197=2Pi0AtiwAqHQE3rGl%2Bo49K
>>>> 7yZZcp%2B
>>>>> 5bvJAknRBMGTrk%3D=0
>>>>>
>>>>>  .../bindings/connector/usb-connector.txt   | 43
>>>> ++
>>>>>  1 file changed, 43 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
>>>>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>>>> index e1463f1..242f6df 100644
>>>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
>>>>> +++
>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>>>> @@ -15,6 +15,30 @@ Optional properties:
>>>>>  - type: size of the connector, should be specified in case of USB-A,
>> USB-B
>>>>>non-fullsize connectors: "mini", "micro".
>>>>>
>>>>> +Required properties for usb-c-connector with power delivery support:
>>>>> +- port-type: should be one of "source", "sink" or "dual".
>>>>> +- default-role: preferred power role if port-type is "dual"(drp),
>>>>> +should be
>>>>> +  "sink" or "source".
>>>> Since port carries data and power, it would be better to explicitly
>>>> mention it in names of properties which can be ambiguous.
>>>> Maybe instead of 'port-type' you can use 'power-role', for example.
>>> Port-type is align with the name of typec coding and ABI, 'power-role'
>>> is more like for the current role rather than the port's ability.
>> I am not sure what are you exactly mean by "coding and ABI", but I guess it 
>> is
>> just about Power-Delivery part of USB-C. And if you want to put this property
>> into USB node without mark it is related to PD part of USB connector, you
>> risk confusion with possible USB data related properties.
> Understood your concern, I am following typec naming conventions,
> for typec, port-type property has the same meaning as
> /sys/class/typec

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

Hi
> -Original Message-
> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
> Sent: 2018年3月5日 17:59
> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
> robh...@kernel.org; heikki.kroge...@linux.intel.com; li...@roeck-us.net
> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
> <peter.c...@nxp.com>; garsi...@embeddedor.com; o_leve...@orange.fr;
> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> On 05.03.2018 08:00, Jun Li wrote:
> >
> >> -Original Message-
> >> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
> >> Sent: 2018年2月27日 16:41
> >> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
> >> robh...@kernel.org; heikki.kroge...@linux.intel.com;
> >> robh+li...@roeck-us.net
> >> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
> >> <peter.c...@nxp.com>; garsi...@embeddedor.com;
> o_leve...@orange.fr;
> >> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> >> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> >> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties
> >> for typec power delivery
> >>
> >> On 26.02.2018 12:49, Li Jun wrote:
> >>> In case of usb-c-connector with power delivery support, add
> >>> bingdings supported by current typec driver, so user can pass all
> >>> those properties via dt.
> >>>
> >>> Signed-off-by: Li Jun <jun...@nxp.com>
> >>> ---
> >>> Changes for v2:
> >>> - Added typec properties are based on general usb connector bindings[1]
> >>>   proposed by Andrzej Hajda.
> >>> - Use the standard unit suffixes as defined in property-units.txt.
> >>>
> >>> [1]
> >>>
> >>
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
> >> t
> >>
> chwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%40
> >> nxp.co
> >>
> m%7Cccf14a36ca6445ee5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99
> >> c5c30163
> >>
> 5%7C0%7C0%7C636553176880292197=2Pi0AtiwAqHQE3rGl%2Bo49K
> >> 7yZZcp%2B
> >>> 5bvJAknRBMGTrk%3D=0
> >>>
> >>>  .../bindings/connector/usb-connector.txt   | 43
> >> ++
> >>>  1 file changed, 43 insertions(+)
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
> >>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> >>> index e1463f1..242f6df 100644
> >>> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> >>> +++
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> >>> @@ -15,6 +15,30 @@ Optional properties:
> >>>  - type: size of the connector, should be specified in case of USB-A,
> USB-B
> >>>non-fullsize connectors: "mini", "micro".
> >>>
> >>> +Required properties for usb-c-connector with power delivery support:
> >>> +- port-type: should be one of "source", "sink" or "dual".
> >>> +- default-role: preferred power role if port-type is "dual"(drp),
> >>> +should be
> >>> +  "sink" or "source".
> >> Since port carries data and power, it would be better to explicitly
> >> mention it in names of properties which can be ambiguous.
> >> Maybe instead of 'port-type' you can use 'power-role', for example.
> > Port-type is align with the name of typec coding and ABI, 'power-role'
> > is more like for the current role rather than the port's ability.
> 
> I am not sure what are you exactly mean by "coding and ABI", but I guess it is
> just about Power-Delivery part of USB-C. And if you want to put this property
> into USB node without mark it is related to PD part of USB connector, you
> risk confusion with possible USB data related properties.

Understood your concern, I am following typec naming conventions,
for typec, port-type property has the same meaning as
/sys/class/typec/portx/port_type
which is not only for power, also for data, there are dedicated
sys files power_role for power and data_role for data.

> Simple question, what if somebody wants to add property describing USB
> data capabilities (DFP, UFP, DRD), how should it be named?
> 

Then we may use data-role?

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

> -Original Message-
> From: Jun Li
> Sent: 2018年3月5日 15:52
> To: Rob Herring <r...@kernel.org>; Andrzej Hajda <a.ha...@samsung.com>
> Cc: gre...@linuxfoundation.org; heikki.kroge...@linux.intel.com;
> li...@roeck-us.net; mark.rutl...@arm.com; yue...@google.com; Peter
> Chen <peter.c...@nxp.com>; garsi...@embeddedor.com;
> o_leve...@orange.fr; shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: RE: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> 
> > -Original Message-
> > From: Rob Herring [mailto:r...@kernel.org]
> > Sent: 2018年3月3日 6:39
> > To: Andrzej Hajda <a.ha...@samsung.com>
> > Cc: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
> > heikki.kroge...@linux.intel.com; li...@roeck-us.net;
> > mark.rutl...@arm.com; yue...@google.com; Peter Chen
> > <peter.c...@nxp.com>; garsi...@embeddedor.com;
> o_leve...@orange.fr;
> > shufan_...@richtek.com; linux-usb@vger.kernel.org;
> > devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> > Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties
> > for typec power delivery
> >
> > On Tue, Feb 27, 2018 at 09:41:19AM +0100, Andrzej Hajda wrote:
> > > On 26.02.2018 12:49, Li Jun wrote:
> > > > In case of usb-c-connector with power delivery support, add
> > > > bingdings supported by current typec driver, so user can pass all
> > > > those properties via dt.
> > > >
> > > > Signed-off-by: Li Jun <jun...@nxp.com>
> > > > ---
> > > > Changes for v2:
> > > > - Added typec properties are based on general usb connector
> bindings[1]
> > > >   proposed by Andrzej Hajda.
> > > > - Use the standard unit suffixes as defined in property-units.txt.
> > > >
> > > > [1]
> > > >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > > >
> >
> atchwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%
> > 40nx
> > > >
> >
> p.com%7Ccea32589f3314488e18f08d5808e5aae%7C686ea1d3bc2b4c6fa92
> > cd99c5
> > > >
> >
> c301635%7C0%7C1%7C636556271266469012=ANr1jeW8x8cy6CG6tz
> > ACgj%2B
> > > > FgNKl9DJbRpervFwaQKM%3D=0
> > > >
> > > >  .../bindings/connector/usb-connector.txt   | 43
> > ++
> > > >  1 file changed, 43 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > > index e1463f1..242f6df 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > > +++
> b/Documentation/devicetree/bindings/connector/usb-connector.tx
> > > > +++ t
> > > > @@ -15,6 +15,30 @@ Optional properties:
> > > >  - type: size of the connector, should be specified in case of
> > > > USB-A,
> > USB-B
> > > >non-fullsize connectors: "mini", "micro".
> > > >
> > > > +Required properties for usb-c-connector with power delivery support:
> > > > +- port-type: should be one of "source", "sink" or "dual".
> > > > +- default-role: preferred power role if port-type is "dual"(drp),
> > > > +should be
> > > > +  "sink" or "source".
> > >
> > > Since port carries data and power, it would be better to explicitly
> > > mention it in names of properties which can be ambiguous.
> > > Maybe instead of 'port-type' you can use 'power-role', for example.
> > > Other thing is that default-role is required only in case of DRP, so
> > > maybe better would be to squash it in power-role, for example:
> > >     power-role: should be on of "source", "sink", "dual-source",
> > > "dual-sink", in case of dual roles suffix determines preferred role.
> > >
> > >
> > > > +- src-pdos: An array of u32 with each entry providing supported
> > > > +power
> > > > +  source data object(PDO), the detailed bit definitions of PDO
> > > > +can be found
> > > > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > > > +6.4.1.2
> > > 

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Andrzej Hajda]

On 05.03.2018 08:00, Jun Li wrote:
>
>> -Original Message-
>> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
>> Sent: 2018年2月27日 16:41
>> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
>> robh...@kernel.org; heikki.kroge...@linux.intel.com; li...@roeck-us.net
>> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
>> <peter.c...@nxp.com>; garsi...@embeddedor.com; o_leve...@orange.fr;
>> shufan_...@richtek.com; linux-usb@vger.kernel.org;
>> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
>> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
>> typec power delivery
>>
>> On 26.02.2018 12:49, Li Jun wrote:
>>> In case of usb-c-connector with power delivery support, add bingdings
>>> supported by current typec driver, so user can pass all those
>>> properties via dt.
>>>
>>> Signed-off-by: Li Jun <jun...@nxp.com>
>>> ---
>>> Changes for v2:
>>> - Added typec properties are based on general usb connector bindings[1]
>>>   proposed by Andrzej Hajda.
>>> - Use the standard unit suffixes as defined in property-units.txt.
>>>
>>> [1]
>>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>> chwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%40
>> nxp.co
>> m%7Cccf14a36ca6445ee5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99
>> c5c30163
>> 5%7C0%7C0%7C636553176880292197=2Pi0AtiwAqHQE3rGl%2Bo49K
>> 7yZZcp%2B
>>> 5bvJAknRBMGTrk%3D=0
>>>
>>>  .../bindings/connector/usb-connector.txt   | 43
>> ++
>>>  1 file changed, 43 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> index e1463f1..242f6df 100644
>>> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
>>> @@ -15,6 +15,30 @@ Optional properties:
>>>  - type: size of the connector, should be specified in case of USB-A, USB-B
>>>non-fullsize connectors: "mini", "micro".
>>>
>>> +Required properties for usb-c-connector with power delivery support:
>>> +- port-type: should be one of "source", "sink" or "dual".
>>> +- default-role: preferred power role if port-type is "dual"(drp),
>>> +should be
>>> +  "sink" or "source".
>> Since port carries data and power, it would be better to explicitly mention 
>> it
>> in names of properties which can be ambiguous.
>> Maybe instead of 'port-type' you can use 'power-role', for example.
> Port-type is align with the name of typec coding and ABI, 'power-role'
> is more like for the current role rather than the port's ability.

I am not sure what are you exactly mean by "coding and ABI", but I guess
it is just about Power-Delivery part of USB-C. And if you want to put
this property into USB node without mark it is related to PD part of USB
connector, you risk confusion with possible USB data related properties.
Simple question, what if somebody wants to add property describing USB
data capabilities (DFP, UFP, DRD), how should it be named?

>
>> Other thing is that default-role is required only in case of DRP, so maybe
>> better would be to squash it in power-role, for example:
>>     power-role: should be on of "source", "sink", "dual-source", "dual-sink",
>> in case of dual roles suffix determines preferred role.
> I don't know how much this squash can benefit, "dual-source/sink" is not
> directly showing its meaning by name.

Some benefit is simpler binding, no conditionally-required property.

Another question is that USB TypeC specification describes 7 different
"behavioral models" [1]:
- Source-only
- Source (Default) – strong preference toward being a Source but
subsequently
capable of becoming a Sink using USB PD swap mechanisms.
- Sink-only
- Sink (Default) – strong preference toward being a Sink but
subsequently capable of
becoming a Source using USB PD swap mechanisms.
- DRP: Toggling (Source/Sink)
- DRP: Sourcing Device
- DRP: Sinking Host

How it maps to your proposed props?

[1]: USB Type-C specification release 1.3, chapter 4.5.1.4.

Regards
Andrzej

>
>>
>>> +- src-pdos: An array of u32 with each entry providing supported power
>>> +  source data object(PDO), the detailed bit definitions of PDO can be
>>> +found
&g

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: 2018年3月3日 6:39
> To: Andrzej Hajda <a.ha...@samsung.com>
> Cc: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
> heikki.kroge...@linux.intel.com; li...@roeck-us.net;
> mark.rutl...@arm.com; yue...@google.com; Peter Chen
> <peter.c...@nxp.com>; garsi...@embeddedor.com; o_leve...@orange.fr;
> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> On Tue, Feb 27, 2018 at 09:41:19AM +0100, Andrzej Hajda wrote:
> > On 26.02.2018 12:49, Li Jun wrote:
> > > In case of usb-c-connector with power delivery support, add
> > > bingdings supported by current typec driver, so user can pass all
> > > those properties via dt.
> > >
> > > Signed-off-by: Li Jun <jun...@nxp.com>
> > > ---
> > > Changes for v2:
> > > - Added typec properties are based on general usb connector bindings[1]
> > >   proposed by Andrzej Hajda.
> > > - Use the standard unit suffixes as defined in property-units.txt.
> > >
> > > [1]
> > >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fp
> > >
> atchwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%
> 40nx
> > >
> p.com%7Ccea32589f3314488e18f08d5808e5aae%7C686ea1d3bc2b4c6fa92
> cd99c5
> > >
> c301635%7C0%7C1%7C636556271266469012=ANr1jeW8x8cy6CG6tz
> ACgj%2B
> > > FgNKl9DJbRpervFwaQKM%3D=0
> > >
> > >  .../bindings/connector/usb-connector.txt   | 43
> ++
> > >  1 file changed, 43 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > index e1463f1..242f6df 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > @@ -15,6 +15,30 @@ Optional properties:
> > >  - type: size of the connector, should be specified in case of USB-A,
> USB-B
> > >non-fullsize connectors: "mini", "micro".
> > >
> > > +Required properties for usb-c-connector with power delivery support:
> > > +- port-type: should be one of "source", "sink" or "dual".
> > > +- default-role: preferred power role if port-type is "dual"(drp),
> > > +should be
> > > +  "sink" or "source".
> >
> > Since port carries data and power, it would be better to explicitly
> > mention it in names of properties which can be ambiguous.
> > Maybe instead of 'port-type' you can use 'power-role', for example.
> > Other thing is that default-role is required only in case of DRP, so
> > maybe better would be to squash it in power-role, for example:
> >     power-role: should be on of "source", "sink", "dual-source",
> > "dual-sink", in case of dual roles suffix determines preferred role.
> >
> >
> > > +- src-pdos: An array of u32 with each entry providing supported
> > > +power
> > > +  source data object(PDO), the detailed bit definitions of PDO can
> > > +be found
> > > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > > +6.4.1.2
> > > +  Source_Capabilities Message, the order of each entry(PDO) should
> > > +follow
> > > +  the PD spec chapter 6.4.1. Required for power source and power dual
> role.
> > > +- snk-pdos: An array of u32 with each entry providing supported
> > > +power
> > > +  sink data object(PDO), the detailed bit definitions of PDO can be
> > > +found in
> > > +  "Universal Serial Bus Power Delivery Specification" chapter
> > > +6.4.1.3 Sink
> > > +  Capabilities Message, the order of each entry(PDO) should follow
> > > +the PD
> > > +  spec chapter 6.4.1. Required for power sink and power dual role.
> >
> > We should avoid magic numbers, magic numbers are bad :)
> 
> I don't mind if there's a defined format for the data.
> 

Yes, there is defined format in spec for this data.

> > If we really need to use PDOs here, I think we can re-use PDO_* macros
> > [1] - DTC should be able to parse it, maybe some lifting will be necessary.
> >
> > [1]:
> >
> https://emea01.s

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

> -Original Message-
> From: Rob Herring [mailto:r...@kernel.org]
> Sent: 2018年3月3日 6:29
> To: Jun Li <jun...@nxp.com>
> Cc: gre...@linuxfoundation.org; heikki.kroge...@linux.intel.com;
> li...@roeck-us.net; a.ha...@samsung.com; mark.rutl...@arm.com;
> yue...@google.com; Peter Chen <peter.c...@nxp.com>;
> garsi...@embeddedor.com; o_leve...@orange.fr;
> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> On Mon, Feb 26, 2018 at 07:49:17PM +0800, Li Jun wrote:
> > In case of usb-c-connector with power delivery support, add bingdings
> > supported by current typec driver, so user can pass all those
> > properties via dt.
> >
> > Signed-off-by: Li Jun <jun...@nxp.com>
> > ---
> > Changes for v2:
> > - Added typec properties are based on general usb connector bindings[1]
> >   proposed by Andrzej Hajda.
> > - Use the standard unit suffixes as defined in property-units.txt.
> >
> > [1]
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >
> chwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%40
> nxp.co
> >
> m%7C1741037c725343583b2c08d5808d034c%7C686ea1d3bc2b4c6fa92cd9
> 9c5c30163
> >
> 5%7C0%7C0%7C636556265503496434=AlR4ybLfxkAedHJCYQtlD%2B
> OXLEOAcbs
> > olMZumioKKNk%3D=0
> >
> >  .../bindings/connector/usb-connector.txt   | 43
> ++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..242f6df 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,30 @@ Optional properties:
> >  - type: size of the connector, should be specified in case of USB-A, USB-B
> >non-fullsize connectors: "mini", "micro".
> >
> > +Required properties for usb-c-connector with power delivery support:
> > +- port-type: should be one of "source", "sink" or "dual".
> > +- default-role: preferred power role if port-type is "dual"(drp),
> > +should be
> > +  "sink" or "source".
> > +- src-pdos: An array of u32 with each entry providing supported power
> > +  source data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual
> role.
> > +- snk-pdos: An array of u32 with each entry providing supported power
> 
> Abbreviating sink to snk doesn't buy much. I'd also just do source instead of
> src.

Will use source/sink-pdos in next version.

> 
> > +  sink data object(PDO), the detailed bit definitions of PDO can be
> > +found in
> > +  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> > +Sink
> > +  Capabilities Message, the order of each entry(PDO) should follow
> > +the PD
> > +  spec chapter 6.4.1. Required for power sink and power dual role.
> > +- max-snk-microvolt: The max voltage the sink can support in micro
> > +volts,
> > +  required for power sink and power dual role.
> > +- max-snk-microamp: The max current the sink can support in micro
> > +amps,
> > +  required for power sink and power dual role.
> > +- max-snk-microwatt-hours: The max power the sink can support in
> > +micro
> > +  Watt-hours, required for power sink and power dual role.
> > +- op-snk-microwatt-hours: Sink required operating power in micro
> > +Watt-hours,
> > +  if source offered power is less then it, Capability Mismatch is
> > +set,
> > +  required for power sink and power dual role.
> 
> None of these properties are part of the PDO?

Sorry, all above 4 props are not required any more with latest code,
I will remove them.

Jun Li
N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��&Ⅷ�G���h�(�茛j"���m赇z罐��帼f＂�h���~�m�

RE: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Jun Li]

> -Original Message-
> From: Andrzej Hajda [mailto:a.ha...@samsung.com]
> Sent: 2018年2月27日 16:41
> To: Jun Li <jun...@nxp.com>; gre...@linuxfoundation.org;
> robh...@kernel.org; heikki.kroge...@linux.intel.com; li...@roeck-us.net
> Cc: mark.rutl...@arm.com; yue...@google.com; Peter Chen
> <peter.c...@nxp.com>; garsi...@embeddedor.com; o_leve...@orange.fr;
> shufan_...@richtek.com; linux-usb@vger.kernel.org;
> devicet...@vger.kernel.org; dl-linux-imx <linux-...@nxp.com>
> Subject: Re: [PATCH v2 10/12] dt-bindings: connector: add properties for
> typec power delivery
> 
> On 26.02.2018 12:49, Li Jun wrote:
> > In case of usb-c-connector with power delivery support, add bingdings
> > supported by current typec driver, so user can pass all those
> > properties via dt.
> >
> > Signed-off-by: Li Jun <jun...@nxp.com>
> > ---
> > Changes for v2:
> > - Added typec properties are based on general usb connector bindings[1]
> >   proposed by Andrzej Hajda.
> > - Use the standard unit suffixes as defined in property-units.txt.
> >
> > [1]
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >
> chwork.kernel.org%2Fpatch%2F10231447%2F=02%7C01%7Cjun.li%40
> nxp.co
> >
> m%7Cccf14a36ca6445ee5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99
> c5c30163
> >
> 5%7C0%7C0%7C636553176880292197=2Pi0AtiwAqHQE3rGl%2Bo49K
> 7yZZcp%2B
> > 5bvJAknRBMGTrk%3D=0
> >
> >  .../bindings/connector/usb-connector.txt   | 43
> ++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..242f6df 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,30 @@ Optional properties:
> >  - type: size of the connector, should be specified in case of USB-A, USB-B
> >non-fullsize connectors: "mini", "micro".
> >
> > +Required properties for usb-c-connector with power delivery support:
> > +- port-type: should be one of "source", "sink" or "dual".
> > +- default-role: preferred power role if port-type is "dual"(drp),
> > +should be
> > +  "sink" or "source".
> 
> Since port carries data and power, it would be better to explicitly mention it
> in names of properties which can be ambiguous.
> Maybe instead of 'port-type' you can use 'power-role', for example.

Port-type is align with the name of typec coding and ABI, 'power-role'
is more like for the current role rather than the port's ability.

> Other thing is that default-role is required only in case of DRP, so maybe
> better would be to squash it in power-role, for example:
>     power-role: should be on of "source", "sink", "dual-source", "dual-sink",
> in case of dual roles suffix determines preferred role.

I don't know how much this squash can benefit, "dual-source/sink" is not
directly showing its meaning by name.

> 
> 
> > +- src-pdos: An array of u32 with each entry providing supported power
> > +  source data object(PDO), the detailed bit definitions of PDO can be
> > +found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter
> > +6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should
> > +follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual
> role.
> > +- snk-pdos: An array of u32 with each entry providing supported power
> > +  sink data object(PDO), the detailed bit definitions of PDO can be
> > +found in
> > +  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> > +Sink
> > +  Capabilities Message, the order of each entry(PDO) should follow
> > +the PD
> > +  spec chapter 6.4.1. Required for power sink and power dual role.
> 
> We should avoid magic numbers, magic numbers are bad :) If we really need
> to use PDOs here, I think we can re-use PDO_* macros [1] - DTC should be
> able to parse it, maybe some lifting will be necessary.
> 
> [1]:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felix
> ir.bootlin.com%2Flinux%2Fv4.16-rc3%2Fsource%2Finclude%2Flinux%2Fusb%
> 2Fpd.h%23L161=02%7C01%7Cjun.li%40nxp.com%7Cccf14a36ca6445e
> e5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636553176880292197=72HA33wgRyd2PMoPnZOlMatI2CuadplkYN
> DDGKF

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Rob Herring]

On Tue, Feb 27, 2018 at 09:41:19AM +0100, Andrzej Hajda wrote:
> On 26.02.2018 12:49, Li Jun wrote:
> > In case of usb-c-connector with power delivery support, add bingdings
> > supported by current typec driver, so user can pass all those properties
> > via dt.
> >
> > Signed-off-by: Li Jun 
> > ---
> > Changes for v2:
> > - Added typec properties are based on general usb connector bindings[1]
> >   proposed by Andrzej Hajda.
> > - Use the standard unit suffixes as defined in property-units.txt.
> >
> > [1] https://patchwork.kernel.org/patch/10231447/
> >
> >  .../bindings/connector/usb-connector.txt   | 43 
> > ++
> >  1 file changed, 43 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
> > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > index e1463f1..242f6df 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > @@ -15,6 +15,30 @@ Optional properties:
> >  - type: size of the connector, should be specified in case of USB-A, USB-B
> >non-fullsize connectors: "mini", "micro".
> >  
> > +Required properties for usb-c-connector with power delivery support:
> > +- port-type: should be one of "source", "sink" or "dual".
> > +- default-role: preferred power role if port-type is "dual"(drp), should be
> > +  "sink" or "source".
> 
> Since port carries data and power, it would be better to explicitly
> mention it in names of properties which can be ambiguous.
> Maybe instead of 'port-type' you can use 'power-role', for example.
> Other thing is that default-role is required only in case of DRP, so
> maybe better would be to squash it in power-role, for example:
>     power-role: should be on of "source", "sink", "dual-source",
> "dual-sink", in case of dual roles suffix determines preferred role.
> 
> 
> > +- src-pdos: An array of u32 with each entry providing supported power
> > +  source data object(PDO), the detailed bit definitions of PDO can be found
> > +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> > +  Source_Capabilities Message, the order of each entry(PDO) should follow
> > +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> > +- snk-pdos: An array of u32 with each entry providing supported power
> > +  sink data object(PDO), the detailed bit definitions of PDO can be found 
> > in
> > +  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3 Sink
> > +  Capabilities Message, the order of each entry(PDO) should follow the PD
> > +  spec chapter 6.4.1. Required for power sink and power dual role.
> 
> We should avoid magic numbers, magic numbers are bad :)

I don't mind if there's a defined format for the data.

> If we really need to use PDOs here, I think we can re-use PDO_* macros
> [1] - DTC should be able to parse it, maybe some lifting will be necessary.
> 
> [1]:
> https://elixir.bootlin.com/linux/v4.16-rc3/source/include/linux/usb/pd.h#L161
> > +- max-snk-microvolt: The max voltage the sink can support in micro volts,
> > +  required for power sink and power dual role.
> > +- max-snk-microamp: The max current the sink can support in micro amps,
> > +  required for power sink and power dual role.
> > +- max-snk-microwatt-hours: The max power the sink can support in micro
> > +  Watt-hours, required for power sink and power dual role.
> > +- op-snk-microwatt-hours: Sink required operating power in micro 
> > Watt-hours,
> > +  if source offered power is less then it, Capability Mismatch is set,
> > +  required for power sink and power dual role.
> 
> What is the relation between above properties and PDOs specified earlier?
> Are you sure all these props are required?
> 
> And general remark regarding all above properties. For me, most of them
> are specific not to the port but to devices responsible for power
> management: chargers, pmics,
> In many cases these properties are redundant with devices capabilities.
> On the other side I guess in many cases it may be convenient to put them
> here, so I am not sure what is better :)

I debated that too, but decided if you had 2 connectors connected to a 
single power controller, then you'd want these in the connector.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Rob Herring]

On Mon, Feb 26, 2018 at 07:49:17PM +0800, Li Jun wrote:
> In case of usb-c-connector with power delivery support, add bingdings
> supported by current typec driver, so user can pass all those properties
> via dt.
> 
> Signed-off-by: Li Jun 
> ---
> Changes for v2:
> - Added typec properties are based on general usb connector bindings[1]
>   proposed by Andrzej Hajda.
> - Use the standard unit suffixes as defined in property-units.txt.
> 
> [1] https://patchwork.kernel.org/patch/10231447/
> 
>  .../bindings/connector/usb-connector.txt   | 43 
> ++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..242f6df 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,30 @@ Optional properties:
>  - type: size of the connector, should be specified in case of USB-A, USB-B
>non-fullsize connectors: "mini", "micro".
>  
> +Required properties for usb-c-connector with power delivery support:
> +- port-type: should be one of "source", "sink" or "dual".
> +- default-role: preferred power role if port-type is "dual"(drp), should be
> +  "sink" or "source".
> +- src-pdos: An array of u32 with each entry providing supported power
> +  source data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> +  Source_Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- snk-pdos: An array of u32 with each entry providing supported power

Abbreviating sink to snk doesn't buy much. I'd also just do source 
instead of src.

> +  sink data object(PDO), the detailed bit definitions of PDO can be found in
> +  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3 Sink
> +  Capabilities Message, the order of each entry(PDO) should follow the PD
> +  spec chapter 6.4.1. Required for power sink and power dual role.
> +- max-snk-microvolt: The max voltage the sink can support in micro volts,
> +  required for power sink and power dual role.
> +- max-snk-microamp: The max current the sink can support in micro amps,
> +  required for power sink and power dual role.
> +- max-snk-microwatt-hours: The max power the sink can support in micro
> +  Watt-hours, required for power sink and power dual role.
> +- op-snk-microwatt-hours: Sink required operating power in micro Watt-hours,
> +  if source offered power is less then it, Capability Mismatch is set,
> +  required for power sink and power dual role.

None of these properties are part of the PDO?

> +
>  Required nodes:
>  - any data bus to the connector should be modeled using the OF graph bindings
>specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +97,22 @@ ccic: s2mm005@33 {
>   };
>   };
>  };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> + ...
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + port-type = "dual";
> + default-role = "sink";
> + src-pdos = <0x380190c8>;
> + snk-pdos = <0x380190c8 0x3802d0c8>;
> + max-snk-microvolt = <9000>;
> + max-snk-microamp = <2000>;
> + max-snk-microwatt-hours = <18000>;
> + op-snk-microwatt-hours = <9000>;
> + };
> +};
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Andrzej Hajda]

On 26.02.2018 12:49, Li Jun wrote:
> In case of usb-c-connector with power delivery support, add bingdings
> supported by current typec driver, so user can pass all those properties
> via dt.
>
> Signed-off-by: Li Jun 
> ---
> Changes for v2:
> - Added typec properties are based on general usb connector bindings[1]
>   proposed by Andrzej Hajda.
> - Use the standard unit suffixes as defined in property-units.txt.
>
> [1] https://patchwork.kernel.org/patch/10231447/
>
>  .../bindings/connector/usb-connector.txt   | 43 
> ++
>  1 file changed, 43 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> index e1463f1..242f6df 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -15,6 +15,30 @@ Optional properties:
>  - type: size of the connector, should be specified in case of USB-A, USB-B
>non-fullsize connectors: "mini", "micro".
>  
> +Required properties for usb-c-connector with power delivery support:
> +- port-type: should be one of "source", "sink" or "dual".
> +- default-role: preferred power role if port-type is "dual"(drp), should be
> +  "sink" or "source".

Since port carries data and power, it would be better to explicitly
mention it in names of properties which can be ambiguous.
Maybe instead of 'port-type' you can use 'power-role', for example.
Other thing is that default-role is required only in case of DRP, so
maybe better would be to squash it in power-role, for example:
    power-role: should be on of "source", "sink", "dual-source",
"dual-sink", in case of dual roles suffix determines preferred role.


> +- src-pdos: An array of u32 with each entry providing supported power
> +  source data object(PDO), the detailed bit definitions of PDO can be found
> +  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> +  Source_Capabilities Message, the order of each entry(PDO) should follow
> +  the PD spec chapter 6.4.1. Required for power source and power dual role.
> +- snk-pdos: An array of u32 with each entry providing supported power
> +  sink data object(PDO), the detailed bit definitions of PDO can be found in
> +  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3 Sink
> +  Capabilities Message, the order of each entry(PDO) should follow the PD
> +  spec chapter 6.4.1. Required for power sink and power dual role.

We should avoid magic numbers, magic numbers are bad :)
If we really need to use PDOs here, I think we can re-use PDO_* macros
[1] - DTC should be able to parse it, maybe some lifting will be necessary.

[1]:
https://elixir.bootlin.com/linux/v4.16-rc3/source/include/linux/usb/pd.h#L161
> +- max-snk-microvolt: The max voltage the sink can support in micro volts,
> +  required for power sink and power dual role.
> +- max-snk-microamp: The max current the sink can support in micro amps,
> +  required for power sink and power dual role.
> +- max-snk-microwatt-hours: The max power the sink can support in micro
> +  Watt-hours, required for power sink and power dual role.
> +- op-snk-microwatt-hours: Sink required operating power in micro Watt-hours,
> +  if source offered power is less then it, Capability Mismatch is set,
> +  required for power sink and power dual role.

What is the relation between above properties and PDOs specified earlier?
Are you sure all these props are required?

And general remark regarding all above properties. For me, most of them
are specific not to the port but to devices responsible for power
management: chargers, pmics,
In many cases these properties are redundant with devices capabilities.
On the other side I guess in many cases it may be convenient to put them
here, so I am not sure what is better :)

Regards
Andrzej

> +
>  Required nodes:
>  - any data bus to the connector should be modeled using the OF graph bindings
>specified in bindings/graph.txt, unless the bus is between parent node and
> @@ -73,3 +97,22 @@ ccic: s2mm005@33 {
>   };
>   };
>  };
> +
> +3. USB-C connector attached to a typec port controller(ptn5110), which has
> +power delivery support and enables drp.
> +
> +typec: ptn5110@50 {
> + ...
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + port-type = "dual";
> + default-role = "sink";
> + src-pdos = <0x380190c8>;
> + snk-pdos = <0x380190c8 0x3802d0c8>;
> + max-snk-microvolt = <9000>;
> + max-snk-microamp = <2000>;
> + max-snk-microwatt-hours = <18000>;
> + op-snk-microwatt-hours = <9000>;
> + };
> +};


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH v2 10/12] dt-bindings: connector: add properties for typec power delivery

[Li Jun]

In case of usb-c-connector with power delivery support, add bingdings
supported by current typec driver, so user can pass all those properties
via dt.

Signed-off-by: Li Jun 
---
Changes for v2:
- Added typec properties are based on general usb connector bindings[1]
  proposed by Andrzej Hajda.
- Use the standard unit suffixes as defined in property-units.txt.

[1] https://patchwork.kernel.org/patch/10231447/

 .../bindings/connector/usb-connector.txt   | 43 ++
 1 file changed, 43 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
b/Documentation/devicetree/bindings/connector/usb-connector.txt
index e1463f1..242f6df 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -15,6 +15,30 @@ Optional properties:
 - type: size of the connector, should be specified in case of USB-A, USB-B
   non-fullsize connectors: "mini", "micro".
 
+Required properties for usb-c-connector with power delivery support:
+- port-type: should be one of "source", "sink" or "dual".
+- default-role: preferred power role if port-type is "dual"(drp), should be
+  "sink" or "source".
+- src-pdos: An array of u32 with each entry providing supported power
+  source data object(PDO), the detailed bit definitions of PDO can be found
+  in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
+  Source_Capabilities Message, the order of each entry(PDO) should follow
+  the PD spec chapter 6.4.1. Required for power source and power dual role.
+- snk-pdos: An array of u32 with each entry providing supported power
+  sink data object(PDO), the detailed bit definitions of PDO can be found in
+  "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3 Sink
+  Capabilities Message, the order of each entry(PDO) should follow the PD
+  spec chapter 6.4.1. Required for power sink and power dual role.
+- max-snk-microvolt: The max voltage the sink can support in micro volts,
+  required for power sink and power dual role.
+- max-snk-microamp: The max current the sink can support in micro amps,
+  required for power sink and power dual role.
+- max-snk-microwatt-hours: The max power the sink can support in micro
+  Watt-hours, required for power sink and power dual role.
+- op-snk-microwatt-hours: Sink required operating power in micro Watt-hours,
+  if source offered power is less then it, Capability Mismatch is set,
+  required for power sink and power dual role.
+
 Required nodes:
 - any data bus to the connector should be modeled using the OF graph bindings
   specified in bindings/graph.txt, unless the bus is between parent node and
@@ -73,3 +97,22 @@ ccic: s2mm005@33 {
};
};
 };
+
+3. USB-C connector attached to a typec port controller(ptn5110), which has
+power delivery support and enables drp.
+
+typec: ptn5110@50 {
+   ...
+   usb_con: connector {
+   compatible = "usb-c-connector";
+   label = "USB-C";
+   port-type = "dual";
+   default-role = "sink";
+   src-pdos = <0x380190c8>;
+   snk-pdos = <0x380190c8 0x3802d0c8>;
+   max-snk-microvolt = <9000>;
+   max-snk-microamp = <2000>;
+   max-snk-microwatt-hours = <18000>;
+   op-snk-microwatt-hours = <9000>;
+   };
+};
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html