arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card

arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -0

index 3900cfc..8b159d1 100644
@@ -11,6 +11,7 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
 #include "pmr735a.dtsi"

arch/arm64/boot/dts/qcom/sc7280-idp.dts +6 -0

+
+	non-removable;
+	no-sd;
+	no-sdio;
+
+	vmmc-supply = <&vreg_l7b_2p9>;

arch/arm64/boot/dts/qcom/sc7280-idp.dts +32 -0

+&tlmm {
+	sdc1_clk: sdc1-clk {
+		pins = "sdc1_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc1_cmd: sdc1-cmd {
+		pins = "sdc1_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc1_data: sdc1-data {
+		pins = "sdc1_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+	sdc1_rclk: sdc1-rclk {
+		pins = "sdc1_rclk";
+		bias-pull-down;
+	};
+
+	sdc2_clk: sdc2-clk {
+		pins = "sdc2_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pins = "sdc2_cmd";
+		bias-pull-up;

Hi,

Quoted 16 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -0

index 3900cfc..8b159d1 100644
@@ -11,6 +11,7 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
 #include "pmr735a.dtsi"

@@ -272,6 +273,36 @@
 	status = "okay";
 };
 

arch/arm64/boot/dts/qcom/sc7280-idp.dts

Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +6 -0

+&sdhc_1 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data &sdc1_rclk>;
+	pinctrl-1 = <&sdc1_clk_sleep &sdc1_cmd_sleep &sdc1_data_sleep 
 &sdc1_rclk_sleep>;

Please condense these pins into a since sdc1_on_state/sdc1_off_state (check sdc1_state_on in [1])

Quoted 12 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +10 -0

+
+	non-removable;
+	no-sd;
+	no-sdio;
+
+	vmmc-supply = <&vreg_l7b_2p9>;
+	vqmmc-supply = <&vreg_l19b_1p8>;
+
+};
+

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 298

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +6 -0

+&sdhc_2 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd>;
+	pinctrl-1 = <&sdc2_clk_sleep &sdc2_cmd_sleep &sdc2_data_sleep>;

Ditto

Quoted 38 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +36 -0

