HI Nícolas,

Thanks for your reminder.

On Mon, 2022-08-01 at 18:01 -0400, Nícolas F. R. A. Prado wrote:

Hi Allen,

On Mon, Aug 01, 2022 at 08:39:52PM +0800, Allen-KH Cheng wrote:

quoted

Add basic chip support for MediaTek MT8186.

Signed-off-by: Allen-KH Cheng <redacted>
---
 arch/arm64/boot/dts/mediatek/Makefile       |   1 +
 arch/arm64/boot/dts/mediatek/mt8186-evb.dts | 238 ++++++
 arch/arm64/boot/dts/mediatek/mt8186.dtsi    | 874
++++++++++++++++++++
 3 files changed, 1113 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8186-evb.dts
 create mode 100644 arch/arm64/boot/dts/mediatek/mt8186.dtsi

[..]

quoted

diff --git a/arch/arm64/boot/dts/mediatek/mt8186-evb.dts

b/arch/arm64/boot/dts/mediatek/mt8186-evb.dts
new file mode 100644
index 000000000000..7d4a64ab95e4

--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8186-evb.dts

[..]

quoted

+&pio {
+	i2c0_pins: i2c0-default-pins {
+		pins-bus {
+			pinmux = <PINMUX_GPIO128__FUNC_SDA0>,
+				 <PINMUX_GPIO127__FUNC_SCL0>;
+			bias-disable;
+			drive-strength-microamp = <1000>;
+			drive-strength = <4>;

You can't use both drive-strength-microamp and drive-strength. You
only want the
microamp one here (and for the others below), since these are i2c
pins.

I will remove drive-strength for i2c pins.

quoted

+			input-enable;
+		};
+	};

[..]

quoted

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi

b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
new file mode 100644
index 000000000000..38f9466eea42

--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi

@@ -0,0 +1,874 @@

[..]

quoted

+		pio: pinctrl@10005000 {
+			compatible = "mediatek,mt8186-pinctrl";
+			reg = <0 0x10005000 0 0x1000>,
+			      <0 0x10002000 0 0x0200>,
+			      <0 0x10002200 0 0x0200>,
+			      <0 0x10002400 0 0x0200>,
+			      <0 0x10002600 0 0x0200>,
+			      <0 0x10002A00 0 0x0200>,
+			      <0 0x10002C00 0 0x0200>,
+			      <0 0x1000B000 0 0x1000>;

Please use lowercase hex for addresses.

ok.

quoted

+			reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
"iocfg_br",
+				    "iocfg_lm", "iocfg_rb", "iocfg_tl",
"eint";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pio 0 0 185>;
+			interrupt-controller;
+			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH
0>;
+			#interrupt-cells = <2>;
+		};

[..]

quoted

+		xhci0: usb@11200000 {
+			compatible = "mediatek,mt8186-xhci",
+				     "mediatek,mtk-xhci";
+			reg = <0 0x11200000 0 0x1000>,
+			      <0 0x11203e00 0 0x0100>;
+			reg-names = "mac", "ippc";
+			interrupts = <GIC_SPI 294 IRQ_TYPE_LEVEL_HIGH
0>;
+			phys = <&u2port0 PHY_TYPE_USB2>;
+			clocks = <&topckgen CLK_TOP_USB_TOP>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_XHCI>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_HCLK>,
+				 <&infracfg_ao CLK_INFRA_AO_ICUSB>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_REF>;
+			clock-names = "sys_ck", "ref_ck", "mcu_ck",
"dma_ck", "xhci_ck";

The clocks and clock-names don't match, can you confirm whether
they're right?
(But keep this clock order, see the comment on xhci1 below)

The clocks of ref_ck and xhci_ck are not correct. I will update in next
version.

quoted

+			mediatek,syscon-wakeup = <&pericfg 0x420 2>;
+			wakeup-source;
+			status = "disabled";
+		};

[..]

quoted

+		xhci1: usb@11280000 {
+			compatible = "mediatek,mt8186-xhci",
+				     "mediatek,mtk-xhci";
+			reg = <0 0x11280000 0 0x1000>,
+			      <0 0x11283e00 0 0x0100>;
+			reg-names = "mac", "ippc";
+			interrupts = <GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH
0>;
+			phys = <&u2port1 PHY_TYPE_USB2>,
+			       <&u3port1 PHY_TYPE_USB3>;
+			clocks = <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_P1_SYS>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_P1_XHCI>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_P1_HCLK>,
+				 <&infracfg_ao
CLK_INFRA_AO_SSUSB_TOP_P1_REF>;
+			clock-names = "sys_ck", "ref_ck", "mcu_ck",
"xhci_ck";

The xhci dt-binding enforces this clock order:

	sys_ck, ref_ck, mcu_ck, dma_ck, xhci_ck

You can't skip dma_ck, otherwise you'll see a dtbs_check warning. So
if this
hardware block has a fixed clock connected to the dma_ck slot (ie it
isn't
controllable), then just add a fixed clock entry for it (eg
<&clk26m>).

The clock names also look switched here, please double-check.

Thanks,
Nícolas

The hw does not control this. I will add fixed clock for dma_ck.

Thanks,
Allen

quoted

+			mediatek,syscon-wakeup = <&pericfg 0x424 2>;
+			wakeup-source;
+			status = "disabled";
+		};

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel