| Message ID | 20230221120612.27366-1-r-gunasekaran@ti.com |
|---|---|
| Headers | show |
| Series | arm64: j721s2: Add support for additional IPs | expand |
On 17:36-20230221, Ravi Gunasekaran wrote: > From: Aswath Govindraju <a-govindraju@ti.com> > > Add support for single instance of USB 3.0 controller in J721S2 SoC. > > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> > Signed-off-by: Matt Ranostay <mranostay@ti.com> > Link: https://lore.kernel.org/r/20221122101616.770050-2-mranostay@ti.com Is the link supposed to signify some reference we need to keep for ever? > Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> > --- > I had reviewed this patch in the v5 series [1]. > Since I'm taking over upstreaming this series, I removed the self > Reviewed-by tag. > > Links: > > [1] - https://lore.kernel.org/all/134c28a0-2d49-549c-dc8d-0887d8fd29c3@ti.com/ What changed in this rev of the patch? > > arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 46 ++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > index 8915132efcc1..84e5689fff9f 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > @@ -26,6 +26,20 @@ > }; > }; > > + scm_conf: syscon@104000 { > + compatible = "ti,j721e-system-controller", "syscon", "simple-mfd"; > + reg = <0x00 0x00104000 0x00 0x18000>; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0x00 0x00 0x00104000 0x18000>; > + > + usb_serdes_mux: mux-controller@0 { > + compatible = "mmio-mux"; > + #mux-control-cells = <1>; > + mux-reg-masks = <0x0 0x8000000>; /* USB0 to SERDES0 lane 1/3 mux */ > + }; > + }; > + > gic500: interrupt-controller@1800000 { > compatible = "arm,gic-v3"; > #address-cells = <2>; > @@ -745,6 +759,38 @@ > }; > }; > > + usbss0: cdns-usb@4104000 { > + compatible = "ti,j721e-usb"; > + reg = <0x00 0x04104000 0x00 0x100>; > + clocks = <&k3_clks 360 16>, <&k3_clks 360 15>; > + clock-names = "ref", "lpm"; > + assigned-clocks = <&k3_clks 360 16>; /* USB2_REFCLK */ > + assigned-clock-parents = <&k3_clks 360 17>; > + power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + dma-coherent; > + > + status = "disabled"; Why disabled by default? > + > + usb0: usb@6000000 { > + compatible = "cdns,usb3"; > + reg = <0x00 0x06000000 0x00 0x10000>, > + <0x00 0x06010000 0x00 0x10000>, > + <0x00 0x06020000 0x00 0x10000>; > + reg-names = "otg", "xhci", "dev"; > + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "host", "peripheral", "otg"; > + maximum-speed = "super-speed"; > + dr_mode = "otg"; > + > + status = "disabled"; Why disabled by default? > + }; > + }; > + > main_mcan0: can@2701000 { > compatible = "bosch,m_can"; > reg = <0x00 0x02701000 0x00 0x200>, > -- > 2.17.1 >
On 17:36-20230221, Ravi Gunasekaran wrote: > From: Matt Ranostay <mranostay@ti.com> > > Add dt node for the single instance of WIZ (SERDES wrapper) and > SERDES module shared by PCIe, eDP and USB. [...] > > diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > index 84e5689fff9f..af6c93f0a055 100644 > --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi [...] > @@ -33,11 +44,18 @@ > #size-cells = <1>; > ranges = <0x00 0x00 0x00104000 0x18000>; > > - usb_serdes_mux: mux-controller@0 { > + usb_serdes_mux: mux-controller-0 { was'nt this just introduced in the patch just prior, if so, NAK, do the changes where they are introduced. > compatible = "mmio-mux"; > #mux-control-cells = <1>; > mux-reg-masks = <0x0 0x8000000>; /* USB0 to SERDES0 lane 1/3 mux */ > };
On 21/02/23 7:28 pm, Nishanth Menon wrote: > On 17:36-20230221, Ravi Gunasekaran wrote: >> From: Aswath Govindraju <a-govindraju@ti.com> >> >> Add support for single instance of USB 3.0 controller in J721S2 SoC. >> >> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com> >> Signed-off-by: Matt Ranostay <mranostay@ti.com> >> Link: https://lore.kernel.org/r/20221122101616.770050-2-mranostay@ti.com > > Is the link supposed to signify some reference we need to keep for ever? No. I will remove the reference to the link. I picked up the v7 series from previous submitter through "b4" and maintained the Link tag as-is since then. I will remove the reference to the link in the next series. > >> Signed-off-by: Ravi Gunasekaran <r-gunasekaran@ti.com> >> --- >> I had reviewed this patch in the v5 series [1]. >> Since I'm taking over upstreaming this series, I removed the self >> Reviewed-by tag. >> >> Links: >> >> [1] - https://lore.kernel.org/all/134c28a0-2d49-549c-dc8d-0887d8fd29c3@ti.com/ > > > What changed in this rev of the patch? Only one change in the patch since v5; Status of the some nodes introduced in this patch is set to disabled. > >> >> arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi | 46 ++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi >> index 8915132efcc1..84e5689fff9f 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi >> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi >> @@ -26,6 +26,20 @@ >> }; >> }; >> >> + scm_conf: syscon@104000 { >> + compatible = "ti,j721e-system-controller", "syscon", "simple-mfd"; >> + reg = <0x00 0x00104000 0x00 0x18000>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + ranges = <0x00 0x00 0x00104000 0x18000>; >> + >> + usb_serdes_mux: mux-controller@0 { >> + compatible = "mmio-mux"; >> + #mux-control-cells = <1>; >> + mux-reg-masks = <0x0 0x8000000>; /* USB0 to SERDES0 lane 1/3 mux */ >> + }; >> + }; >> + >> gic500: interrupt-controller@1800000 { >> compatible = "arm,gic-v3"; >> #address-cells = <2>; >> @@ -745,6 +759,38 @@ >> }; >> }; >> >> + usbss0: cdns-usb@4104000 { >> + compatible = "ti,j721e-usb"; >> + reg = <0x00 0x04104000 0x00 0x100>; >> + clocks = <&k3_clks 360 16>, <&k3_clks 360 15>; >> + clock-names = "ref", "lpm"; >> + assigned-clocks = <&k3_clks 360 16>; /* USB2_REFCLK */ >> + assigned-clock-parents = <&k3_clks 360 17>; >> + power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + dma-coherent; >> + >> + status = "disabled"; > > Why disabled by default? One of the comment received in the v9 series was to disable the node in the include file and then enable it in the board specific DTS file. Changes in this series addressed that comment. > >> + >> + usb0: usb@6000000 { >> + compatible = "cdns,usb3"; >> + reg = <0x00 0x06000000 0x00 0x10000>, >> + <0x00 0x06010000 0x00 0x10000>, >> + <0x00 0x06020000 0x00 0x10000>; >> + reg-names = "otg", "xhci", "dev"; >> + interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-names = "host", "peripheral", "otg"; >> + maximum-speed = "super-speed"; >> + dr_mode = "otg"; >> + >> + status = "disabled"; > > Why disabled by default? See above. > >> + }; >> + }; >> + >> main_mcan0: can@2701000 { >> compatible = "bosch,m_can"; >> reg = <0x00 0x02701000 0x00 0x200>, >> -- >> 2.17.1 >>
On 21/02/23 7:31 pm, Nishanth Menon wrote: > On 17:36-20230221, Ravi Gunasekaran wrote: >> From: Matt Ranostay <mranostay@ti.com> >> >> Add dt node for the single instance of WIZ (SERDES wrapper) and >> SERDES module shared by PCIe, eDP and USB. > > [...] > >> >> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi >> index 84e5689fff9f..af6c93f0a055 100644 >> --- a/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi >> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-main.dtsi > [...] >> @@ -33,11 +44,18 @@ >> #size-cells = <1>; >> ranges = <0x00 0x00 0x00104000 0x18000>; >> >> - usb_serdes_mux: mux-controller@0 { >> + usb_serdes_mux: mux-controller-0 { > > was'nt this just introduced in the patch just prior, if so, NAK, > do the changes where they are introduced. Noted. I will fix this. Thanks for reviewing. > >> compatible = "mmio-mux"; >> #mux-control-cells = <1>; >> mux-reg-masks = <0x0 0x8000000>; /* USB0 to SERDES0 lane 1/3 mux */ >> };
On 10:21-20230222, Ravi Gunasekaran wrote: [...] > >> + usbss0: cdns-usb@4104000 { > >> + compatible = "ti,j721e-usb"; > >> + reg = <0x00 0x04104000 0x00 0x100>; > >> + clocks = <&k3_clks 360 16>, <&k3_clks 360 15>; > >> + clock-names = "ref", "lpm"; > >> + assigned-clocks = <&k3_clks 360 16>; /* USB2_REFCLK */ > >> + assigned-clock-parents = <&k3_clks 360 17>; > >> + power-domains = <&k3_pds 360 TI_SCI_PD_EXCLUSIVE>; > >> + #address-cells = <2>; > >> + #size-cells = <2>; > >> + ranges; > >> + dma-coherent; > >> + > >> + status = "disabled"; > > > > Why disabled by default? > > One of the comment received in the v9 series was to disable the node in > the include file and then enable it in the board specific DTS file. > Changes in this series addressed that comment. Document in the node why it is disabled by default. Also do make sure All K3 SoCs dtsi nodes follow the same argument.