Re: [PATCH V2 3/3] arm64: dts: qcom: monaco-evk: Add SDHCI support for eMMC via overlay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Device Tree resource duplication for the vreg_s2s regulator.

Pre-existing issues:
- [High] Incomplete base DT refactoring leaves mutually exclusive eMMC properties, breaking the SD card overlay.
--

commit 457596ee7fdadc67892c938f430cbd6d76fabaf4
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/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 {
> +                compatible = "regulator-fixed";
> +                regulator-name = "regulator-s2s";
> +
> +                regulator-min-microvolt = <1800000>;
> +                regulator-max-microvolt = <1800000>;
> +        };
> +};

[Severity: Medium]
Does this create a duplicate regulator node?

The base device tree in arch/arm64/boot/dts/qcom/monaco-evk.dts already
provisions this exact 1.8V fixed regulator as vreg-s2s with the vreg_s2s
label:

arch/arm64/boot/dts/qcom/monaco-evk.dts:
	vreg_s2s: vreg-s2s {
		compatible = "regulator-fixed";
		regulator-name = "vreg_s2s";

Since the node names differ, applying this overlay instantiates a duplicate
physical regulator node instead of referencing the existing one.

> +
> +&sdhc_1 {
> +	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";
> +};

[Severity: High]
This isn't a bug introduced by this patch, but this commit fails to address
it while supposedly completing the eMMC overlay separation.

The base device tree arch/arm64/boot/dts/qcom/monaco-evk.dts retains
hardcoded eMMC properties for sdhc_1:

arch/arm64/boot/dts/qcom/monaco-evk.dts:sdhc_1
	no-sd;
	no-sdio;
	non-removable;

Because overlays cannot easily delete properties, applying the previously
merged SD card overlay arch/arm64/boot/dts/qcom/monaco-evk-sd-card.dtso
inherits no-sd from the base DT while appending its own no-mmc. This causes
the MMC core to skip probing entirely, rendering the SD card controller
non-functional.

Should the base DT be stripped of these media-specific properties to properly
implement mutually exclusive overlays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616130347.3096034-1-monish.chunara@oss.qualcomm.com?part=3