linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
@ 2021-06-09 15:20 Shaik Sajida Bhanu
  2021-06-09 20:45 ` Konrad Dybcio
  2021-06-11  3:42 ` Bjorn Andersson
  0 siblings, 2 replies; 8+ messages in thread
From: Shaik Sajida Bhanu @ 2021-06-09 15:20 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, robh+dt
  Cc: asutoshd, stummala, vbadigan, rampraka, sayalil, sartgarg,
	rnayak, saiprakash.ranjan, sibis, okukatla, djakov, cang,
	pragalla, nitirawa, linux-mmc, linux-kernel, linux-arm-msm,
	devicetree, agross, bjorn.andersson, Shaik Sajida Bhanu

Add nodes for eMMC and SD card on sc7280.

Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
---

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(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 3900cfc..8b159d1 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -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";
 };
 
+&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>;
+
+
+	non-removable;
+	no-sd;
+	no-sdio;
+
+	vmmc-supply = <&vreg_l7b_2p9>;
+	vqmmc-supply = <&vreg_l19b_1p8>;
+
+};
+
+&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>;
+	};
+
+	sdc2_data: sdc2-data {
+		pins = "sdc2_data";
+		bias-pull-up;
+		drive-strength = <10>;
+	};
+
+	sd_cd: sd-cd {
+		pins = "gpio91";
+		bias-pull-up;
+	};
+
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 0b6f119..eab6f7b 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -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>;
 		};
 
+		sdhc_1: sdhci@7c4000 {
+			compatible = "qcom,sc7280-sdhci", "qcom,sdhci-msm-v5";
+			status = "disabled";
+
+			reg = <0 0x007c4000 0 0x1000>,
+					<0 0x007c5000 0 0x1000>;
+			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>;
+
+			mmc-ddr-1_8v;
+			mmc-hs200-1_8v;
+			mmc-hs400-1_8v;
+			mmc-hs400-enhanced-strobe;
+
+			sdhc1_opp_table: sdhc1-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>;
+				};
+			};
+
+		};
+
 		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{
+				pins ="sdc2_cmd";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
+			sdc2_data_sleep: sdc2-data-sleep {
+				pins ="sdc2_data";
+				drive-strength = <2>;
+				bias-bus-hold;
+			};
+
 		};
 
 		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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-09 15:20 [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card Shaik Sajida Bhanu
@ 2021-06-09 20:45 ` Konrad Dybcio
  2021-06-14 11:25   ` sbhanu
  2021-06-11  3:42 ` Bjorn Andersson
  1 sibling, 1 reply; 8+ messages in thread
From: Konrad Dybcio @ 2021-06-09 20:45 UTC (permalink / raw)
  To: Shaik Sajida Bhanu, adrian.hunter, ulf.hansson, robh+dt
  Cc: asutoshd, stummala, vbadigan, rampraka, sayalil, sartgarg,
	rnayak, saiprakash.ranjan, sibis, okukatla, djakov, cang,
	pragalla, nitirawa, linux-mmc, linux-kernel, linux-arm-msm,
	devicetree, agross, bjorn.andersson

Hi,


> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index 3900cfc..8b159d1 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -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";
>  };
>  
> +&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])



> +
> +	non-removable;
> +	no-sd;
> +	no-sdio;
> +
> +	vmmc-supply = <&vreg_l7b_2p9>;
> +	vqmmc-supply = <&vreg_l19b_1p8>;
> +
> +};
> +
> +&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



> +&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 {
> +		pins = "sdc2_data";
> +		bias-pull-up;
> +		drive-strength = <10>;
> +	};
> +
> +	sd_cd: sd-cd {

Please make it sdc2 to keep things coherent.



> +		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.



> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 0b6f119..eab6f7b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -24,6 +24,11 @@
>  
>  	chosen { };
>  
> +	aliases {
> +		mmc1 = &sdhc_1;
> +		mmc2 = &sdhc_2;
> +	};

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



> +			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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-09 15:20 [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card Shaik Sajida Bhanu
  2021-06-09 20:45 ` Konrad Dybcio
@ 2021-06-11  3:42 ` Bjorn Andersson
  2021-06-14 11:30   ` sbhanu
  1 sibling, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2021-06-11  3:42 UTC (permalink / raw)
  To: Shaik Sajida Bhanu
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross

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

> Add nodes for eMMC and SD card on sc7280.
> 
> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
> ---
> 
> 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(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> index 3900cfc..8b159d1 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
> @@ -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";
>  };
>  
> +&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.

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

Extra newline.

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

		};
	};

> +
> +};
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 0b6f119..eab6f7b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -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>;
>  		};
>  
> +		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.

> +			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>;
> +
> +			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.

> +				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{
> +				pins ="sdc2_cmd";
> +				drive-strength = <2>;
> +				bias-bus-hold;
> +			};
> +
> +			sdc2_data_sleep: sdc2-data-sleep {
> +				pins ="sdc2_data";
> +				drive-strength = <2>;
> +				bias-bus-hold;
> +			};

Please structure these as suggested above.

> +

And please drop this empty line.

Thanks,
Bjorn

>  		};
>  
>  		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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-09 20:45 ` Konrad Dybcio
@ 2021-06-14 11:25   ` sbhanu
  0 siblings, 0 replies; 8+ messages in thread
From: sbhanu @ 2021-06-14 11:25 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross, bjorn.andersson

On 2021-06-10 02:15, Konrad Dybcio wrote:
> Hi,
> 
> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> index 3900cfc..8b159d1 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> @@ -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";
>>  };
>> 
>> +&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])
> 
Sure
> 
> 
>> +
>> +	non-removable;
>> +	no-sd;
>> +	no-sdio;
>> +
>> +	vmmc-supply = <&vreg_l7b_2p9>;
>> +	vqmmc-supply = <&vreg_l19b_1p8>;
>> +
>> +};
>> +
>> +&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
> 
Sure
> 
> 
>> +&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 {
>> +		pins = "sdc2_data";
>> +		bias-pull-up;
>> +		drive-strength = <10>;
>> +	};
>> +
>> +	sd_cd: sd-cd {
> 
> Please make it sdc2 to keep things coherent.
> 
Sure
> 
> 
>> +		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.
> 
I have made this change as per Doug Anderson comment on patch V2 
(https://lore.kernel.org/patchwork/patch/1399453/#1598871)
And along with drive-strength change, on some of boards Sd card gpio pin 
also may change right.
> 
> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 0b6f119..eab6f7b 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -24,6 +24,11 @@
>> 
>>  	chosen { };
>> 
>> +	aliases {
>> +		mmc1 = &sdhc_1;
>> +		mmc2 = &sdhc_2;
>> +	};
> 
> This is board specific. Something might have a SDIO Wi-Fi card on it.
we are assuming eMMC and SD support available on all boards and if any 
board supports SDIO Wi-Fi card
they can add aliases for SDIO Wi-Fi card in that particular board device 
DT file.
> 
> 
> 
>> +			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
> 
we are keeping these flags in SOC file because all platforms would 
support these modes,
and yes we support only these and at 1.8V.
> 
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git/commit/?h=for-next&id=35a4a8b6e9b133cf3a7d059ad4cf0e24cb4bd029
> 
> 
> Konrad

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-11  3:42 ` Bjorn Andersson
@ 2021-06-14 11:30   ` sbhanu
  2021-06-15  8:56     ` sbhanu
  0 siblings, 1 reply; 8+ messages in thread
From: sbhanu @ 2021-06-14 11:30 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross

On 2021-06-11 09:12, Bjorn Andersson wrote:
> On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:
> 
>> Add nodes for eMMC and SD card on sc7280.
>> 
>> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
>> ---
>> 
>> 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(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts 
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> index 3900cfc..8b159d1 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> @@ -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";
>>  };
>> 
>> +&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
> 
>> +
>> +	non-removable;
>> +	no-sd;
>> +	no-sdio;
>> +
>> +	vmmc-supply = <&vreg_l7b_2p9>;
>> +	vqmmc-supply = <&vreg_l19b_1p8>;
>> +
> 
> Extra newline.
Sure
> 
>> +};
>> +
>> +&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>;
>> +	};
>> +
>> +	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 {
> 
> 		};
> 	};
> 
>> +
>> +};
Sure will change this in next version
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> index 0b6f119..eab6f7b 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>> @@ -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>;
>>  		};
>> 
>> +		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
> 
>> +			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>;
>> +
>> +			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
> 
>> +				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{
>> +				pins ="sdc2_cmd";
>> +				drive-strength = <2>;
>> +				bias-bus-hold;
>> +			};
>> +
>> +			sdc2_data_sleep: sdc2-data-sleep {
>> +				pins ="sdc2_data";
>> +				drive-strength = <2>;
>> +				bias-bus-hold;
>> +			};
> 
> Please structure these as suggested above.
> 
Sure
>> +
> 
> And please drop this empty line.
> 
Sure
> Thanks,
> Bjorn
> 
>>  		};
>> 
>>  		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
>> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-14 11:30   ` sbhanu
@ 2021-06-15  8:56     ` sbhanu
  2021-06-16  0:08       ` Bjorn Andersson
  0 siblings, 1 reply; 8+ messages in thread
From: sbhanu @ 2021-06-15  8:56 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross

On 2021-06-14 17:00, sbhanu@codeaurora.org wrote:
> On 2021-06-11 09:12, Bjorn Andersson wrote:
>> On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:
>> 
>>> Add nodes for eMMC and SD card on sc7280.
>>> 
>>> Signed-off-by: Shaik Sajida Bhanu <sbhanu@codeaurora.org>
>>> ---
>>> 
>>> 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(+)
>>> 
>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts 
>>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>>> index 3900cfc..8b159d1 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>>> @@ -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";
>>>  };
>>> 
>>> +&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
>> 
>>> +
>>> +	non-removable;
>>> +	no-sd;
>>> +	no-sdio;
>>> +
>>> +	vmmc-supply = <&vreg_l7b_2p9>;
>>> +	vqmmc-supply = <&vreg_l19b_1p8>;
>>> +
>> 
>> Extra newline.
> Sure
>> 
>>> +};
>>> +
>>> +&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>;
>>> +	};
>>> +
>>> +	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 {
>> 
>> 		};
>> 	};
>> 
>>> +
>>> +};
> Sure will change this in next version
>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi 
>>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> index 0b6f119..eab6f7b 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
>>> @@ -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>;
>>>  		};
>>> 
>>> +		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
>> 
>>> +			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>;
>>> +
>>> +			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
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.

Thanks,
Sajida
>> 
>>> +				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{
>>> +				pins ="sdc2_cmd";
>>> +				drive-strength = <2>;
>>> +				bias-bus-hold;
>>> +			};
>>> +
>>> +			sdc2_data_sleep: sdc2-data-sleep {
>>> +				pins ="sdc2_data";
>>> +				drive-strength = <2>;
>>> +				bias-bus-hold;
>>> +			};
>> 
>> Please structure these as suggested above.
>> 
> Sure
>>> +
>> 
>> And please drop this empty line.
>> 
> Sure
>> Thanks,
>> Bjorn
>> 
>>>  		};
>>> 
>>>  		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
>>> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-15  8:56     ` sbhanu
@ 2021-06-16  0:08       ` Bjorn Andersson
  2021-06-16  5:54         ` sbhanu
  0 siblings, 1 reply; 8+ messages in thread
From: Bjorn Andersson @ 2021-06-16  0:08 UTC (permalink / raw)
  To: sbhanu
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross

On Tue 15 Jun 03:56 CDT 2021, sbhanu@codeaurora.org wrote:

> On 2021-06-14 17:00, sbhanu@codeaurora.org wrote:
> > On 2021-06-11 09:12, Bjorn Andersson wrote:
> > > On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:
[..]
> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
[..]
> > > > +		sdhc_1: sdhci@7c4000 {
[..]
> > > > +			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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card
  2021-06-16  0:08       ` Bjorn Andersson
@ 2021-06-16  5:54         ` sbhanu
  0 siblings, 0 replies; 8+ messages in thread
From: sbhanu @ 2021-06-16  5:54 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: adrian.hunter, ulf.hansson, robh+dt, asutoshd, stummala,
	vbadigan, rampraka, sayalil, sartgarg, rnayak, saiprakash.ranjan,
	sibis, okukatla, djakov, cang, pragalla, nitirawa, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, agross

On 2021-06-16 05:38, Bjorn Andersson wrote:
> On Tue 15 Jun 03:56 CDT 2021, sbhanu@codeaurora.org wrote:
> 
>> On 2021-06-14 17:00, sbhanu@codeaurora.org wrote:
>> > On 2021-06-11 09:12, Bjorn Andersson wrote:
>> > > On Wed 09 Jun 10:20 CDT 2021, Shaik Sajida Bhanu wrote:
> [..]
>> > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> [..]
>> > > > +		sdhc_1: sdhci@7c4000 {
> [..]
>> > > > +			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

yes

Thanks,
Sajida

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-06-16  5:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 15:20 [PATCH V3] arm64: dts: qcom: sc7280: Add nodes for eMMC and SD card Shaik Sajida Bhanu
2021-06-09 20:45 ` Konrad Dybcio
2021-06-14 11:25   ` sbhanu
2021-06-11  3:42 ` Bjorn Andersson
2021-06-14 11:30   ` sbhanu
2021-06-15  8:56     ` sbhanu
2021-06-16  0:08       ` Bjorn Andersson
2021-06-16  5:54         ` sbhanu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).