On Fri, 2021-11-26 at 11:36 +0100, AngeloGioacchino Del Regno wrote: > Il 02/11/21 07:00, Chunfeng Yun ha scritto: > > Add all four USB xHCI controllers for MT8195 > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx> > > --- > > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 79 > > ++++++++++++++++++++++++ > > 1 file changed, 79 insertions(+) > > > > Hello! > Thanks for the patch! However, there is something to improve... > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi > > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi > > index a59c0e9d1fc2..263eebfd2ea1 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi > > @@ -8,6 +8,7 @@ > > #include <dt-bindings/clock/mt8195-clk.h> > > #include <dt-bindings/interrupt-controller/arm-gic.h> > > #include <dt-bindings/interrupt-controller/irq.h> > > +#include <dt-bindings/phy/phy.h> > > #include <dt-bindings/power/mt8195-power.h> > > > > / { > > @@ -823,6 +824,26 @@ > > status = "disabled"; > > }; > > > > + xhci0: usb@11200000 { > > + compatible = "mediatek,mt8195-xhci", > > "mediatek,mtk-xhci"; > > + reg = <0 0x11200000 0 0x1000>, <0 0x11203e00 0 > > 0x0100>; > > + reg-names = "mac", "ippc"; > > + interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH > > 0>; > > Here, and on the other xhci nodes (from what I know, xhci{0,1,3}), > you should use > interrupts-extended and declare the wakeup interrupt on pio. > > interrupts-extended = <&gic GIC_SPI 129 > IRQ_TYPE_LEVEL_HIGH 0>, > > <&pio 219 > IRQ_TYPE_LEVEL_LOW>; > > interrupt-names = "host", "wakeup"; Ok, this patch doesn't support runtime suspend, will add it in next version, thanks a lot > > > > + phys = <&u2port0 PHY_TYPE_USB2>, <&u3port0 > > PHY_TYPE_USB3>; > > + assigned-clocks = <&topckgen CLK_TOP_USB_TOP>, > > + <&topckgen > > CLK_TOP_SSUSB_XHCI>; > > + assigned-clock-parents = <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>, > > + <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>; > > + clocks = <&infracfg_ao CLK_INFRA_AO_SSUSB>, > > + <&infracfg_ao > > CLK_INFRA_AO_SSUSB_XHCI>, > > + <&topckgen CLK_TOP_SSUSB_REF>, > > + <&apmixedsys CLK_APMIXED_USB1PLL>; > > + clock-names = "sys_ck", "xhci_ck", "ref_ck", > > "mcu_ck"; > > + mediatek,syscon-wakeup = <&pericfg 0x400 103>; > > + wakeup-source; > > + status = "disabled"; > > + }; > > + > > mmc0: mmc@11230000 { > > compatible = "mediatek,mt8195-mmc", > > "mediatek,mt8192-mmc"; > > reg = <0 0x11230000 0 0x10000>, > > @@ -843,6 +864,64 @@ > > status = "disabled"; > > }; > > > > + xhci1: usb@11290000 { > > + compatible = "mediatek,mt8195-xhci", > > "mediatek,mtk-xhci"; > > + reg = <0 0x11290000 0 0x1000>, <0 0x11293e00 0 > > 0x0100>; > > + reg-names = "mac", "ippc"; > > + interrupts = <GIC_SPI 530 IRQ_TYPE_LEVEL_HIGH > > 0>; > > interrupts-extended = <&gic GIC_SPI 530 > IRQ_TYPE_LEVEL_HIGH 0>, > > <&pio 218 > IRQ_TYPE_LEVEL_LOW>; > > > + phys = <&u2port1 PHY_TYPE_USB2>; > > + assigned-clocks = <&topckgen > > CLK_TOP_USB_TOP_1P>, > > + <&topckgen > > CLK_TOP_SSUSB_XHCI_1P>; > > + assigned-clock-parents = <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>, > > + <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>; > > + clocks = <&pericfg_ao > > CLK_PERI_AO_SSUSB_1P_BUS>, > > + <&pericfg_ao > > CLK_PERI_AO_SSUSB_1P_XHCI>, > > + <&topckgen CLK_TOP_SSUSB_P1_REF>, > > + <&apmixedsys CLK_APMIXED_USB1PLL>; > > + clock-names = "sys_ck", "xhci_ck", "ref_ck", > > "mcu_ck"; > > + mediatek,syscon-wakeup = <&pericfg 0x400 104>; > > + wakeup-source; > > + status = "disabled"; > > + }; > > + > > + xhci2: usb@112a0000 { > > + compatible = "mediatek,mt8195-xhci", > > "mediatek,mtk-xhci"; > > + reg = <0 0x112a0000 0 0x1000>, <0 0x112a3e00 0 > > 0x0100>; > > + reg-names = "mac", "ippc"; > > + interrupts = <GIC_SPI 533 IRQ_TYPE_LEVEL_HIGH > > 0>; > > + phys = <&u2port2 PHY_TYPE_USB2>; > > + assigned-clocks = <&topckgen > > CLK_TOP_USB_TOP_2P>, > > + <&topckgen > > CLK_TOP_SSUSB_XHCI_2P>; > > + assigned-clock-parents = <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>, > > + <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>; > > + clocks = <&pericfg_ao > > CLK_PERI_AO_SSUSB_2P_BUS>, > > + <&pericfg_ao > > CLK_PERI_AO_SSUSB_2P_XHCI>, > > + <&topckgen CLK_TOP_SSUSB_P2_REF>; > > + clock-names = "sys_ck", "xhci_ck", "ref_ck"; > > + mediatek,syscon-wakeup = <&pericfg 0x400 105>; > > + status = "disabled"; > > + }; > > + > > + xhci3: usb@112b0000 { > > + compatible = "mediatek,mt8195-xhci", > > "mediatek,mtk-xhci"; > > + reg = <0 0x112b0000 0 0x1000>, <0 0x112b3e00 0 > > 0x0100>; > > + reg-names = "mac", "ippc"; > > + interrupts = <GIC_SPI 536 IRQ_TYPE_LEVEL_HIGH > > 0>; > > interrupts-extended = <&gic GIC_SPI 536 > IRQ_TYPE_LEVEL_HIGH 0>, > > <&pio 221 > IRQ_TYPE_LEVEL_LOW>; > > interrupts-names = "host", "wakeup"; > > > + phys = <&u2port3 PHY_TYPE_USB2>; > > + assigned-clocks = <&topckgen > > CLK_TOP_USB_TOP_3P>, > > + <&topckgen > > CLK_TOP_SSUSB_XHCI_3P>; > > + assigned-clock-parents = <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>, > > + <&topckgen > > CLK_TOP_UNIVPLL_D5_D4>; > > + clocks = <&pericfg_ao > > CLK_PERI_AO_SSUSB_3P_BUS>, > > + <&pericfg_ao > > CLK_PERI_AO_SSUSB_3P_XHCI>, > > + <&topckgen CLK_TOP_SSUSB_P3_REF>; > > + clock-names = "sys_ck", "xhci_ck", "ref_ck"; > > + mediatek,syscon-wakeup = <&pericfg 0x400 106>; > > + wakeup-source; > > + usb2-lpm-disable; > > + status = "disabled"; > > + }; > > + > > nor_flash: nor@1132c000 { > > compatible = "mediatek,mt8195-nor", > > "mediatek,mt8173-nor"; > > reg = <0 0x1132c000 0 0x1000>; > > > > Regards, > - Angelo