| Message ID | 20191007114142.5182-2-rogerq@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | usb: cdns3: Add TI wrapper | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Mon, Oct 07, 2019 at 02:41:41PM +0300, Roger Quadros wrote: > TI platforms have a wrapper module around the Cadence USB3 > controller. Add binding information for that. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > .../devicetree/bindings/usb/cdns-usb3-ti.txt | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt Please convert to DT schema. > diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt > new file mode 100644 > index 000000000000..12c7c903e6da > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt > @@ -0,0 +1,59 @@ > +Binding for the TI specific wrapper for the Cadence USBSS-DRD controller > + > +Required properties: > + - compatible: Should contain "ti,j721e-usb" > + - reg: Physical base address and size of the wrappers register area. > + - power-domains: Should contain a phandle to a PM domain provider node > + and an args specifier containing the USB device id > + value. This property is as per the binding documentation: > + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt > + - clocks: Clock phandles to usb2_refclk and lpm_clk > + - clock-names: Should contain "usb2_refclk" and "lpm_clk" _clk is redundant. 'ref' amd 'lpm' is sufficient. > + > +Optional properties: > + - ti,usb2-only: If present, it restricts the controller to USB2.0 mode of > + operation. Must be present if USB3 PHY is not available > + for USB. Seems like this should be discoverable based on describing the phy connections. > + - ti,modestrap-host: Set controller modestrap to HOST mode. > + - ti,modestrap-peripheral: Set controller modestrap to PERIPHERAL mode. What does modestrap mean? Fixed to the mode or that's the default? For default, John Stultz sent a similar binding. Seems we need something common. > + - ti,vbus-divider: Should be present if USB VBUS line is connected to the > + VBUS pin of the SoC via a 1/3 voltage divider. > + > +Sub-nodes: > +The USB2 PHY and the Cadence USB3 controller should be the sub-nodes. > + > +Example: > + > + ti_usb0: cdns_usb@4104000 { > + compatible = "ti,j721e-usb"; > + reg = <0x00 0x4104000 0x00 0x100>; > + power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 288 15>, <&k3_clks 288 3>; > + clock-names = "usb2_refclk", "lpm_clk"; > + assigned-clocks = <&k3_clks 288 15>; /* USB2_REFCLK */ > + assigned-clock-parents = <&k3_clks 288 16>; /* HFOSC0 */ > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + phy@4108000 { > + compatible = "ti,j721e-usb2-phy"; > + reg = <0x00 0x4108000 0x00 0x400>; > + }; Why is this a child node? Use the phy binding. > + > + usb0: usb@6000000 { > + compatible = "cdns,usb3-1.0.1"; Not documented. > + reg = <0x00 0x6000000 0x00 0x10000>, > + <0x00 0x6010000 0x00 0x10000>, > + <0x00 0x6020000 0x00 0x10000>; > + reg-names = "otg", "xhci", "dev"; > + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */ > + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, /* irq.6 */ > + <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; /* otgirq.0 */ > + interrupt-names = "host", > + "peripheral", > + "otg"; > + maximum-speed = "super-speed"; > + dr_mode = "otg"; > + }; > + }; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >
Hi, On 16/10/2019 00:29, Rob Herring wrote: > On Mon, Oct 07, 2019 at 02:41:41PM +0300, Roger Quadros wrote: >> TI platforms have a wrapper module around the Cadence USB3 >> controller. Add binding information for that. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> .../devicetree/bindings/usb/cdns-usb3-ti.txt | 59 +++++++++++++++++++ >> 1 file changed, 59 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt > > Please convert to DT schema. > >> diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt >> new file mode 100644 >> index 000000000000..12c7c903e6da >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt >> @@ -0,0 +1,59 @@ >> +Binding for the TI specific wrapper for the Cadence USBSS-DRD controller >> + >> +Required properties: >> + - compatible: Should contain "ti,j721e-usb" >> + - reg: Physical base address and size of the wrappers register area. >> + - power-domains: Should contain a phandle to a PM domain provider node >> + and an args specifier containing the USB device id >> + value. This property is as per the binding documentation: >> + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt >> + - clocks: Clock phandles to usb2_refclk and lpm_clk >> + - clock-names: Should contain "usb2_refclk" and "lpm_clk" > > _clk is redundant. 'ref' amd 'lpm' is sufficient. > >> + >> +Optional properties: >> + - ti,usb2-only: If present, it restricts the controller to USB2.0 mode of >> + operation. Must be present if USB3 PHY is not available >> + for USB. > > Seems like this should be discoverable based on describing the phy > connections. I don't think so. the PHY connections are in the child node. > >> + - ti,modestrap-host: Set controller modestrap to HOST mode. >> + - ti,modestrap-peripheral: Set controller modestrap to PERIPHERAL mode. > > What does modestrap mean? Fixed to the mode or that's the default? For > default, John Stultz sent a similar binding. Seems we need something > common. It means if the controller needs to be hard-wired (using a register bit) to be either host or peripheral. I think that this will not really be used so I'll get rid of it. It was mostly used for initial debug. The controller is dual-role and the role can be set to either host or peripheral using the operational registers. > >> + - ti,vbus-divider: Should be present if USB VBUS line is connected to the >> + VBUS pin of the SoC via a 1/3 voltage divider. >> + >> +Sub-nodes: >> +The USB2 PHY and the Cadence USB3 controller should be the sub-nodes. >> + >> +Example: >> + >> + ti_usb0: cdns_usb@4104000 { >> + compatible = "ti,j721e-usb"; >> + reg = <0x00 0x4104000 0x00 0x100>; >> + power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>; >> + clocks = <&k3_clks 288 15>, <&k3_clks 288 3>; >> + clock-names = "usb2_refclk", "lpm_clk"; >> + assigned-clocks = <&k3_clks 288 15>; /* USB2_REFCLK */ >> + assigned-clock-parents = <&k3_clks 288 16>; /* HFOSC0 */ >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + phy@4108000 { >> + compatible = "ti,j721e-usb2-phy"; >> + reg = <0x00 0x4108000 0x00 0x400>; >> + }; > > Why is this a child node? Use the phy binding. > This is a USB2 PHY that physically sits within the ti_usb0 wrapper module that controls power and clock to it. There isn't a device driver required for it so should I just get rid of it? Or can it just live there? what can the compatible be? Or we can add it later if at all a driver is needed. >> + >> + usb0: usb@6000000 { >> + compatible = "cdns,usb3-1.0.1"; > > Not documented. should be compatible = "cdns,usb3"; > >> + reg = <0x00 0x6000000 0x00 0x10000>, >> + <0x00 0x6010000 0x00 0x10000>, >> + <0x00 0x6020000 0x00 0x10000>; >> + reg-names = "otg", "xhci", "dev"; >> + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */ >> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, /* irq.6 */ >> + <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; /* otgirq.0 */ >> + interrupt-names = "host", >> + "peripheral", >> + "otg"; >> + maximum-speed = "super-speed"; >> + dr_mode = "otg"; >> + }; >> + };
diff --git a/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt new file mode 100644 index 000000000000..12c7c903e6da --- /dev/null +++ b/Documentation/devicetree/bindings/usb/cdns-usb3-ti.txt @@ -0,0 +1,59 @@ +Binding for the TI specific wrapper for the Cadence USBSS-DRD controller + +Required properties: + - compatible: Should contain "ti,j721e-usb" + - reg: Physical base address and size of the wrappers register area. + - power-domains: Should contain a phandle to a PM domain provider node + and an args specifier containing the USB device id + value. This property is as per the binding documentation: + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt + - clocks: Clock phandles to usb2_refclk and lpm_clk + - clock-names: Should contain "usb2_refclk" and "lpm_clk" + +Optional properties: + - ti,usb2-only: If present, it restricts the controller to USB2.0 mode of + operation. Must be present if USB3 PHY is not available + for USB. + - ti,modestrap-host: Set controller modestrap to HOST mode. + - ti,modestrap-peripheral: Set controller modestrap to PERIPHERAL mode. + - ti,vbus-divider: Should be present if USB VBUS line is connected to the + VBUS pin of the SoC via a 1/3 voltage divider. + +Sub-nodes: +The USB2 PHY and the Cadence USB3 controller should be the sub-nodes. + +Example: + + ti_usb0: cdns_usb@4104000 { + compatible = "ti,j721e-usb"; + reg = <0x00 0x4104000 0x00 0x100>; + power-domains = <&k3_pds 288 TI_SCI_PD_EXCLUSIVE>; + clocks = <&k3_clks 288 15>, <&k3_clks 288 3>; + clock-names = "usb2_refclk", "lpm_clk"; + assigned-clocks = <&k3_clks 288 15>; /* USB2_REFCLK */ + assigned-clock-parents = <&k3_clks 288 16>; /* HFOSC0 */ + #address-cells = <2>; + #size-cells = <2>; + ranges; + + phy@4108000 { + compatible = "ti,j721e-usb2-phy"; + reg = <0x00 0x4108000 0x00 0x400>; + }; + + usb0: usb@6000000 { + compatible = "cdns,usb3-1.0.1"; + reg = <0x00 0x6000000 0x00 0x10000>, + <0x00 0x6010000 0x00 0x10000>, + <0x00 0x6020000 0x00 0x10000>; + reg-names = "otg", "xhci", "dev"; + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, /* irq.0 */ + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, /* irq.6 */ + <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; /* otgirq.0 */ + interrupt-names = "host", + "peripheral", + "otg"; + maximum-speed = "super-speed"; + dr_mode = "otg"; + }; + };