+&tlmm {
+	sdc1_clk: sdc1-clk {
+		pins = "sdc1_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc1_cmd: sdc1-cmd {
+		pins = "sdc1_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc1_data: sdc1-data {
+		pins = "sdc1_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+	sdc1_rclk: sdc1-rclk {
+		pins = "sdc1_rclk";
+		bias-pull-down;
+	};
+
+	sdc2_clk: sdc2-clk {
+		pins = "sdc2_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pins = "sdc2_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc2_data: sdc2-data {

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 367

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +6 -0

+		pins = "sdc2_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sd_cd: sd-cd {

Please make it sdc2 to keep things coherent.

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 372

Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +5 -0

+		pins = "gpio91";
+		bias-pull-up;
+	};
+
+};

Why are you defining on_state pins in the device dt and sleep state in the SoC one?

Most devices share a common config for these, often coming from MTP or QRD boards

and it makes little to no sense to define these separately every time, because if you hit the

rare case of needing to make a change against that, it's probably just drive-strength.

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +0 -0

index 0b6f119..eab6f7b 100644
@@ -24,6 +24,11 @@
 

arch/arm64/boot/dts/qcom/sc7280.dtsi line 30

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +4 -0

 	chosen { };
 
+	aliases {
+		mmc1 = &sdhc_1;
+		mmc2 = &sdhc_2;
+	};

This is board specific. Something might have a SDIO Wi-Fi card on it.

arch/arm64/boot/dts/qcom/sc7280.dtsi line 470

Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +4 -0

+			mmc-ddr-1_8v;
+			mmc-hs200-1_8v;
+			mmc-hs400-1_8v;
+			mmc-hs400-enhanced-strobe;

These properties should probably be in the device DT, unless the SoC controller

can only support these speeds and only at 1.8v

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=35a4a8b6e9b133cf3a7d059ad4cf0e24cb4bd029

Konrad

Add nodes for eMMC and SD card on sc7280.

Signed-off-by Shaik Sajida Bhanu <[email protected]>

---

This change is depends on the below patch series:
https://lore.kernel.org/patchwork/cover/1418814/

Change since V2:
- Added leading zero's for register address and "qcom,sc7280-sdhci" string in compatible as suggested by Stephen Boyd and Doug.
- Removed max-frequency flag, no-mmc and no-sdio flags for Sd card as suggested by Doug and Stephen Boyd.
- Moved non-removable, no-sd, no-sdio and some pin config changes from soc to board dts file as suggested by Doug.
- Removed sleep state for CD line and drive-strength for input pins as suggested by Doug.
- Updated bus vote numbers for eMMC and SD card.

Changes since V1:
- Moved SDHC nodes as suggested by Bjorn Andersson.
- Dropped "pinconf-" prefix as suggested by Bjorn Andersson.
- Removed extra newlines as suggested by Konrad Dybcio.
- Changed sd-cd pin to bias-pull-up in sdc2_off as suggested by Veerabhadrarao Badiganti.
- Added bandwidth votes for eMMC and SD card.

---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts |  79 +++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi    | 146 ++++++++++++++++++++++++++++++++
 2 files changed, 225 insertions(+)

arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -0

index 3900cfc..8b159d1 100644
@@ -11,6 +11,7 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
 #include "pmr735a.dtsi"

arch/arm64/boot/dts/qcom/sc7280-idp.dts +51 -0

+};
+
+&sdhc_2 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd>;
+	pinctrl-1 = <&sdc2_clk_sleep &sdc2_cmd_sleep &sdc2_data_sleep>;
+
+	vmmc-supply = <&vreg_l9c_2p9>;
+	vqmmc-supply = <&vreg_l6c_2p9>;
+
+	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
+};
+
 &uart5 {
 	status = "okay";
 };

@@ -291,3 +322,51 @@
 		bias-pull-up;
 	};
 };
+
+&tlmm {
+	sdc1_clk: sdc1-clk {
+		pins = "sdc1_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc1_cmd: sdc1-cmd {
+		pins = "sdc1_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc1_data: sdc1-data {
+		pins = "sdc1_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+	sdc1_rclk: sdc1-rclk {
+		pins = "sdc1_rclk";
+		bias-pull-down;
+	};
+
+	sdc2_clk: sdc2-clk {
+		pins = "sdc2_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pins = "sdc2_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+

arch/arm64/boot/dts/qcom/sc7280-idp.dts +2 -0

+
+};

arch/arm64/boot/dts/qcom/sc7280.dtsi +5 -0

index 0b6f119..eab6f7b 100644
@@ -24,6 +24,11 @@
 
 	chosen { };
 
+	aliases {
+		mmc1 = &sdhc_1;
+		mmc2 = &sdhc_2;
+	};
+
 	clocks {
 		xo_board: xo-board {
 			compatible = "fixed-clock";

arch/arm64/boot/dts/qcom/sc7280.dtsi +19 -0

+			reg-names = "hc", "cqhci";
+
+			iommus = <&apps_smmu 0xc0 0x0>;
+			interrupts = <GIC_SPI 652 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 656 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_1 0 &mc_virt SLAVE_EBI1 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_1 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc1_opp_table>;
+
+			bus-width = <8>;
+			supports-cqe;

arch/arm64/boot/dts/qcom/sc7280.dtsi +95 -0

+				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>;
+				};
+			};
+
+		};
+
 		qupv3_id_0: geniqup@9c0000 {
 			compatible = "qcom,geni-se-qup";
 			reg = <0 0x009c0000 0 0x2000>;

@@ -973,6 +1032,51 @@
 			};
 		};
 
