Re: [PATCH v7 4/4] arm64: dts: Add mediatek SoC mt8195 and evaluation board
From: Chen-Yu Tsai <wenst@chromium.org>
Date: 2022-01-07 08:36:32
Also in:
linux-devicetree, linux-gpio, linux-mediatek, linux-spi, lkml
On Thu, Jan 6, 2022 at 7:15 PM Tinghan Shen [off-list ref] wrote:
On Thu, 2021-12-23 at 17:59 +0800, Chen-Yu Tsai wrote:quoted
On Mon, Dec 20, 2021 at 8:20 PM Tinghan Shen [off-list ref] wrote:
[...]
quoted
[...]quoted
+ 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>; + 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";The binding for this needs to be fixed. It expects clocks in the order specified in the binding, and this doesn't match.okquoted
Also, "dma_ck" is missing and will likely cause warnings to be generated.only sys_ck is required, others are optional as described in binding
I understand, but the bindings language is somewhat limited and right now is written in a way that if "dma_ck" is missing it would fail the DT bindings check.
quoted
This goes for all the xhci device nodes.quoted
+ status = "disabled"; + }; + + mmc0: mmc@11230000 { + compatible = "mediatek,mt8195-mmc", + "mediatek,mt8183-mmc"; + reg = <0 0x11230000 0 0x10000>, + <0 0x11f50000 0 0x1000>;The binding only allows one entry. Please fix the binding first. This was added with MT8183, and the fix should list the relavent commit.quoted
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>; + clocks = <&topckgen CLK_TOP_MSDC50_0>, + <&infracfg_ao CLK_INFRA_AO_MSDC0>, + <&infracfg_ao CLK_INFRA_AO_MSDC0_SRC>; + clock-names = "source", "hclk", "source_cg"; + status = "disabled"; + }; +[...]quoted
+ + 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>; + phys = <&u2port1 PHY_TYPE_USB2>;Shouldn't there be a USB3 phy?currently only enable usb2, usb3 phy is used by pcie.
Got it.
quoted
quoted
+ 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>, + <&topckgen CLK_TOP_SSUSB_P1_REF>, + <&pericfg_ao CLK_PERI_AO_SSUSB_1P_XHCI>, + <&apmixedsys CLK_APMIXED_USB1PLL>; + clock-names = "sys_ck", "ref_ck", "xhci_ck", "mcu_ck"; + 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>, + <&topckgen CLK_TOP_SSUSB_P2_REF>, + <&pericfg_ao CLK_PERI_AO_SSUSB_2P_XHCI>; + clock-names = "sys_ck", "ref_ck", "xhci_ck"; + 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>; + 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"; + usb2-lpm-disable;Could you explain why this is needed only for this controller?This controller is fixed with a BT, there is something issue when enable usb2 lpm, so just disabled tmp.
Please add a comment explaining things.
quoted
quoted
+ status = "disabled"; + }; + + u3phy2: t-phy@11c40000 {Just "phy" for the node name. (Or maybe "serdes".) t-phy is not generic.following t-phy’s dt-binding. here using t-phy is to avoid dt-check warning, because it has some sub- phys.
I see. t-phy it is, then.
quoted
quoted
+ compatible = "mediatek,mt8195-tphy", "mediatek,generic-tphy-v3"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0 0x11c40000 0x700>; + status = "disabled"; + + u2port2: usb-phy@0 { + reg = <0x0 0x700>; + clocks = <&topckgen CLK_TOP_SSUSB_PHY_P2_REF>; + clock-names = "ref"; + #phy-cells = <1>; + }; + }; +[...]quoted
+ ufsphy: ufs-phy@11fa0000 {I would have preferred "phy" for the device node, but this seems already defined in the binding. This IP block is not listed in the datasheet I have, so I am unable to verify the properties listed here.quoted
+ compatible = "mediatek,mt8195-ufsphy", "mediatek,mt8183-ufsphy"; + reg = <0 0x11fa0000 0 0xc000>; + clocks = <&clk26m>, <&clk26m>; + clock-names = "unipro", "mp"; + #phy-cells = <0>; + status = "disabled"; + }; +Most of the issues I raised in this version were issues with things not matching the bindings. Please apply your patches on -next and run `make dtbs_check`.ok. I'll apply comments at next version. Thank you.quoted
ChenYu
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel