Re: [PATCH v6 7/7] arm64: dts: imx8mm: imx8mp: Add FIO1-Audio DTOs for Data Modul i.MX8M Mini and Plus eDM SBC

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The i.MX8MP overlay fails to override the interrupt for the PCA9554 IO expander (`gpio_feature`), causing the driver to listen on an incorrect SoC pin and silently drop interrupts.
- [Medium] The commit message claims the overlay provides 'additional UARTs' and a 'PWM Beeper', but these features are missing from the overlay implementation.
- [Medium] The `imx8mp` overlay fails to configure the parent clock and rate for the SAI5 clock, potentially leaving the SGTL5000 codec's master clock in an undefined state.
- [Low] The overlays introduce a new `clock-controller` node with the same unit address as the existing `sai` audio interface node, causing a duplicate unit-address DTC warning.
--

--- Patch [7]: [PATCH v6 7/7] arm64: dts: imx8mm: imx8mp: Add FIO1-Audio DTOs for Data Modul i.MX8M Mini and Plus eDM SBC ---
commit cabe51dd814613d42f8b138a12dbbf1692edd096
Author: Marek Vasut <[email protected]>
Date:   Mon Jul 27 19:52:22 2026 +0000

arm64: dts: imx8mm: imx8mp: Add FIO1-Audio DTOs for Data Modul i.MX8M Mini and Plus eDM SBC

Add DT overlay for feature connector expansion module eDM-MOD-iMX8Mm-FIO1-Audio
providing additional UARTs, CAN, PWM Beeper, I2C, SPI, GPIO breakout and
SGTL5000 codec. This adapter can be optionally populated onto the eDM SBC.

> Add DT overlay for feature connector expansion module eDM-MOD-iMX8Mm-FIO1-Audio
> providing additional UARTs, CAN, PWM Beeper, I2C, SPI, GPIO breakout and
> SGTL5000 codec.

[Severity: Medium]
Is the commit message accurate about providing "additional UARTs" and a "PWM
Beeper"? The included overlay explicitly disables uart4 and lacks any PWM
beeper configuration.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtsi
> new file mode 100644
> index 0000000000000..01cf5c89dbff4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtsi

[ ... ]

> +	gpio_feature: io-expander@20 {
> +		compatible = "nxp,pca9554";
> +		reg = <0x20>;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		interrupt-parent = <&gpio5>;
> +		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;

[Severity: Medium]
Since this .dtsi file is shared, will this hardcoded interrupt mapping to
gpio5 work for i.MX8MP? The i.MX8MP overlay does not override these
properties, which could cause the io-expander driver to listen on the wrong
SoC pin and drop interrupts.

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso b/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso
> new file mode 100644
> index 0000000000000..4a02bd026ac16
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso

[ ... ]

> +&spba2 {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	sai5clk: clock-controller@30050000 {	/* SAI5 */
> +		compatible = "fsl,imx8mm-sai-clock", "fsl,imx8mq-sai-clock";

[Severity: Low]
Does this create a duplicate node overlapping with the existing sai@30050000
node on i.MX8MM? Should this override the &sai5 label instead to avoid a DTC
unit-address warning?

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso b/arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso
> new file mode 100644
> index 0000000000000..90c2dd1e67248
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc-overlay-edm-mod-imx8mm-fio1-audio.dtso

[ ... ]

> +&spba5 {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	sai5clk: clock-controller@30c50000 {
> +		compatible = "fsl,imx8mp-sai-clock", "fsl,imx8mq-sai-clock";
> +		reg = <0x30c50000 0x10000>;
> +		#clock-cells = <1>;
> +		clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SAI5_IPG>,
> +			 <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SAI5_MCLK1>;
> +		clock-names = "bus", "mclk1";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_codec_mclk>;
> +		status = "okay";
> +	};
> +};

[Severity: Low]
Does this create a duplicate node overlapping with the existing sai@30c50000
node on i.MX8MP? Should this override the &sai5 label instead to avoid a DTC
unit-address warning?

[Severity: Medium]
Will the SGTL5000 codec get a reliable master clock on i.MX8MP? The i.MX8MM
overlay sets assigned-clocks and assigned-clock-rates for sai5clk, but they
are missing here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7