From: allen-kh.cheng <allen-kh.cheng@mediatek.com>
To: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
<Project_Global_Chrome_Upstream_Group@mediatek.com>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<linux-mediatek@lists.infradead.org>,
"Chen-Yu Tsai" <wenst@chromium.org>, <hsinyi@chromium.org>
Subject: Re: [PATCH v12 1/1] arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile
Date: Tue, 2 Aug 2022 16:06:31 +0800 [thread overview]
Message-ID: <2e221c97699fa540ac416518d730b13c83e12da8.camel@mediatek.com> (raw)
In-Reply-To: <20220801220136.4lll2wifc7muocuc@notapiano>
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:
> > Add basic chip support for MediaTek MT8186.
> >
> > Signed-off-by: Allen-KH Cheng <Allen-KH.Cheng@mediatek.com>
> > ---
> > 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
> >
>
> [..]
> > 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
>
> [..]
> > +&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.
> > + input-enable;
> > + };
> > + };
>
> [..]
> > 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 @@
>
> [..]
> > + 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.
> > + 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>;
> > + };
>
> [..]
> > + 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.
> > + mediatek,syscon-wakeup = <&pericfg 0x420 2>;
> > + wakeup-source;
> > + status = "disabled";
> > + };
>
> [..]
> > + 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
> > + 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
prev parent reply other threads:[~2022-08-02 8:48 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-01 12:39 [PATCH v12 0/1] Add basic node support for MediaTek MT8186 SoC Allen-KH Cheng
2022-08-01 12:39 ` [PATCH v12 1/1] arm64: dts: Add MediaTek MT8186 dts and evaluation board and Makefile Allen-KH Cheng
2022-08-01 22:01 ` Nícolas F. R. A. Prado
2022-08-02 8:06 ` allen-kh.cheng [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2e221c97699fa540ac416518d730b13c83e12da8.camel@mediatek.com \
--to=allen-kh.cheng@mediatek.com \
--cc=Project_Global_Chrome_Upstream_Group@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=hsinyi@chromium.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=nfraprado@collabora.com \
--cc=robh+dt@kernel.org \
--cc=wenst@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).