Re: [PATCH v1 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 11536000 Hz clock rate on sai3
  node
- [Low] arm64: dts: freescale: unused vddio-supply on TI DP83867 PHY
  nodes
--

commit 50d8452ea2f7f3e97972d70c5d7ed45e6dc9ead0
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 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
[ ... ]
> +	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>;
> +	};
[ ... ]
> +&eqos {
[ ... ]
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		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 needed on ethphy0 and
ethphy1?

Neither Documentation/devicetree/bindings/net/ti,dp83867.yaml nor
Documentation/devicetree/bindings/net/ethernet-phy.yaml defines
vddio-supply, so it fails schema validation under ti,dp83867.yaml's
unevaluatedProperties: false rule.  Neither the DP83867 driver nor the PHY
core requests vddio-supply, leaving reg_phy0_vddio and reg_phy1_vddio
unclaimed to be disabled as unused regulators at late_initcall.

[ ... ]
> +&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).  Requesting
11536000 Hz selects a /34 divider (11565176 Hz), which is not a 256 * fs
multiple for any standard audio rate.  Because clocks is specified on
wm8904_smarc rather than on the simple-audio-card,codec subnode,
simple_util_parse_clk() leaves simple_dai->clk unset and never updates the
MCLK rate in hw_params, so wm8904_set_sysclk() sees 11565176 Hz !=
12288000 Hz at 48 kHz and has to fall back to the internal FLL instead of
using MCLK directly.

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