+		sdhc_2: sdhci@8804000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x08804000 0 0x1000>;
+
+			iommus = <&apps_smmu 0x100 0x0>;
+			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_2 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc2_opp_table>;
+
+			bus-width = <4>;
+
+			qcom,dll-config = <0x0007642c>;
+
+			sdhc2_opp_table: sdhc2-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-202000000 {
+					opp-hz = /bits/ 64 <202000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <5400000 1600000>;
+					opp-avg-kBps = <200000 0>;
+				};
+			};
+
+		};
+
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7280-llcc";
 			reg = <0 0x09200000 0 0xd0000>, <0 0x09600000 0 0x50000>;

@@ -1102,6 +1206,48 @@
 				pins = "gpio46", "gpio47";
 				function = "qup13";
 			};
+
+			sdc1_clk_sleep: sdc1-clk-sleep {
+				pins = "sdc1_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_cmd_sleep: sdc1-cmd-sleep {
+				pins = "sdc1_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_data_sleep: sdc1-data-sleep {
+				pins = "sdc1_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_rclk_sleep: sdc1-rclk-sleep {
+				pins = "sdc1_rclk";
+				bias-bus-hold;
+			};
+
+			sdc2_clk_sleep: sdc2-clk-sleep {
+				pins = "sdc2_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc2_cmd_sleep: sdc2-cmd-sleep{

arch/arm64/boot/dts/qcom/sc7280.dtsi +0 -0

 		};
 
 		apps_smmu: iommu@15000000 {

Signature · 2 lines

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation

Quoted 50 lines · Shaik Sajida Bhanu

Add nodes for eMMC and SD card on sc7280.

Signed-off-by Shaik Sajida Bhanu <[email protected]>

---

This change is depends on the below patch series:
https://lore.kernel.org/patchwork/cover/1418814/

Change since V2:
- Added leading zero's for register address and "qcom,sc7280-sdhci" string in compatible as suggested by Stephen Boyd and Doug.
- Removed max-frequency flag, no-mmc and no-sdio flags for Sd card as suggested by Doug and Stephen Boyd.
- Moved non-removable, no-sd, no-sdio and some pin config changes from soc to board dts file as suggested by Doug.
- Removed sleep state for CD line and drive-strength for input pins as suggested by Doug.
- Updated bus vote numbers for eMMC and SD card.

Changes since V1:
- Moved SDHC nodes as suggested by Bjorn Andersson.
- Dropped "pinconf-" prefix as suggested by Bjorn Andersson.
- Removed extra newlines as suggested by Konrad Dybcio.
- Changed sd-cd pin to bias-pull-up in sdc2_off as suggested by Veerabhadrarao Badiganti.
- Added bandwidth votes for eMMC and SD card.

---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts |  79 +++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi    | 146 
++++++++++++++++++++++++++++++++
 2 files changed, 225 insertions(+)

arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -0

index 3900cfc..8b159d1 100644
@@ -11,6 +11,7 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
 #include "pmr735a.dtsi"

@@ -272,6 +273,36 @@
 	status = "okay";
 };
 

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 282

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +7 -0

+&sdhc_1 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data &sdc1_rclk>;
+	pinctrl-1 = <&sdc1_clk_sleep &sdc1_cmd_sleep &sdc1_data_sleep 
 &sdc1_rclk_sleep>;
+

Extra newline.

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 290

Quoted 9 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +8 -0

+
+	non-removable;
+	no-sd;
+	no-sdio;
+
+	vmmc-supply = <&vreg_l7b_2p9>;
+	vqmmc-supply = <&vreg_l19b_1p8>;
+

Extra newline.

Quoted 59 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +15 -0

+};
+
+&sdhc_2 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd>;
+	pinctrl-1 = <&sdc2_clk_sleep &sdc2_cmd_sleep &sdc2_data_sleep>;
+
+	vmmc-supply = <&vreg_l9c_2p9>;
+	vqmmc-supply = <&vreg_l6c_2p9>;
+
+	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
+};
+
 &uart5 {
 	status = "okay";

};

arch/arm64/boot/dts/qcom/sc7280-idp.dts +36 -0

