[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250816151015.621f8da4@minigeek.lan>
Date: Sat, 16 Aug 2025 15:10:15 +0100
From: Andre Przywara <andre.przywara@....com>
To: iuncuim <iuncuim@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>, Jernej
Skrabec <jernej.skrabec@...il.com>, Samuel Holland <samuel@...lland.org>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I
<kishon@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-phy@...ts.infradead.org,
linux-clk@...r.kernel.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH 6/7] arm64: dts: allwinner: a523: add DWC3 USB3.0 node
On Sat, 16 Aug 2025 16:46:59 +0800
iuncuim <iuncuim@...il.com> wrote:
Hi,
> From: Mikhail Kalashnikov <iuncuim@...il.com>
>
> After adding the phy bindings, we can also add dwc3 node, which uses the
> previously added usbphy2 and part of usb3 from combophy.
> All settings declared in dwc3 node are obtained from the x96qproplus' dtb.
> BSP contains an additional glue driver for dwc3, but it seems that it is
> not needed.
>
> Signed-off-by: Mikhail Kalashnikov <iuncuim@...il.com>
> ---
> .../arm64/boot/dts/allwinner/sun55i-a523.dtsi | 21 +++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> index 233365496..ec170888a 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> @@ -606,6 +606,27 @@ mdio0: mdio {
> };
> };
>
> + dwc3: usb@...0000 {
> + compatible = "snps,dwc3";
Wouldn't we need an A523 specific compatible string first?
> + reg = <0x04d00000 0x100000>;
> + interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> + dr_mode = "host";
> + clocks = <&ccu CLK_MBUS_USB3>, <&ccu CLK_USB3>,
> + <&ccu CLK_USB2>, <&ccu CLK_USB3_SUSPEND>;
> + clock-names = "bus_clk", "ref_clk3", "ref_clk2", "suspend";
How does this work, exactly? I see "bus_clk" (deprecated, should be
"bus_early") and "suspend" in the bindings and the Linux driver, but
where do ref_clk3 and ref_clk2 come from, and more importantly who is
going to use them? IIUC, the binding hints that certain implementations
could need more clocks, but then it's their responsibility to parse and
enable them, in platform specific glue code, I think.
> + maximum-speed = "super-speed";
> + phy_type = "utmi";
> + snps,dis_enblslpm_quirk;
> + snps,dis-u1-entry-quirk;
> + snps,dis-u2-entry-quirk;
> + snps,dis_u3_susphy_quirk;
> + snps,dis_u2_susphy_quirk;
> + snps,dis_rxdet_inp3_quirk;
> + phys = <&usbphy 2>, <&combophy>;
Related to my comment on the other DT patch, this should be
"<&combophy 0>" (with "#phy-cells = <1>;" in the PHY node, to be forward
compatible.
Cheers,
Andre
> + phy-names = "usb2-phy", "usb3-phy";
> + status = "disabled";
> + };
> +
> combophy: phy@...0000 {
> compatible = "allwinner,sun55i-a523-usb3-pcie-phy";
> reg = <0x04f00000 0x100000>;
Powered by blists - more mailing lists