Re: [PATCH v2 2/2] arm64: dts: freescale: add initial device tree for TQMa8MPQS with i.MX8MP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] PCIe peripheral reset (PERST#) is statically hogged high instead of being managed by the PCIe controller driver.
- [High] The DisplayPort bridge `reset-gpios` property is configured as `GPIO_ACTIVE_HIGH` for an active-high Enable pin, which will cause the driver to permanently disable the bridge.
- [High] The `.dtsi` file uses the `IMX8_PCIE_REFCLK_PAD_INPUT` macro without including the required header.
- [High] Mismatch in array lengths between `assigned-clocks` and `assigned-clock-parents` in `media_blk_ctrl`.
- [Medium] The pin `MX8MP_IOMUXC_SAI1_RXD3__GPIO4_IO05` is configured twice with conflicting pad settings in the same state.
- [Medium] The `dp-hpd-int-hog` claims a GPIO pin whose pinmux configuration is located in a disabled device node.
- [Medium] The `tmp1075` temperature sensor node lacks the `interrupts` property, disabling its alarm functionality.
- [Medium] Active device interrupt lines are redundantly claimed as `gpio-hog` inputs.
--

commit 94f71b7114703e908e4afab6cc26436862728cfb
Author: Paul Gerber <paul.gerber@tq-group.com>

arm64: dts: freescale: add initial device tree for TQMa8MPQS with i.MX8MP
    
This adds support for TQMa8MPQS module on MB-SMARC-2 board.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi
> new file mode 100644
> index 0000000000000..9576152657405
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpqs.dtsi
> @@ -0,0 +1,1177 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright 2025-2026 TQ-Systems GmbH <linux@ew.tq-group.com>,
> + * D-82229 Seefeld, Germany.
> + * Author: Paul Gerber
> + */
> +
> +#include <dt-bindings/net/ti-dp83867.h>
> +#include "imx8mp.dtsi"

[Severity: High]
Does this file need to include <dt-bindings/phy/phy-imx8-pcie.h>?

The IMX8_PCIE_REFCLK_PAD_INPUT macro is used later in the &pcie_phy node,
which could cause a compilation error if another board includes this .dtsi
without manually adding the missing header.

[ ... ]
> +&gpio1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_gpio1>;
> +
> +	gpio-line-names = "SLEEP", "BATLOW#", "", "LID",
> +			  "", "GPIO10", "CHARGING#", "CHG_PRSNT#",
> +			  "PMIC_IRQ#", "ESPI_CS1_ALERT#", "USB1_OTG_ID", "USB2_OTG_ID",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "",
> +			  "", "", "", "";
> +
> +	pmic_irq_hog: pmic-irq-hog {
> +		gpio-hog;
> +		gpios = <8 0>;
> +		input;
> +		line-name = "PMIC_IRQ#";
> +	};
> +};

[Severity: Medium]
Is this gpio-hog for the interrupt pin necessary?

Active device interrupt lines such as PMIC_IRQ#, ENET0_INT#, ENET_INT#, and
RTC_INT# are assigned to their respective devices using the interrupts
property. Simultaneously claiming these pins as gpio-hog inputs requests
exclusive ownership of the GPIO descriptor, which might conflict with the
interrupt subsystem's attempt to use them.

