| Message ID | 1557922152-16449-2-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add USB3.0 and TI HD3SS3220 driver support | expand |
Hello! On 05/15/2019 03:09 PM, Biju Das wrote: > Add device tree binding document for TI HD3SS3220 Type-C DRP port > controller driver. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > V5-->V6 > * No change. > V4-->V5 > * No Change. > V3-->V4 > * No Change. > V2-->V3 > * Added Rob's Reviewed by tag. > V1-->V2 > * Added connector node. > * updated the example with connector node. > --- > .../devicetree/bindings/usb/ti,hd3ss3220.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > new file mode 100644 > index 0000000..7f41400 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > @@ -0,0 +1,37 @@ > +TI HD3SS3220 TypeC DRP Port Controller. > + > +Required properties: > + - compatible: Must be "ti,hd3ss3220". > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. > + - interrupts: <a b> where a is the interrupt number and b represents an > + encoding of the sense and level information for the interrupt. This depends on an interrupt controller used. I'd just said "an interrupt specifier", w/o further details. > + > +Required sub-node: > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The > + bindings of the connector node are specified in: > + > + Documentation/devicetree/bindings/connector/usb-connector.txt > + [...] MBR, Sergei
Hi Biju-san, Thank you for the patch! > From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM > > Add device tree binding document for TI HD3SS3220 Type-C DRP port > controller driver. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > V5-->V6 > * No change. > V4-->V5 > * No Change. > V3-->V4 > * No Change. > V2-->V3 > * Added Rob's Reviewed by tag. > V1-->V2 > * Added connector node. > * updated the example with connector node. > --- > .../devicetree/bindings/usb/ti,hd3ss3220.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > new file mode 100644 > index 0000000..7f41400 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > @@ -0,0 +1,37 @@ > +TI HD3SS3220 TypeC DRP Port Controller. > + > +Required properties: > + - compatible: Must be "ti,hd3ss3220". > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. > + - interrupts: <a b> where a is the interrupt number and b represents an > + encoding of the sense and level information for the interrupt. > + > +Required sub-node: > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The s/connector :/connector:/ > + bindings of the connector node are specified in: > + > + Documentation/devicetree/bindings/connector/usb-connector.txt > + > +Example: > +hd3ss3220@47 { > + compatible = "ti,hd3ss3220"; > + reg = <0x47>; > + interrupt-parent = <&gpio6>; > + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; > + > + usb_con: connector { > + compatible = "usb-c-connector"; > + label = "USB-C"; > + data-role = "dual"; > + }; > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + hd3ss3220_ep: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&usb3peri_role_switch>; > + }; > + }; According to the connector/usb-connector.txt, should the connector node have ports, port@1 and an endpoint nodes like below? usb_con: connector { compatible = "usb-c-connector"; label = "USB-C"; data-role = "dual"; ports { #address-cells = <1>; #size-cells = <0>; port@1 { reg = <1>; hd3ss3220_ep: endpoint { remote-endpoint = <&usb3peri_role_switch>; }; }; }; }; Best regards, Yoshihiro Shimoda
Hi > > +Required properties: > > + - compatible: Must be "ti,hd3ss3220". > > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. > > + - interrupts: <a b> where a is the interrupt number and b represents an > > + encoding of the sense and level information for the interrupt. > > + > > +Required sub-node: > > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The > > s/connector :/connector:/ Maybe it is for alignment ? > According to the connector/usb-connector.txt, should the connector node > have ports, port@1 and an endpoint nodes like below? "ports" is needed if it has multiple "port", otherwise, single port is allowed from OF-graph point of view. Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Sergei, Thanks for the feedback. > Subject: Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding > document > > Hello! > > On 05/15/2019 03:09 PM, Biju Das wrote: > > > Add device tree binding document for TI HD3SS3220 Type-C DRP port > > controller driver. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > V5-->V6 > > * No change. > > V4-->V5 > > * No Change. > > V3-->V4 > > * No Change. > > V2-->V3 > > * Added Rob's Reviewed by tag. > > V1-->V2 > > * Added connector node. > > * updated the example with connector node. > > --- > > .../devicetree/bindings/usb/ti,hd3ss3220.txt | 37 > ++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > > > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > new file mode 100644 > > index 0000000..7f41400 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt > > @@ -0,0 +1,37 @@ > > +TI HD3SS3220 TypeC DRP Port Controller. > > + > > +Required properties: > > + - compatible: Must be "ti,hd3ss3220". > > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. > > + - interrupts: <a b> where a is the interrupt number and b represents an > > + encoding of the sense and level information for the interrupt. > > This depends on an interrupt controller used. I'd just said "an interrupt > specifier", w/o further details. Fine , If it is ok for everyone. Regards, Biju > > + > > +Required sub-node: > > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The > > + bindings of the connector node are specified in: > > + > > + Documentation/devicetree/bindings/connector/usb-connector.txt > > + > [...] > > MBR, Sergei
Hi Morimoto-San and Shimoda-San, Thanks for the feedback. > Subject: Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding > document > > > Hi > > > > +Required properties: > > > + - compatible: Must be "ti,hd3ss3220". > > > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. > > > + - interrupts: <a b> where a is the interrupt number and b represents an > > > + encoding of the sense and level information for the interrupt. > > > + > > > +Required sub-node: > > > + - connector : The "usb-c-connector" attached to the hd3ss3220 > > > +chip. The > > > > s/connector :/connector:/ > > Maybe it is for alignment ? Yes, I need to fix the extra space. > > According to the connector/usb-connector.txt, should the connector > > node have ports, port@1 and an endpoint nodes like below? > > "ports" is needed if it has multiple "port", otherwise, single port is allowed > from OF-graph point of view. OK. I will use single port on the next patch series. Regards, Biju
Hi Biju-san, Morimoto-san, > From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM <snip> > > > According to the connector/usb-connector.txt, should the connector > > > node have ports, port@1 and an endpoint nodes like below? > > > > "ports" is needed if it has multiple "port", otherwise, single port is allowed > > from OF-graph point of view. > > OK. I will use single port on the next patch series. According to the connector/usb-connector.txt [1], even if this device uses a single port, we should describe ports node and port@1 (for SuperSpeed) subnode like usb/typec-tcpci.txt. [1] 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 the connector. Since single connector can have multiple data buses every bus has assigned OF graph port number as follows: 0: High Speed (HS), present in all connectors, 1: Super Speed (SS), present in SS capable connectors, 2: Sideband use (SBU), present in USB-C. Best regards, Yoshihiro Shimoda
Hi Shimoda-San, Thanks for the feedback. > Subject: RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding > document > > Hi Biju-san, Morimoto-san, > > > From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM > <snip> > > > > According to the connector/usb-connector.txt, should the connector > > > > node have ports, port@1 and an endpoint nodes like below? > > > > > > "ports" is needed if it has multiple "port", otherwise, single port > > > is allowed from OF-graph point of view. > > > > OK. I will use single port on the next patch series. > > According to the connector/usb-connector.txt [1], even if this device uses a > single port, we should describe ports node and port@1 (for SuperSpeed) > subnode like usb/typec-tcpci.txt. OK. I will update the example like below. hd3ss3220@47 { compatible = "ti,hd3ss3220"; reg = <0x47>; interrupt-parent = <&gpio6>; interrupts = <3 IRQ_TYPE_LEVEL_LOW>; usb_con: connector { compatible = "usb-c-connector"; label = "USB-C"; data-role = "dual"; ports { #address-cells = <1>; #size-cells = <0>; port@1 { reg = <1>; hd3ss3220_ep: endpoint { remote-endpoint = <&usb3_role_switch>; }; }; }; }; }; Regards, Biju > [1] > 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 > the connector. Since single connector can have multiple data buses every > bus > has assigned OF graph port number as follows: > 0: High Speed (HS), present in all connectors, > 1: Super Speed (SS), present in SS capable connectors, > 2: Sideband use (SBU), present in USB-C.
diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt new file mode 100644 index 0000000..7f41400 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt @@ -0,0 +1,37 @@ +TI HD3SS3220 TypeC DRP Port Controller. + +Required properties: + - compatible: Must be "ti,hd3ss3220". + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin. + - interrupts: <a b> where a is the interrupt number and b represents an + encoding of the sense and level information for the interrupt. + +Required sub-node: + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The + bindings of the connector node are specified in: + + Documentation/devicetree/bindings/connector/usb-connector.txt + +Example: +hd3ss3220@47 { + compatible = "ti,hd3ss3220"; + reg = <0x47>; + interrupt-parent = <&gpio6>; + interrupts = <3 IRQ_TYPE_LEVEL_LOW>; + + usb_con: connector { + compatible = "usb-c-connector"; + label = "USB-C"; + data-role = "dual"; + }; + + port { + #address-cells = <1>; + #size-cells = <0>; + + hd3ss3220_ep: endpoint@0 { + reg = <0>; + remote-endpoint = <&usb3peri_role_switch>; + }; + }; +};