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
prev parent 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