@@ -291,3 +322,51 @@
 		bias-pull-up;
 	};
 };
+
+&tlmm {
+	sdc1_clk: sdc1-clk {
+		pins = "sdc1_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc1_cmd: sdc1-cmd {
+		pins = "sdc1_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc1_data: sdc1-data {
+		pins = "sdc1_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+	sdc1_rclk: sdc1-rclk {
+		pins = "sdc1_rclk";
+		bias-pull-down;
+	};
+
+	sdc2_clk: sdc2-clk {
+		pins = "sdc2_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pins = "sdc2_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 370

Quoted 10 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +10 -0

+	sdc2_data: sdc2-data {
+		pins = "sdc2_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sd_cd: sd-cd {
+		pins = "gpio91";
+		bias-pull-up;
+	};

As Konrad pointed out, this is much cleaner described as:

	sdc1_default_state: sdc1-default-state {
		clk {
		};

		cmd {

		};

		data {

		};

		rclk {

		};
	};

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 372

Quoted 2 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +2 -0

+
+};

Quoted 20 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +5 -0

index 0b6f119..eab6f7b 100644
@@ -24,6 +24,11 @@
 
 	chosen { };
 
+	aliases {
+		mmc1 = &sdhc_1;
+		mmc2 = &sdhc_2;
+	};
+
 	clocks {
 		xo_board: xo-board {
 			compatible = "fixed-clock";

@@ -430,6 +435,60 @@
 			#mbox-cells = <2>;
 		};
 

arch/arm64/boot/dts/qcom/sc7280.dtsi line 443

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+		sdhc_1: sdhci@7c4000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x007c4000 0 0x1000>,
+					<0 0x007c5000 0 0x1000>;

Whenever you break lines, align your '<' on each line.

Quoted 25 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +23 -0

+			reg-names = "hc", "cqhci";
+
+			iommus = <&apps_smmu 0xc0 0x0>;
+			interrupts = <GIC_SPI 652 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 656 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_1 0 &mc_virt SLAVE_EBI1 
 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_1 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc1_opp_table>;
+
+			bus-width = <8>;
+			supports-cqe;
+
+			qcom,dll-config = <0x0007642c>;
+			qcom,ddr-config = <0x80040868>;
+

arch/arm64/boot/dts/qcom/sc7280.dtsi line 472

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+			mmc-ddr-1_8v;
+			mmc-hs200-1_8v;
+			mmc-hs400-1_8v;
+			mmc-hs400-enhanced-strobe;
+
+			sdhc1_opp_table: sdhc1-opp-table {

No need for "sdhc1-" in the node name.

Quoted 115 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +19 -0

+				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>;
+				};
+			};
+
+		};
+
 		qupv3_id_0: geniqup@9c0000 {
 			compatible = "qcom,geni-se-qup";

arch/arm64/boot/dts/qcom/sc7280.dtsi +45 -0

 			reg = <0 0x009c0000 0 0x2000>;

@@ -973,6 +1032,51 @@
 			};
 		};
 
+		sdhc_2: sdhci@8804000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x08804000 0 0x1000>;
+
+			iommus = <&apps_smmu 0x100 0x0>;
+			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 
 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_2 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc2_opp_table>;
+
+			bus-width = <4>;
+
+			qcom,dll-config = <0x0007642c>;
+
+			sdhc2_opp_table: sdhc2-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-202000000 {
+					opp-hz = /bits/ 64 <202000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <5400000 1600000>;
+					opp-avg-kBps = <200000 0>;
+				};
+			};
+
+		};
+
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7280-llcc";

arch/arm64/boot/dts/qcom/sc7280.dtsi +35 -0

 			reg = <0 0x09200000 0 0xd0000>, <0 0x09600000 0 0x50000>;

@@ -1102,6 +1206,48 @@
 				pins = "gpio46", "gpio47";
 				function = "qup13";
 			};
