Re: [PATCH v4 2/3] arm64: dts: freescale: Add support for Variscite DART-MX8M-PLUS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The SAI3 audio clock is assigned a non-standard frequency of 11536000 Hz, which is highly likely a typographical error for 11289600 Hz.
- [Medium] Bluetooth wakeup and enable GPIOs (GPIO2_IO06, GPIO2_IO09) are configured in pinctrl but never bound to the bluetooth driver, which breaks hardware sleep/wake signaling.
- [Low] The MX8MP_IOMUXC_SD2_WP__GPIO2_IO20 pin is configured in `pinctrl_eqos` but is never consumed by the Ethernet PHY or any regulator node, potentially leaving the Ethernet PHY unpowered or unnecessarily reserving a pin.
--

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

arm64: dts: freescale: Add support for Variscite DART-MX8M-PLUS

Add device tree support for the Variscite DART-MX8MP system on module.
This SOM is designed to be used with various carrier boards. Only
SOM-specific peripherals are enabled by default.

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

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-var-dart.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-var-dart.dtsi
> new file mode 100644
> index 0000000000000..a647a2e0ab94b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-var-dart.dtsi

[ ... ]

> +&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: High]
Does this assigned clock rate contain a typo?

A frequency of 11536000 Hz does not map to any standard audio sample
rate (e.g., 11289600 Hz is standard for 44.1kHz * 256fs).

Could this cause the ALSA framework to operate the codec at an incorrect,
non-standard drift frequency, resulting in subtle pitch shifts or failing
to configure the audio PLL cleanly?

> +	clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SAI3_IPG>,
> +		<&clk IMX8MP_CLK_DUMMY>,

[ ... ]

> +/* BT module */
> +&uart4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart4>, <&pinctrl_bt>;
> +	assigned-clocks = <&clk IMX8MP_CLK_UART4>;
> +	assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_80M>;
> +	uart-has-rtscts;
> +	status = "okay";
> +
> +	bluetooth_iw61x: bluetooth {
> +		compatible = "nxp,88w8987-bt";
> +	};

[Severity: Medium]
Should this node include the GPIO bindings required to utilize the pins
configured in pinctrl_bt?

The pinctrl_bt group configures GPIO2_IO06 and GPIO2_IO09, but this bluetooth
node never references these pins via device-wakeup-gpios or reset-gpios.

Without the kernel driving or monitoring these lines, do we risk optimal
low-power state transitions failing and the system relying entirely on
implicit hardware pull-ups?

> +};

[ ... ]

> +&iomuxc {
> +	pinctrl_bt: btgrp {

[ ... ]

> +	pinctrl_eqos: eqosgrp {
> +		fsl,pins = <
> +			MX8MP_IOMUXC_ENET_MDC__ENET_QOS_MDC				0x2
> +			MX8MP_IOMUXC_ENET_MDIO__ENET_QOS_MDIO				0x2
> +			MX8MP_IOMUXC_ENET_RD0__ENET_QOS_RGMII_RD0			0x90

[ ... ]

> +			MX8MP_IOMUXC_ENET_TXC__CCM_ENET_QOS_CLOCK_GENERATE_TX_CLK	0x16
> +			MX8MP_IOMUXC_SD2_WP__GPIO2_IO20					0x10

[Severity: Low]
Is the GPIO2_IO20 pin correctly reserved here?

This GPIO is configured in pinctrl_eqos but is never consumed by the Ethernet
PHY or any regulator node.

If it is genuinely unused on this module, should it be removed from the pinctrl
to prevent incorrectly reserving a pin that should be left available for the
carrier board?

> +			MX8MP_IOMUXC_SD1_STROBE__GPIO2_IO11				0x150
> +		>;
> +	};

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