[ ... ]
> +&gpio2 {
[ ... ]
> +	perst-hog {
> +		gpio-hog;
> +		gpios = <10 0>;
> +		output-high;
> +		line-name = "PERST#";
> +	};

[Severity: High]
Will this static PERST# configuration prevent the PCIe driver from properly
sequencing the reset?

The PCI Express specification requires the host to toggle the PERST# signal
during link training. By hogging the pin statically high and omitting
reset-gpios from the &pcie node, the controller driver cannot toggle the pin.

[ ... ]
> +&gpio3 {
[ ... ]
> +	temp-event--hog {
> +		gpio-hog;
> +		gpios = <28 0>;
> +		input;
> +		line-name = "TEMP_EVENT#";
> +	};
> +};

[Severity: Medium]
Should this pin be configured as an interrupt for the tmp1075 device instead
of a static gpio-hog?

The tmp1075 temperature sensor node applies pinctrl_tmp1075 to multiplex
GPIO3_IO28 for the TEMP_EVENT# alert pin, but lacks the interrupts and
interrupt-parent properties. Without them, the thermal alarm feature is
unusable.

[ ... ]
> +&gpio5 {
[ ... ]
> +	dp-hpd-int-hog {
> +		gpio-hog;
> +		gpios = <21 0>;
> +		input;
> +		line-name = "DP_HPD_INT";
> +	};
> +};

[Severity: Medium]
Will this gpio-hog function correctly when its corresponding pinmux
configuration is located in a disabled device node?

This claims GPIO5_IO21 as an input, but the required pinmux setting
(pinctrl_tc9595) is assigned to the dp_bridge node, which has
status = "disabled". The pinctrl is never applied, leaving the physical
pin in its default state.

[ ... ]
> +&i2c3 {
[ ... ]
> +	dp_bridge: dp-bridge@f {
> +		compatible = "toshiba,tc9595", "toshiba,tc358767";
> +		reg = <0xf>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_tc9595>;
> +		clock-names = "ref";
> +		clocks = <&clk_dp>;
> +		reset-gpios = <&expander0 4 GPIO_ACTIVE_HIGH>;

[Severity: High]
Could this polarity configuration keep the bridge permanently disabled?

The DP_BRIDGE_EN pin is an enable pin that requires driving HIGH. When the
driver deasserts the reset (logical 0), it will drive the physical line LOW
due to the GPIO_ACTIVE_HIGH polarity. Should this be GPIO_ACTIVE_LOW so
that deasserting the reset translates to driving the pin HIGH?

[ ... ]
> +// Change parent clock of disp1 pixel clock to audio_pll2
> +&media_blk_ctrl {
> +	assigned-clocks = <&clk IMX8MP_CLK_MEDIA_AXI>,
> +			  <&clk IMX8MP_CLK_MEDIA_APB>,
> +			  <&clk IMX8MP_CLK_MEDIA_DISP1_PIX>,
> +			  <&clk IMX8MP_CLK_MEDIA_DISP2_PIX>,
> +			  <&clk IMX8MP_CLK_MEDIA_ISP>,
> +			  <&clk IMX8MP_VIDEO_PLL1>;
> +	assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
> +				 <&clk IMX8MP_SYS_PLL1_800M>,
> +				 <&clk IMX8MP_AUDIO_PLL2_OUT>,
> +				 <&clk IMX8MP_VIDEO_PLL1_OUT>,
> +				 <&clk IMX8MP_SYS_PLL2_500M>;
> +};

[Severity: High]
Are we missing a clock parent in the assigned-clock-parents list?

The assigned-clocks list contains 6 elements, but assigned-clock-parents
only has 5. There is also no assigned-clock-rates property. This means
IMX8MP_VIDEO_PLL1 will not find a corresponding parent or rate entry.

[ ... ]
> +&iomuxc {
[ ... ]
> +	pinctrl_espi_rst: espirstgrp {
> +		fsl,pins = <MX8MP_IOMUXC_SAI1_RXD3__GPIO4_IO05		0x144>;
> +	};
[ ... ]
> +	pinctrl_gpio4: gpio4grp {
> +		fsl,pins = <MX8MP_IOMUXC_SAI1_RXC__GPIO4_IO01		0x10>,
> +			   <MX8MP_IOMUXC_SAI1_RXD2__GPIO4_IO04		0x10>,
> +			   <MX8MP_IOMUXC_SAI1_RXD3__GPIO4_IO05		0x10>,

[Severity: Medium]
Is this pin meant to be configured twice with different pad settings?

The &gpio4 node applies both pinctrl_gpio4 and pinctrl_espi_rst, configuring
MX8MP_IOMUXC_SAI1_RXD3__GPIO4_IO05 twice with conflicting settings
(0x144 and 0x10). This creates a pinmux overlap that can result in
unpredictable pin configurations.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520074754.1631543-1-alexander.stein@ew.tq-group.com?part=2