Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: Biju <biju.das.au@gmail.com>
Cc: wsa+renesas@sang-engineering.com, robh@kernel.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v19 2/4] arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK
Date: Thu, 23 Jul 2026 16:03:03 +0000	[thread overview]
Message-ID: <20260723160321.21B771F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260723154154.319674-3-biju.das.jz@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The SDHI device tree nodes specify only the SoC-specific compatible string (`renesas,sdhi-r9a08g046`) and omit the required fallback string (`renesas,rzg2l-sdhi`).
- [Medium] Missing `#else` branch leaves `sdhi1` enabled without pin multiplexing or supplies when `RZ_BOOT_MODE3` is disabled.
- [Medium] The pin configuration for `sd1-data` in the `sdhi1_pins` and `sdhi1_uhs_pins` nodes is missing the `bias-pull-up` property.
--

--- Patch [2]: [PATCH v19 2/4] arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK ---
commit 6289153083cb6f174266f6b60614c70a75cd4294
Author: Biju Das <biju.das.jz@bp.renesas.com>

arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK
    
Add device tree nodes for the three SDHI controllers (SDHI{0,1,2})
on the RZ/G3L SoC (r9a08g046) and enable SDHI1 on the RZ/G3L SMARC
EVK platform with pincontrol and GPIO-based voltage switching
regulator support.

> diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> --- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> @@ -762,9 +762,80 @@ dmac: dma-controller@11820000 {
>  			dma-channels = <16>;
>  		};
>  
> +		sdhi0: mmc@11c00000 {
> +			compatible = "renesas,sdhi-r9a08g046";

[Severity: High]
Does this node need the "renesas,rzg2l-sdhi" fallback compatible string?

The driver source code for SDHI does not contain a direct match for
"renesas,sdhi-r9a08g046", so it relies on the "renesas,rzg2l-sdhi"
fallback to successfully probe the device. Without the fallback string,
the driver will not probe the controllers, breaking SD/eMMC support.

This same issue also appears in the sdhi1 and sdhi2 nodes below.

[ ... ]

> diff --git a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
> --- a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
> +++ b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts

[ ... ]

> +	sdhi1_pins: sd1 {
> +		sd1-cd {
> +			pinmux = <RZG3L_PORT_PINMUX(J, 0, 8)>; /* SD1_CD */
> +		};
> +
> +		sd1-clk {
> +			pinmux = <RZG3L_PORT_PINMUX(G, 0, 1)>; /* SD1_CLK */
> +			power-source = <3300>;
> +		};
> +
> +		sd1-cmd {
> +			pinmux = <RZG3L_PORT_PINMUX(G, 1, 1)>; /* SD1_CMD */
> +			input-enable;
> +			power-source = <3300>;
> +			bias-pull-up;
> +		};
> +
> +		sd1-data {
> +			pinmux = <RZG3L_PORT_PINMUX(G, 2, 1)>, /* SD1_DAT0 */
> +				 <RZG3L_PORT_PINMUX(G, 3, 1)>, /* SD1_DAT1 */
> +				 <RZG3L_PORT_PINMUX(G, 4, 1)>, /* SD1_DAT2 */
> +				 <RZG3L_PORT_PINMUX(G, 5, 1)>; /* SD1_DAT3 */
> +			input-enable;
> +			power-source = <3300>;

[Severity: Medium]
Should the data lines also specify bias-pull-up?

The sd1-cmd node correctly includes bias-pull-up. If the board relies on
the SoC's internal pull-ups rather than external resistors, leaving the data
lines floating when the bus is idle could lead to intermittent SD card
communication errors. 

This also applies to the sdhi1_uhs_pins configuration.

[ ... ]

> +#if RZ_BOOT_MODE3
> +&sdhi1 {
> +	pinctrl-0 = <&sdhi1_pins>;
> +	pinctrl-1 = <&sdhi1_uhs_pins>;
> +	pinctrl-names = "default", "state_uhs";
> +
> +	vmmc-supply = <&reg_3p3v>;
> +	vqmmc-supply = <&vqmmc_sd1_pvdd>;
> +};
> +#endif

[Severity: Medium]
Does this conditional block need an #else branch to set status = "disabled"
for &sdhi1 when RZ_BOOT_MODE3 is false?

Since the included renesas-smarc2.dtsi unconditionally sets status = "okay"
for &sdhi1, failing to disable it here when RZ_BOOT_MODE3 is 0 leaves the
node enabled but without required pin multiplexing and supplies. The MMC driver
would then attempt to probe a non-routed device, which could lead to timeouts
and boot delays.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723154154.319674-1-biju.das.jz@bp.renesas.com?part=2

  reply	other threads:[~2026-07-23 16:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23 15:41 [PATCH v19 0/4] Add SDHI support for RZ/G3L SoC Biju
2026-07-23 15:41 ` [PATCH v19 2/4] arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK Biju
2026-07-23 16:03   ` sashiko-bot [this message]
2026-07-23 15:41 ` [PATCH v19 3/4] arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0 Biju
2026-07-23 15:55   ` sashiko-bot
2026-07-23 15:41 ` [PATCH v19 4/4] arm64: dts: renesas: rzg3l-smarc-som: Enable SDHI2 Biju

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=20260723160321.21B771F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=biju.das.au@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wsa+renesas@sang-engineering.com \
    /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