Hi > -----Original Message----- > From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxxxxxxxx] > Sent: 2018年3月6日 20:02 > To: Jun Li <jun.li@xxxxxxx> > Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; > robh+dt@xxxxxxxxxx; 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 > > Hi guys, > > On Tue, Mar 06, 2018 at 09:38:59AM +0000, 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. > 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). > 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). > try-sink and try-source, both are optional, can't be true at the same time. > 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. > Agree, I think we can judge if the typec connector support PD by checking if there are PD required properties(like PDO). Thanks Jun Li > > Br, > > -- > heikki ?韬{.n?????%??檩??w?{.n????z谵{???塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
