From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch via GPIO References: <1552299557-6306-1-git-send-email-jun.li@nxp.com> <20190312104512.GA7752@kuha.fi.intel.com> From: Hans de Goede Message-ID: Date: Tue, 12 Mar 2019 12:46:36 +0100 MIME-Version: 1.0 In-Reply-To: <20190312104512.GA7752@kuha.fi.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit To: Heikki Krogerus , Jun Li Cc: "robh+dt@kernel.org" , "gregkh@linuxfoundation.org" , "andy.shevchenko@gmail.com" , "linux-usb@vger.kernel.org" , "devicetree@vger.kernel.org" , dl-linux-imx List-ID: Hi, On 12-03-19 11:45, Heikki Krogerus wrote: > Hi, > > On Tue, Mar 12, 2019 at 10:32:00AM +0000, Jun Li wrote: >> Hi Hans >>> -----Original Message----- >>> From: Hans de Goede >>> Sent: 2019年3月11日 19:03 >>> To: Jun Li ; robh+dt@kernel.org; heikki.krogerus@linux.intel.com >>> Cc: gregkh@linuxfoundation.org; andy.shevchenko@gmail.com; >>> linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx >>> >>> 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 >>>> --- >>>> .../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". >> */ > > Yes, but it's still OK to change it. It's not documented anywhere yet. > >>>> + >>>> +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? > > Hans, in OF graph both endpoints will have a remote-endpoint pointing > to each other.. > >> 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>; >> }; >> }; >> }; >> }; > > So like that. Ah I see, thank you for clarifying that. Regards, Hans