+
+			sdc1_clk_sleep: sdc1-clk-sleep {
+				pins = "sdc1_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_cmd_sleep: sdc1-cmd-sleep {
+				pins = "sdc1_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_data_sleep: sdc1-data-sleep {
+				pins = "sdc1_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_rclk_sleep: sdc1-rclk-sleep {
+				pins = "sdc1_rclk";
+				bias-bus-hold;
+			};
+
+			sdc2_clk_sleep: sdc2-clk-sleep {
+				pins = "sdc2_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc2_cmd_sleep: sdc2-cmd-sleep{
+				pins ="sdc2_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};

arch/arm64/boot/dts/qcom/sc7280.dtsi line 1249

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+
+			sdc2_data_sleep: sdc2-data-sleep {
+				pins ="sdc2_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};

Please structure these as suggested above.

arch/arm64/boot/dts/qcom/sc7280.dtsi line 1250

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+

And please drop this empty line.

Thanks,
Bjorn

Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +0 -1

 		};
 
 		apps_smmu: iommu@15000000 {
--

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Quoted 52 lines · Bjorn Andersson

Quoted 50 lines · Shaik Sajida Bhanu

Add nodes for eMMC and SD card on sc7280.

Signed-off-by Shaik Sajida Bhanu <[email protected]>

---

This change is depends on the below patch series:
https://lore.kernel.org/patchwork/cover/1418814/

Change since V2:
- Added leading zero's for register address and "qcom,sc7280-sdhci" string in compatible as suggested by Stephen Boyd and Doug.
- Removed max-frequency flag, no-mmc and no-sdio flags for Sd card as suggested by Doug and Stephen Boyd.
- Moved non-removable, no-sd, no-sdio and some pin config changes from soc to board dts file as suggested by Doug.
- Removed sleep state for CD line and drive-strength for input pins as suggested by Doug.
- Updated bus vote numbers for eMMC and SD card.

Changes since V1:
- Moved SDHC nodes as suggested by Bjorn Andersson.
- Dropped "pinconf-" prefix as suggested by Bjorn Andersson.
- Removed extra newlines as suggested by Konrad Dybcio.
- Changed sd-cd pin to bias-pull-up in sdc2_off as suggested by Veerabhadrarao Badiganti.
- Added bandwidth votes for eMMC and SD card.

---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts |  79 +++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280.dtsi    | 146 
++++++++++++++++++++++++++++++++
 2 files changed, 225 insertions(+)

arch/arm64/boot/dts/qcom/sc7280-idp.dts +1 -0

index 3900cfc..8b159d1 100644
@@ -11,6 +11,7 @@
 #include <dt-bindings/iio/qcom,spmi-adc7-pmr735b.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
 #include <dt-bindings/iio/qcom,spmi-adc7-pmk8350.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "sc7280.dtsi"
 #include "pm7325.dtsi"
 #include "pmr735a.dtsi"

@@ -272,6 +273,36 @@
 	status = "okay";
 };
 

Quoted 10 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 282

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +7 -0

+&sdhc_1 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc1_clk &sdc1_cmd &sdc1_data &sdc1_rclk>;
+	pinctrl-1 = <&sdc1_clk_sleep &sdc1_cmd_sleep &sdc1_data_sleep 
 &sdc1_rclk_sleep>;
+

Extra newline.

Sure

Quoted 11 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 290

Quoted 9 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +8 -0

+
+	non-removable;
+	no-sd;
+	no-sdio;
+
+	vmmc-supply = <&vreg_l7b_2p9>;
+	vqmmc-supply = <&vreg_l19b_1p8>;
+

Extra newline.

Sure

Quoted 89 lines

Quoted 59 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +15 -0

+};
+
+&sdhc_2 {
+	status = "okay";
+
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&sdc2_clk &sdc2_cmd &sdc2_data &sd_cd>;
+	pinctrl-1 = <&sdc2_clk_sleep &sdc2_cmd_sleep &sdc2_data_sleep>;
+
+	vmmc-supply = <&vreg_l9c_2p9>;
+	vqmmc-supply = <&vreg_l6c_2p9>;
+
+	cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
+};
+
 &uart5 {
 	status = "okay";

};

