Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Marek Vasut" <marex@nabladev.com>
Cc: conor+dt@kernel.org, Frank.Li@kernel.org, robh@kernel.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH v6 7/7] arm64: dts: imx8mm: imx8mp: Add FIO1-Audio DTOs for Data Modul i.MX8M Mini and Plus eDM SBC
Date: Mon, 27 Jul 2026 20:08:49 +0000	[thread overview]
Message-ID: <20260727200849.96F151F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260727195244.78199-7-marex@nabladev.com>

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 <marex@nabladev.com>
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/20260727195244.78199-1-marex@nabladev.com?part=7

      reply	other threads:[~2026-07-27 20:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27 19:52 [PATCH v6 1/7] arm64: dts: imx8mp: Add DT label to SPBA5 bus Marek Vasut
2026-07-27 19:52 ` [PATCH v6 2/7] arm64: dts: imx8mm: imx8mp: Add LVDS DTOs for Data Modul i.MX8M Mini and Plus eDM SBC Marek Vasut
2026-07-27 20:07   ` sashiko-bot
2026-07-27 19:52 ` [PATCH v6 3/7] arm64: dts: imx8mm: imx8mp: Add legacy board " Marek Vasut
2026-07-27 20:14   ` sashiko-bot
2026-07-27 19:52 ` [PATCH v6 4/7] arm64: dts: imx8mm: imx8mp: Add HDMI " Marek Vasut
2026-07-27 19:52 ` [PATCH v6 5/7] arm64: dts: imx8mm: imx8mp: Add CM " Marek Vasut
2026-07-27 19:52 ` [PATCH v6 6/7] arm64: dts: imx8mm: imx8mp: Add FIO1 " Marek Vasut
2026-07-27 19:52 ` [PATCH v6 7/7] arm64: dts: imx8mm: imx8mp: Add FIO1-Audio " Marek Vasut
2026-07-27 20:08   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260727200849.96F151F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=marex@nabladev.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox