Re: [PATCH 3/3] arm64: dts: rockchip: add support for NanoPi M6 board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: rockchip: wrong MCLK clock in rk3588s-nanopi-m6v2 audio-codec
- [Low] arm64: dts: rockchip: commit subject names NanoPi M6 instead of NanoPi M6V2
--

commit b98c017b6747ab0aa87e4af9234cbf5396581d64
Author: Joachim Eastwood <joachim.eastwood@gmail.com>

arm64: dts: rockchip: add support for NanoPi M6 board

This commit adds the device tree for the FriendlyElec NanoPi M6V2 board,
including support for its Everest ES8389 audio codec, regulator, and user
button.

>     arm64: dts: rockchip: add support for NanoPi M6 board
>     
>     The NanoPi M6V2 board is an updated version of the original M6 board
>     with the following changes.
>     * New audio CODEC (Everest ES8389)
>     * New Ethernet PHY (MotorComm YT8531C)
>     * 1 extra user button
>     * Misc connector changes on the board
>     
>     The codec_es_3v3 regulator enable pin is hardwired to the 5V input rail.
>     
>     See link below for detailed information:
>     https://wiki.friendlyelec.com/wiki/index.php/NanoPi_M6V2
>     
>     Signed-off-by: Joachim Eastwood <joachim.eastwood@gmail.com>

[Severity: Low]
This isn't a bug, but commit 2640aea473b4 ("arm64: dts: rockchip: add support
for NanoPi M6 board") already used this exact subject line when adding
rk3588s-nanopi-m6.dtsi and rk3588s-nanopi-m6v1.dts. Since this commit adds
rk3588s-nanopi-m6v2.dts for the FriendlyElec NanoPi M6V2, should the commit
subject say NanoPi M6V2 instead of NanoPi M6?

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6v2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6v2.dts
> new file mode 100644
> index 0000000000000..2ac00f2de6844
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6v2.dts
> @@ -0,0 +1,90 @@

[ ... ]

> +&i2c7 {
> +	audio-codec@10 {
> +		compatible = "everest,es8389";
> +		reg = <0x10>;
> +		assigned-clocks = <&cru I2S1_8CH_MCLKOUT>;
> +		assigned-clock-rates = <12288000>;
> +		clocks = <&cru I2S1_8CH_MCLKOUT>;
> +		clock-names = "mclk";
> +		vddd-supply = <&vcc_1v8_s3>;
> +		vdda-supply = <&codec_es_3v3>;
> +		#sound-dai-cells = <0>;
> +
> +		port {
> +			es8389_p0: endpoint {
> +				remote-endpoint = <&i2s0_8ch_p0_0>;
> +			};
> +		};
> +	};
> +};
> +
> +&i2s0_8ch {
> +	i2s0_8ch_p0: port {
> +		i2s0_8ch_p0_0: endpoint {
> +			dai-format = "i2s";
> +			mclk-fs = <256>;
> +			remote-endpoint = <&es8389_p0>;
> +		};
> +	};
> +};

[Severity: Medium]
Should audio-codec@10 under i2c7 use <&cru I2S0_8CH_MCLKOUT_TO_IO> (or
I2S0_8CH_MCLKOUT) instead of <&cru I2S1_8CH_MCLKOUT> for assigned-clocks and
clocks?

In arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6v2.dts, the codec endpoint
es8389_p0 is connected to i2s0_8ch_p0_0 under i2s0_8ch, and the included
arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6.dtsi enables i2s0_8ch with the
i2s0_mclk pin muxed:

&i2s0_8ch {
	pinctrl-names = "default";
	pinctrl-0 = <&i2s0_lrck
		     &i2s0_mclk
		     &i2s0_sclk
		     &i2s0_sdi0
		     &i2s0_sdo0>;
	status = "okay";
};

For comparison, audio-codec@1b in
arch/arm64/boot/dts/rockchip/rk3588s-nanopi-m6v1.dts uses:

		assigned-clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;
		assigned-clock-rates = <12288000>;
		clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;

With I2S1_8CH_MCLKOUT specified here, of_clk_set_defaults(),
simple_util_parse_clk(), es8389_probe(), and es8389_set_bias_level() will
configure and gate the unused I2S1 MCLK clock branch rather than the I2S0
MCLK output driving the codec.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-nanopi-m6v2-v1-0-e401ea64b394@gmail.com?part=3