RE: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch via GPIO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans
> -----Original Message-----
> From: Hans de Goede <hdegoede@xxxxxxxxxx>
> Sent: 2019年3月11日 19:03
> To: Jun Li <jun.li@xxxxxxx>; robh+dt@xxxxxxxxxx; heikki.krogerus@xxxxxxxxxxxxxxx
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; andy.shevchenko@xxxxxxxxx;
> linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; dl-linux-imx
> <linux-imx@xxxxxxx>
> Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch
> via GPIO
> 
> Hi,
> 
> On 11-03-19 11:40, Jun Li wrote:
> > Some typec super speed active channel switch can be controlled via a
> > GPIO, this binding can be used to specify the switch node by a GPIO
> > and the remote endpoint of its consumer.
> >
> > Signed-off-by: Li Jun <jun.li@xxxxxxx>
> > ---
> >   .../devicetree/bindings/usb/typec-switch-gpio.txt  | 30
> ++++++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
> > b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
> > new file mode 100644
> > index 0000000..4ef76cf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
> > @@ -0,0 +1,30 @@
> > +Typec orientation switch via a GPIO
> > +-----------------------------------
> > +
> > +Required properties:
> > +- compatible: should be set one of following:
> > +	- "nxp,ptn36043" for NXP Type-C SuperSpeed active switch.
> > +
> > +- gpios: the GPIO used to switch the super speed active channel,
> > +		GPIO_ACTIVE_HIGH: GPIO state high for cc1;
> > +		GPIO_ACTIVE_LOW:  GPIO state low for cc1.
> > +- orientation-switch: must be present.
> 
> Shouldn't this have usb-c in the propery name, e.g.:
> usb-c-orientation-switch  ?

This is decided by drivers/usb/typec/mux.c:36
/*
 * With OF graph the mux node must have a boolean device property named
 * "orientation-switch".
 */
> 
> > +
> > +Required sub-node:
> > +- port: specify the remote endpoint of typec switch consumer.
> > +
> > +Example:
> > +
> > +ptn36043 {
> > +	compatible = "nxp,ptn36043";
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pinctrl_ss_sel>;
> > +	gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>;
> > +	orientation-switch;
> > +
> > +	port {
> > +		usb3_data_ss: endpoint {
> > +			remote-endpoint = <&typec_con_ss>;
> 
> 
> Isn't this the wrong way around, shouldn't the "usb-c-connector"
> compatible port be pointing to the orientation switch, rather then the other way
> around?  

I am not sure I am getting your point, "usb-c-connector" is the user of typec switch,
yes, it is pointing to the orientation switch provider(i.e, this example node).

>Both will work in the end. but to me it feels more natural to group all the
> info about the type-c connector together in the "usb-c-connector" compatible port
>

        ptn36043 {
                compatible = "nxp,ptn36043";
                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_ss_sel>;
                gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>;
                orientation-switch;

                port {
                        usb3_data_ss: endpoint {
                                remote-endpoint = <&typec_con_ss>;
                        };
                };
        };

usb_con: connector {
	compatible = "usb-c-connector";
	...
	ports {
		#address-cells = <1>;
		#size-cells = <0>;

		port@1 {
			reg = <1>;
			typec_con_ss: endpoint {
				remote-endpoint = <&usb3_data_ss>;
			};
		};
	};
};

Regards
Jun
> Regards,
> 
> Hans





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux