> -----Original Message----- > From: Andrzej Hajda [mailto:a.hajda@xxxxxxxxxxx] > Sent: 2018年2月27日 16:41 > To: Jun Li <jun.li@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; > robh+dt@xxxxxxxxxx; heikki.krogerus@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx > Cc: mark.rutland@xxxxxxx; yueyao@xxxxxxxxxx; Peter Chen > <peter.chen@xxxxxxx>; garsilva@xxxxxxxxxxxxxx; o_leveque@xxxxxxxxx; > shufan_lee@xxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> > 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.li@xxxxxxx> > > --- > > 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&data=02%7C01%7Cjun.li%40 > nxp.co > > > m%7Cccf14a36ca6445ee5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99 > c5c30163 > > > 5%7C0%7C0%7C636553176880292197&sdata=2Pi0AtiwAqHQE3rGl%2Bo49K > 7yZZcp%2B > > 5bvJAknRBMGTrk%3D&reserved=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&data=02%7C01%7Cjun.li%40nxp.com%7Cccf14a36ca6445e > e5f1108d57dbde3c7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C636553176880292197&sdata=72HA33wgRyd2PMoPnZOlMatI2CuadplkYN > DDGKFSUB0%3D&reserved=0 > > +- 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? > Sorry, with latest tcpm code, those props are not required, will remove them. Jun Li > 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>; > > + }; > > +}; > ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