| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5G45UP=kvk8UOiFWYfdWgdjboL-UfkBbfPuEmQpwKMNHQ@mail.gmail.com>
Date: Fri, 7 Jan 2022 16:36:15 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Tinghan Shen <tinghan.shen@...iatek.com>
Cc: robh+dt@...nel.org, linus.walleij@...aro.org,
matthias.bgg@...il.com, broonie@...nel.org,
bgolaszewski@...libre.com, sean.wang@...iatek.com,
bayi.cheng@...iatek.com, gch981213@...il.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-spi@...r.kernel.org,
Project_Global_Chrome_Upstream_Group@...iatek.com,
Seiya Wang <seiya.wang@...iatek.com>, chunfeng.yun@...iatek.com
Subject: Re: [PATCH v7 4/4] arm64: dts: Add mediatek SoC mt8195 and evaluation board
On Thu, Jan 6, 2022 at 7:15 PM Tinghan Shen <tinghan.shen@...iatek.com> wrote:
> On Thu, 2021-12-23 at 17:59 +0800, Chen-Yu Tsai wrote:
> > On Mon, Dec 20, 2021 at 8:20 PM Tinghan Shen <tinghan.shen@...iatek.com> wrote:
[...]
> > [...]
> >
> > > + xhci0: usb@...00000 {
> > > + 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.
>
> ok
>
> > 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.
> >
> > This goes for all the xhci device nodes.
> >
> > > + status = "disabled";
> > > + };
> > > +
> > > + mmc0: mmc@...30000 {
> > > + 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.
> >
> > > + 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";
> > > + };
> > > +
> >
> > [...]
> >
> > > +
> > > + xhci1: usb@...90000 {
> > > + 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.
> >
> > > + 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@...a0000 {
> > > + 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@...b0000 {
> > > + 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.
> > > + status = "disabled";
> > > + };
> > > +
> > > + u3phy2: t-phy@...40000 {
> >
> > 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.
> > > + 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>;
> > > + };
> > > + };
> > > +
> >
> > [...]
> >
> > > + ufsphy: ufs-phy@...a0000 {
> >
> > 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.
> >
> > > + 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.
>
> >
> >
> > ChenYu
>
Powered by blists - more mailing lists