arch/arm64/boot/dts/qcom/sc7280-idp.dts +36 -0

@@ -291,3 +322,51 @@
 		bias-pull-up;
 	};
 };
+
+&tlmm {
+	sdc1_clk: sdc1-clk {
+		pins = "sdc1_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc1_cmd: sdc1-cmd {
+		pins = "sdc1_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sdc1_data: sdc1-data {
+		pins = "sdc1_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+	sdc1_rclk: sdc1-rclk {
+		pins = "sdc1_rclk";
+		bias-pull-down;
+	};
+
+	sdc2_clk: sdc2-clk {
+		pins = "sdc2_clk";
+		bias-disable;
+		drive-strength = <16>;
+	};
+
+	sdc2_cmd: sdc2-cmd {
+		pins = "sdc2_cmd";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 370

Quoted 10 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +10 -0

+	sdc2_data: sdc2-data {
+		pins = "sdc2_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sd_cd: sd-cd {
+		pins = "gpio91";
+		bias-pull-up;
+	};

As Konrad pointed out, this is much cleaner described as:

	sdc1_default_state: sdc1-default-state {
		clk {
		};

		cmd {

		};

		data {

		};

		rclk {

		};
	};

Quoted 2 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts line 372

Quoted 2 lines

arch/arm64/boot/dts/qcom/sc7280-idp.dts +2 -0

+
+};

Sure will change this in next version

Quoted 20 lines

Quoted 20 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +5 -0

index 0b6f119..eab6f7b 100644
@@ -24,6 +24,11 @@
 
 	chosen { };
 
+	aliases {
+		mmc1 = &sdhc_1;
+		mmc2 = &sdhc_2;
+	};
+
 	clocks {
 		xo_board: xo-board {
 			compatible = "fixed-clock";

@@ -430,6 +435,60 @@
 			#mbox-cells = <2>;
 		};
 

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 443

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+		sdhc_1: sdhci@7c4000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x007c4000 0 0x1000>,
+					<0 0x007c5000 0 0x1000>;

Whenever you break lines, align your '<' on each line.

ok

Quoted 25 lines

Quoted 25 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +23 -0

+			reg-names = "hc", "cqhci";
+
+			iommus = <&apps_smmu 0xc0 0x0>;
+			interrupts = <GIC_SPI 652 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 656 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC1_APPS_CLK>,
+					<&gcc GCC_SDCC1_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_1 0 &mc_virt SLAVE_EBI1 
 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_1 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc1_opp_table>;
+
+			bus-width = <8>;
+			supports-cqe;
+
+			qcom,dll-config = <0x0007642c>;
+			qcom,ddr-config = <0x80040868>;
+

Quoted 8 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 472

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+			mmc-ddr-1_8v;
+			mmc-hs200-1_8v;
+			mmc-hs400-1_8v;
+			mmc-hs400-enhanced-strobe;
+
+			sdhc1_opp_table: sdhc1-opp-table {

No need for "sdhc1-" in the node name.

ok

Quoted 115 lines

Quoted 115 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +19 -0

+				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>;
+				};
+			};
+
+		};
+
 		qupv3_id_0: geniqup@9c0000 {
 			compatible = "qcom,geni-se-qup";

arch/arm64/boot/dts/qcom/sc7280.dtsi +45 -0

 			reg = <0 0x009c0000 0 0x2000>;

@@ -973,6 +1032,51 @@
 			};
 		};
 
+		sdhc_2: sdhci@8804000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x08804000 0 0x1000>;
+
+			iommus = <&apps_smmu 0x100 0x0>;
+			interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC2_APPS_CLK>,
+					<&gcc GCC_SDCC2_AHB_CLK>,
+					<&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "core", "iface", "xo";
+			interconnects = <&aggre1_noc MASTER_SDCC_2 0 &mc_virt SLAVE_EBI1 
 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &cnoc2 SLAVE_SDCC_2 0>;
