Re: [PATCH 1/1] arm64: dts: msm: Add eMMC support for qcs8300

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/06/2025 11:21, Sayali Lokhande wrote:
> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi


Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

It is NEVER msm. Also missing soc/board prefix.

> index 7ada029c32c1..5dee0b913b88 100644
> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> @@ -3837,6 +3837,62 @@
>  			clock-names = "apb_pclk";
>  		};
>  
> +		sdhc_1: mmc@87C4000 {
> +			compatible = "qcom,sdhci-msm-v5";
> +			status = "disabled";

That's not correct place. Please follow DTS coding style.

> +
> +			reg = <0x0 0x87C4000 0x0 0x1000>,
> +				<0x0 0x87C5000 0x0 0x1000>;

Look at the rest of the file: lower or upper hex is used?

> +			reg-names = "hc", "cqhci";
> +
> +			interrupts = <GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>,
> +					<GIC_SPI 521 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "hc_irq", "pwr_irq";
> +
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +					<&gcc GCC_SDCC1_APPS_CLK>,
> +					<&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "iface", "core", "xo";
> +			interconnects = <&aggre1_noc MASTER_SDC 0 &mc_virt SLAVE_EBI1 0>,
> +					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_SDC1 0>;
> +			interconnect-names = "sdhc-ddr","cpu-sdhc";
> +
> +			operating-points-v2 = <&sdhc1_opp_table>;
> +			bus-width = <8>;
> +			supports-cqe;
> +			dma-coherent;
> +
> +			qcom,dll-config = <0x000F64EE>;
> +			qcom,ddr-config = <0x80040868>;
> +
> +			mmc-ddr-1_8v;
> +			mmc-hs200-1_8v;
> +			mmc-hs400-1_8v;
> +			mmc-hs400-enhanced-strobe;

All these do not look like SoC-level properties.

> +
> +			iommus = <&apps_smmu 0x0 0x0>;
> +
> +			resets = <&gcc GCC_SDCC1_BCR>;
> +
> +			sdhc1_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				opp-100000000 {
> +					opp-hz = /bits/ 64 <100000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <1800000 400000>;
> +					opp-avg-kBps = <100000 0>;
> +				};
> +
> +				opp-384000000 {
> +					opp-hz = /bits/ 64 <384000000>;
> +					required-opps = <&rpmhpd_opp_nom>;
> +					opp-peak-kBps = <5400000 1600000>;
> +					opp-avg-kBps = <390000 0>;
> +				};
> +			};
> +		};
> +
>  		usb_1_hsphy: phy@8904000 {
>  			compatible = "qcom,qcs8300-usb-hs-phy",
>  				     "qcom,usb-snps-hs-7nm-phy";
> @@ -5042,6 +5098,47 @@
>  				pins = "gpio13";
>  				function = "qup2_se0";
>  			};
> +
> +			sdc1_clk: sdc1-clk-state {
> +				pins = "sdc1_clk";
> +

Stray blank line

> +			};
> +
> +			sdc1_cmd: sdc1-cmd-state {
> +				pins = "sdc1_cmd";
> +			};
> +
> +			sdc1_data: sdc1-data-state {
> +				pins = "sdc1_data";
> +			};
> +
> +			sdc1_rclk: sdc1-rclk-state {
> +				pins = "sdc1_rclk";
> +			};

Anyway, what is the point of all above pin nodes without config or muxing?


Best regards,
Krzysztof