Re: [PATCH v2 2/3] arm64: dts: freescale: Add support for Variscite VAR-SMARC-MX8M-PLUS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] arm64: dts: freescale: invalid vddio-supply on imx8mp-var-smarc
  PHY nodes
- [Low] arm64: dts: freescale: invalid 11536000 Hz clock rate on sai3
  node
--

commit c95e5618fc5c11cbd6b4b895fc99908ac72c78a0
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: freescale: Add support for Variscite VAR-SMARC-MX8M-PLUS

This commit adds the common device tree include file for the Variscite
VAR-SMARC-MX8M-PLUS system on module.

Link: https://variscite.com/system-on-module-som/i-mx-8/i-mx-8m-plus/var-smarc-mx8m-plus/

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-var-smarc.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-var-smarc.dtsi
> new file mode 100644
> index 0000000000000..139815cfd5f8c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-var-smarc.dtsi
> @@ -0,0 +1,640 @@

[ ... ]

> +	reg_phy0_vddio: regulator-phy0-vddio {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vddio_phy0_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};
> +
> +	reg_phy1_vddio: regulator-phy1-vddio {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vddio_phy1_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};

[ ... ]

> +		ethphy0: ethernet-phy@4 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <4>;
> +			reset-gpios = <&pcal6416 6 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <100000>;
> +			vddio-supply = <&reg_phy0_vddio>;
> +			ti,min-output-impedance;
> +			enet-phy-lane-no-swap;
> +		};
> +
> +		ethphy1: ethernet-phy@5 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <5>;
> +			reset-gpios = <&pcal6416 7 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <10000>;
> +			reset-deassert-us = <20000>;
> +			vddio-supply = <&reg_phy1_vddio>;
> +			ti,min-output-impedance;
> +			enet-phy-lane-no-swap;
> +		};

[Severity: Low]
Are vddio-supply, reg_phy0_vddio and reg_phy1_vddio valid for ethphy0 and
ethphy1?

Neither Documentation/devicetree/bindings/net/ti,dp83867.yaml (indicated
by ti,min-output-impedance) nor drivers/net/phy/dp83867.c defines or
requests vddio-supply, so the property fails schema validation and leaves
both fixed regulators unused.  Even if a SOM variant populates an AR8031
PHY, at8031_register_regulators() matches a child node named
vddio-regulator under the PHY node to configure its internal 1.8V RGMII
regulator, so pointing vddio-supply at top-level fixed regulators will not
program the AR8031 internal regulator either.

[ ... ]

> +&sai3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai3>;
> +	assigned-clocks = <&clk IMX8MP_CLK_SAI3>;
> +	assigned-clock-parents = <&clk IMX8MP_AUDIO_PLL1_OUT>;
> +	assigned-clock-rates = <11536000>;

[Severity: Low]
Should assigned-clock-rates on sai3 be 12288000 instead of 11536000?

IMX8MP_AUDIO_PLL1_OUT runs at 393216000 Hz (32 * 12288000 Hz), so
requesting 11536000 Hz selects a /34 divider and outputs 11565176 Hz on
IMX8MP_CLK_AUDIOMIX_SAI3_MCLK1.  Because clocks is only specified on
wm8904_smarc and not on smarc_codec_dai, simple-audio-card does not
reprogram MCLK during hw_params, and wm8904_set_sysclk() sees 11565176 Hz
instead of 12288000 Hz and falls back to its internal FLL rather than
using MCLK directly for 48 kHz audio.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794420001.git.stefano.r@variscite.com?part=2