Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Joseph Guo" <qijian.guo@nxp.com>
Cc: imx@lists.linux.dev, devicetree@vger.kernel.org,
	conor+dt@kernel.org, robh@kernel.org, Frank.Li@kernel.org
Subject: Re: [PATCH v4 2/2] arm64: dts: freescale: add i.MX95 19x19 FRDM PRO board dts
Date: Mon, 18 May 2026 03:20:45 +0000	[thread overview]
Message-ID: <20260518032046.198F0C2BCB3@smtp.kernel.org> (raw)
In-Reply-To: <20260518-imx95_frdm_pro-v4-2-975346555a9b@nxp.com>

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Architectural flaw preventing the M7 core from using the CAN1 transceiver.
- [High] Incorrect drive strength in `pinctrl_usdhc2_200mhz` causing potential SD card instability.
- [Medium] Mismatch between commit message claiming 10G ETH support and device tree implementation.
- [Medium] Dead pinctrl configuration in the reserved `flexcan1` node.
- [Low] Unused `reg_3p3v` regulator node.
--

commit 65457c99452cde3ade57a7afda12053dd0e2d5ee
Author: Joseph Guo <qijian.guo@nxp.com>

arm64: dts: freescale: add i.MX95 19x19 FRDM PRO board dts
    
This commit adds the device tree for the NXP i.MX95 19x19 FRDM PRO board,
which is based on the i.MX95 19x19 SoC. It configures various peripherals
including USB, PCIe, ethernet, CAN, and regulators.

> - 10G ETH interface

The commit message claims support for a 10G ETH interface, but the device tree
only configures enetc_port0 and enetc_port1 as 1 Gbps rgmii-id interfaces and
entirely omits the i.MX95 10G Ethernet MAC. Is this omission intentional?

> diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-frdm-pro.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-frdm-pro.dts
> new file mode 100644
> index 0000000000000..b87a26b0d7fcf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-19x19-frdm-pro.dts
> @@ -0,0 +1,1021 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright 2026 NXP
> + */

[ ... ]

> +	flexcan1_phy: can-phy0 {
> +		compatible = "nxp,tja1057";
> +		#phy-cells = <0>;
> +		max-bitrate = <5000000>;
> +		silent-gpios = <&i2c4_gpio_expander_22 11 GPIO_ACTIVE_HIGH>;
> +	};

[ ... ]

> +	reg_3p3v: regulator-3p3v {
> +		compatible = "regulator-fixed";
> +		regulator-name = "+V3.3_SW";
> +		regulator-max-microvolt = <3300000>;
> +		regulator-min-microvolt = <3300000>;
> +	};

Is this regulator used anywhere? It doesn't have the regulator-always-on
property and isn't referenced by any device node as a supply.

[ ... ]

> +&flexcan1 {
> +	phys = <&flexcan1_phy>;
> +	pinctrl-0 = <&pinctrl_flexcan1>;
> +	pinctrl-names = "default";
> +	status = "reserved";
> +};

Will the M7 core be able to use the CAN1 transceiver with this setup?

Since flexcan1 is marked as reserved, the Linux CAN driver won't probe it.
As a result, Linux will never power on flexcan1_phy or drive the standby GPIO
low. The Linux driver core will also ignore the pinctrl-0 configuration.

If the M7 core depends on Linux to set up the pin multiplexing and enable the
PHY, does this need a gpio-hog on the expander and the pinctrl moved to an
active pinctrl_hog group instead?

[ ... ]

> +	pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
> +		fsl,pins = <
> +			IMX95_PAD_SD2_CLK__USDHC2_CLK				0x158e
> +			IMX95_PAD_SD2_CMD__USDHC2_CMD				0x138e
> +			IMX95_PAD_SD2_DATA0__USDHC2_DATA0			0x138e
> +			IMX95_PAD_SD2_DATA1__USDHC2_DATA1			0x138e
> +			IMX95_PAD_SD2_DATA2__USDHC2_DATA2			0x138e
> +			IMX95_PAD_SD2_DATA3__USDHC2_DATA3			0x138e
> +			IMX95_PAD_SD2_VSELECT__USDHC2_VSELECT			0x51e
> +		>;
> +	};

Are these pad settings correct for 200MHz operation?

These use the exact same pad settings (0x138e and 0x158e) as the 100MHz group,
whereas usdhc1 and usdhc3 correctly configure higher drive strength (0x13fe and
0x15fe) for 200MHz. Can this lower drive strength cause signal integrity
issues or CRC errors with the SD card at 200MHz?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-imx95_frdm_pro-v4-0-975346555a9b@nxp.com?part=2

      reply	other threads:[~2026-05-18  3:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18  2:58 [PATCH v4 0/2] Add NXP FRDM-IMX95-PRO board support Joseph Guo
2026-05-18  2:58 ` [PATCH v4 1/2] dt-bindings: arm: fsl: Add i.MX95 19x19 FRDM PRO board Joseph Guo
2026-05-18  2:58 ` [PATCH v4 2/2] arm64: dts: freescale: add i.MX95 19x19 FRDM PRO board dts Joseph Guo
2026-05-18  3:20   ` 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=20260518032046.198F0C2BCB3@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=qijian.guo@nxp.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