[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <61e77351-f82c-4450-88f1-cd074423a840@kernel.org>
Date: Thu, 12 Jun 2025 12:02:22 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Sayali Lokhande <quic_sayalil@...cinc.com>, andersson@...nel.org,
konradybcio@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mmc-owner@...r.kernel.org
Subject: Re: [PATCH 1/1] arm64: dts: msm: Add eMMC support for qcs8300
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@...4000 {
> + 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@...4000 {
> 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
Powered by blists - more mailing lists