Re: [PATCH 08/10] arm64: dts: amlogic: meson-s4: add the USB host

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] arm64: dts: amlogic: unmatched s4-usb2-phy compatible breaks
  USB probe
- [Medium] arm64: dts: amlogic: meson-s4 USB glue and xHCI violate DT
  binding
- [Low] arm64: dts: amlogic: meson-s4 USB2 PHY nodes out of address
  order
--

commit 52aac76c3162fb981eef1f4f1e554811e324a07e
Author: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

arm64: dts: amlogic: meson-s4: add the USB host

This commit adds the two USB2 PHY nodes and the USB controller node with
its child xHCI host node to the Amlogic Meson S4 SoC device tree.

> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> index 8bbd70e52a299..155997649dc3a 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> @@ -770,6 +770,26 @@ cpu_temp: temperature-sensor@20000 {
>  				amlogic,secure-monitor = <&sm 1>;
>  			};
>  
> +			usb2_phy0: phy@3c000 {
> +				compatible = "amlogic,s4-usb2-phy";
> +				reg = <0x0 0x3c000 0x0 0x2000>;
> +				clocks = <&xtal>;
> +				clock-names = "xtal";
> +				resets = <&reset RESET_USBPHY20>;
> +				reset-names = "phy";
> +				#phy-cells = <0>;
> +			};
> +
> +			usb2_phy1: phy@3e000 {
> +				compatible = "amlogic,s4-usb2-phy";
> +				reg = <0x0 0x3e000 0x0 0x2000>;
> +				clocks = <&xtal>;
> +				clock-names = "xtal";
> +				resets = <&reset RESET_USBPHY21>;
> +				reset-names = "phy";
> +				#phy-cells = <0>;
> +			};

[Severity: High]
Does "amlogic,s4-usb2-phy" have a matching driver or binding in the kernel,
or does it need a fallback such as "amlogic,g12a-usb2-phy"?

phy_meson_g12a_usb2_of_match[] in drivers/phy/amlogic/phy-meson-g12a-usb2.c
and Documentation/devicetree/bindings/phy/amlogic,g12a-usb2-phy.yaml only
list "amlogic,g12a-usb2-phy" and "amlogic,a1-usb2-phy". Without a matching
driver or fallback compatible on usb2_phy0 and usb2_phy1, no PHY provider
is registered, so dwc3_meson_g12a_probe() -> dwc3_meson_g12a_get_phys() ->
devm_phy_optional_get() returns -EPROBE_DEFER permanently when usb@fe03a000
is enabled, and dtbs_check fails.

> +
>  			eth_phy: mdio-multiplexer@28000 {
>  				compatible = "amlogic,g12a-mdio-mux";
>  				reg = <0x0 0x28000 0x0 0xa4>;

[Severity: Low]
This isn't a bug, but phy@3c000 and phy@3e000 are placed before
mdio-multiplexer@28000 instead of after it in unit-address order under
apb4.

> @@ -988,6 +1008,31 @@ mdio0: mdio {
>  			};
>  		};
>  
> +		usb: usb@fe03a000 {
> +			compatible = "amlogic,meson-s4-usb-ctrl",
> +				     "amlogic,meson-g12a-usb-ctrl";
> +			reg = <0x0 0xfe03a000 0x0 0xa0>;
> +			interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			clocks = <&clkc_periphs CLKID_USB>;
> +			resets = <&reset RESET_USBCTRL>;
> +			power-domains = <&pwrc PWRC_S4_USB_COMB_ID>;
> +			dr_mode = "host";
> +			phys = <&usb2_phy0>, <&usb2_phy1>;
> +			phy-names = "usb2-phy0", "usb2-phy1";
> +			status = "disabled";
> +
> +			crg_xhci: usb@fde00000 {
> +				compatible = "generic-xhci";
> +				reg = <0x0 0xfde00000 0x0 0x100000>;
> +				interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clkc_periphs CLKID_USB1_TO_DDR>;
> +				status = "okay";
> +			};
> +		};

[Severity: Medium]
Do the compatible strings on usb@fe03a000 and its child usb@fde00000 match
Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml and
drivers/usb/dwc3/dwc3-meson-g12a.c?

The binding only allows a single compatible entry from its enum (which does
not include "amlogic,meson-s4-usb-ctrl") and restricts child usb@ nodes to
dwc2.yaml# or snps,dwc3.yaml#, so both nodes fail dtbs_check. In addition,
dwc3_meson_g12a_otg_init() looks up the host child via
dwc3_meson_g12_find_child(dev, "snps,dwc3"), which returns NULL when the
child uses "generic-xhci".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106104659.54656-1-christian.koever-draxl@student.uibk.ac.at?part=8

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