> -----Original Message----- > From: Jun Li > Sent: 2018年3月5日 15:52 > To: Rob Herring <robh@xxxxxxxxxx>; Andrzej Hajda <a.hajda@xxxxxxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; heikki.krogerus@xxxxxxxxxxxxxxx; > linux@xxxxxxxxxxxx; 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 > > > > -----Original Message----- > > From: Rob Herring [mailto:robh@xxxxxxxxxx] > > Sent: 2018年3月3日 6:39 > > To: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > > Cc: Jun Li <jun.li@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; > > heikki.krogerus@xxxxxxxxxxxxxxx; linux@xxxxxxxxxxxx; > > 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 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.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%2Fp > > > > > > > atchwork.kernel.org%2Fpatch%2F10231447%2F&data=02%7C01%7Cjun.li% > > 40nx > > > > > > > p.com%7Ccea32589f3314488e18f08d5808e5aae%7C686ea1d3bc2b4c6fa92 > > cd99c5 > > > > > > > c301635%7C0%7C1%7C636556271266469012&sdata=ANr1jeW8x8cy6CG6tz > > ACgj%2B > > > > FgNKl9DJbRpervFwaQKM%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.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 > > > > + 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.safelinks.protection.outlook.com/?url=https%3A%2F%2Feli > > > > > > xir.bootlin.com%2Flinux%2Fv4.16-rc3%2Fsource%2Finclude%2Flinux%2Fusb > > %2 > > > > > > Fpd.h%23L161&data=02%7C01%7Cjun.li%40nxp.com%7Ccea32589f3314488 > > e18f08d > > > > > > 5808e5aae%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636556 > > 271266469 > > > > > > 012&sdata=rebFCafzTpqI7FyYk9ZgW2nIhJ0ca5IB%2BBUa7WC05lM%3D&res > > erved=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? > > > > > > 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. > > > > The standard typec port controller(tcpci) can only manage one typec port > (connector), so in my case, those props should be applied to the typec > controller, if only for power, connector child node is not required. > I will move those typec props descriptions to a separate file: > (Documentation/devicetree/bindings/usb/tyepc.txt) After clarify with typec subsystem maintainer Heikki, we aligned and correctly understand your idea that typec props should be in usb connector node in all cases, I will keep this, sorry for the noise. Jun Li > > > Rob ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