+			interconnect-names = "sdhc-ddr","cpu-sdhc";
+			power-domains = <&rpmhpd SC7280_CX>;
+			operating-points-v2 = <&sdhc2_opp_table>;
+
+			bus-width = <4>;
+
+			qcom,dll-config = <0x0007642c>;
+
+			sdhc2_opp_table: sdhc2-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-202000000 {
+					opp-hz = /bits/ 64 <202000000>;
+					required-opps = <&rpmhpd_opp_nom>;
+					opp-peak-kBps = <5400000 1600000>;
+					opp-avg-kBps = <200000 0>;
+				};
+			};
+
+		};
+
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc7280-llcc";

arch/arm64/boot/dts/qcom/sc7280.dtsi +35 -0

 			reg = <0 0x09200000 0 0xd0000>, <0 0x09600000 0 0x50000>;

@@ -1102,6 +1206,48 @@
 				pins = "gpio46", "gpio47";
 				function = "qup13";
 			};
+
+			sdc1_clk_sleep: sdc1-clk-sleep {
+				pins = "sdc1_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_cmd_sleep: sdc1-cmd-sleep {
+				pins = "sdc1_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_data_sleep: sdc1-data-sleep {
+				pins = "sdc1_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc1_rclk_sleep: sdc1-rclk-sleep {
+				pins = "sdc1_rclk";
+				bias-bus-hold;
+			};
+
+			sdc2_clk_sleep: sdc2-clk-sleep {
+				pins = "sdc2_clk";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc2_cmd_sleep: sdc2-cmd-sleep{
+				pins ="sdc2_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};

Quoted 9 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 1249

Quoted 6 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +6 -0

+
+			sdc2_data_sleep: sdc2-data-sleep {
+				pins ="sdc2_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};

Please structure these as suggested above.

Sure

Quoted 4 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 1250

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+

And please drop this empty line.

Sure

Quoted 10 lines

Thanks,
Bjorn

Quoted 7 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi +0 -1

 		};
 
 		apps_smmu: iommu@15000000 {
--

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Quoted 2 lines · On 2021-06-14 17:00, [email protected] wrote:

Quoted 1 line · Bjorn Andersson

On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:

Quoted 1 line

Quoted 1 line

Quoted 1 line

(quoted diff) +0 -0

Quoted 1 line

Quoted 1 line

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+		sdhc_1: sdhci@7c4000 {

Quoted 4 lines

Quoted 3 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 472

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+			sdhc1_opp_table: sdhc1-opp-table {

No need for "sdhc1-" in the node name.

ok

Hi,

For Sd card also we have opp-table info so if we remove "sdhc1-" here and "sdhc2-" in Sd crad opp table we may have dublicate nodes so, it is better to keep sdhc1 and sdhc2 in node numbers right.

Quoted 3 lines · On Tue 15 Jun 03:56 CDT 2021, [email protected] wrote:

Quoted 2 lines · On 2021-06-14 17:00, [email protected] wrote:

Quoted 1 line · Bjorn Andersson

On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:

[..]

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

(quoted diff) +0 -0

[..]

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+		sdhc_1: sdhci@7c4000 {

[..]

Quoted 10 lines

Quoted 4 lines

Quoted 3 lines

arch/arm64/boot/dts/qcom/sc7280.dtsi line 472

Quoted 1 line

arch/arm64/boot/dts/qcom/sc7280.dtsi +1 -0

+			sdhc1_opp_table: sdhc1-opp-table {

No need for "sdhc1-" in the node name.

ok

Hi,

For Sd card also we have opp-table info so if we remove "sdhc1-" here and "sdhc2-" in Sd crad opp table we may have dublicate nodes so, it is better to keep sdhc1 and sdhc2 in node numbers right.

Are you saying that /soc/sdhci@7c4000/opp-table needs to have a unique name to not collide with /soc/sdhci@8804000/opp-table?

Regards,
Bjorn