From: sashiko-bot@kernel.org
To: "Monish Chunara" <monish.chunara@oss.qualcomm.com>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH V3 3/3] arm64: dts: qcom: monaco-evk: Add SDHCI support for eMMC via overlay
Date: Wed, 01 Jul 2026 08:29:16 +0000 [thread overview]
Message-ID: <20260701082916.A892A1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260701081715.2580329-4-monish.chunara@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Duplicate regulator node creation and label shadowing in overlay.
- [Low] Redundant device tree overlay duplicating base configuration.
Pre-existing issues:
- [High] Conflicting device tree properties silently disable SD card functionality.
--
commit c17f3092c8fef709ae2384b75b1582cdb37b5837
Author: Monish Chunara <monish.chunara@oss.qualcomm.com>
arm64: dts: qcom: monaco-evk: Add SDHCI support for eMMC via overlay
Enable the SDHCI controller for eMMC functionality on the Monaco EVK
using a device tree overlay.
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 0b60626369190..a341d6dab6150 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -71,6 +71,9 @@ dtb-$(CONFIG_ARCH_QCOM) += monaco-evk-ifp-mezzanine.dtb
> monaco-evk-sd-card-dtbs := monaco-evk.dtb monaco-evk-sd-card.dtbo
[Severity: High]
This is a pre-existing issue, but does applying the monaco-evk-sd-card.dtso
overlay break SD card functionality by conflicting with the base device tree?
The base device tree monaco-evk.dts defaults to eMMC and hardcodes no-sd and
non-removable for sdhc_1.
The monaco-evk-sd-card.dtso overlay adds no-mmc and cd-gpios, but doesn't
explicitly delete the base boolean properties using /delete-property/.
Because overlays don't overwrite or delete boolean properties without that
directive, the merged node would retain both no-sd and no-mmc, which could
completely disable media scanning.
> dtb-$(CONFIG_ARCH_QCOM) += monaco-evk-sd-card.dtb
>
> +monaco-evk-emmc-dtbs := monaco-evk.dtb monaco-evk-emmc.dtbo
> +dtb-$(CONFIG_ARCH_QCOM) += monaco-evk-emmc.dtb
> +
[ ... ]
> diff --git a/arch/arm64/boot/dts/qcom/monaco-evk-emmc.dtso b/arch/arm64/boot/dts/qcom/monaco-evk-emmc.dtso
> new file mode 100644
> index 0000000000000..cb2566ac6923c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/monaco-evk-emmc.dtso
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> + vreg_s2s: regulator-vreg-s2s {
[Severity: Medium]
Does this create a duplicate phantom node instead of merging with the intended
base regulator?
The base device tree monaco-evk.dts defines a fixed regulator node named
vreg-s2s. This overlay defines a new fixed regulator node named
regulator-vreg-s2s and assigns it the same label vreg_s2s.
Because the node names differ, the dtc overlay compiler won't merge them.
This could create a second phantom node at the root (/regulator-vreg-s2s).
Will local phandle references to <&vreg_s2s> in this overlay resolve to the
new phantom node instead of generating a fixup to the correct base board
regulator resource?
> + compatible = "regulator-fixed";
> + regulator-name = "regulator-s2s";
> +
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +};
> +
> +&sdhc_1 {
[Severity: Low]
Does this overlay duplicate properties that are already fully configured in
the base device tree?
The base board DT monaco-evk.dts already sets up sdhc_1 for eMMC with
properties such as no-sd, no-sdio, non-removable, vmmc-supply, and
vqmmc-supply.
Since this overlay redefines all of these exact properties, does it offer new
functionality beyond supports-cqe, or should the base board DT instead leave
the controller unconfigured for media types so overlays can correctly apply
the specific parameters?
> + vmmc-supply = <&vreg_l8a>;
> + vqmmc-supply = <&vreg_s2s>;
> +
> + supports-cqe;
> +
> + pinctrl-0 = <&sdc1_state_on>;
> + pinctrl-1 = <&sdc1_state_off>;
> +
> + pinctrl-names = "default", "sleep";
> +
> + non-removable;
> +
> + bus-width = <8>;
> + no-sd;
> + no-sdio;
> +
> + status = "okay";
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701081715.2580329-1-monish.chunara@oss.qualcomm.com?part=3
prev parent reply other threads:[~2026-07-01 8:29 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-01 8:17 [PATCH V3 0/3] arm64: dts: qcom: Monaco: Enable SDHCI storage support Monish Chunara
2026-07-01 8:17 ` [PATCH V3 1/3] arm64: dts: qcom: monaco: Move eMMC CQE support from SoC to board DT Monish Chunara
2026-07-01 8:28 ` sashiko-bot
2026-07-01 8:17 ` [PATCH V3 2/3] arm64: dts: qcom: monaco-evk: Enable SDHCI for SD Card via overlay Monish Chunara
2026-07-01 8:31 ` sashiko-bot
2026-07-01 8:17 ` [PATCH V3 3/3] arm64: dts: qcom: monaco-evk: Add SDHCI support for eMMC " Monish Chunara
2026-07-01 8:29 ` 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=20260701082916.A892A1F00A3A@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=monish.chunara@oss.qualcomm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.