[PATCH v3 0/2] arm64: dts: qcom: ipq6018: enable sdhci node

[PATCH v3 0/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Chukun Pan]

Changes in v3:
  Remove always-on for LDOA2 regulator.
  Remove 1.8v properties of the node added in dtsi.

Changes in v2:
  Add LDOA2 regulator to support SDCC voltage scaling.

-- 
2.25.1

[PATCH v3 1/2] arm64: dts: qcom: ipq6018: add LDOA2 regulator

[Chukun Pan]

Add LDOA2 regulator of MP5496 to support SDCC voltage scaling.

Suggested-by: Robert Marko <robimarko@gmail.com>
Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 5e1277fea725..322eced0b876 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -174,6 +174,11 @@ ipq6018_s2: s2 {
 						regulator-max-microvolt = <1062500>;
 						regulator-always-on;
 					};
+
+					ipq6018_l2: l2 {
+						regulator-min-microvolt = <1800000>;
+						regulator-max-microvolt = <3300000>;
+					};
 				};
 			};
 		};
-- 
2.25.1

[PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Chukun Pan]

Enable mmc device found on ipq6018 devices.
This node supports both eMMC and SD cards.

Tested with:
  eMMC (HS200)
  SD Card (SDR50/SDR104)

Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index 322eced0b876..420c192bccd9 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -441,6 +441,25 @@ dwc_1: usb@7000000 {
 			};
 		};
 
+		sdhc: mmc@7804000 {
+			compatible = "qcom,ipq6018-sdhci", "qcom,sdhci-msm-v5";
+			reg = <0x0 0x7804000 0x0 0x1000>,
+			      <0x0 0x7805000 0x0 0x1000>;
+			reg-names = "hc", "cqhci";
+
+			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "hc_irq", "pwr_irq";
+
+			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
+				 <&gcc GCC_SDCC1_APPS_CLK>,
+				 <&xo>;
+			clock-names = "iface", "core", "xo";
+			resets = <&gcc GCC_SDCC1_BCR>;
+			max-frequency = <192000000>;
+			status = "disabled";
+		};
+
 		blsp_dma: dma-controller@7884000 {
 			compatible = "qcom,bam-v1.7.0";
 			reg = <0x0 0x07884000 0x0 0x2b000>;
-- 
2.25.1

Re: [PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Bjorn Andersson]

On Thu, Jan 18, 2024 at 09:30:22PM +0800, Chukun Pan wrote:
> Enable mmc device found on ipq6018 devices.
> This node supports both eMMC and SD cards.
> 
> Tested with:
>   eMMC (HS200)
>   SD Card (SDR50/SDR104)
> 
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> index 322eced0b876..420c192bccd9 100644
> --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> @@ -441,6 +441,25 @@ dwc_1: usb@7000000 {
>  			};
>  		};
>  
> +		sdhc: mmc@7804000 {
> +			compatible = "qcom,ipq6018-sdhci", "qcom,sdhci-msm-v5";
> +			reg = <0x0 0x7804000 0x0 0x1000>,
> +			      <0x0 0x7805000 0x0 0x1000>;
> +			reg-names = "hc", "cqhci";
> +
> +			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "hc_irq", "pwr_irq";
> +
> +			clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> +				 <&gcc GCC_SDCC1_APPS_CLK>,
> +				 <&xo>;
> +			clock-names = "iface", "core", "xo";
> +			resets = <&gcc GCC_SDCC1_BCR>;
> +			max-frequency = <192000000>;
> +			status = "disabled";

Subject and commit message says "enable", but this says disable. Could
you change this to "Add" instead?

Do you have a patch for any board where this is actually enabled?
Perhaps you missed a 3rd patch that enables this and uses the ipq6018_l2
regulator you add in patch 1?

Regards,
Bjorn

> +		};
> +
>  		blsp_dma: dma-controller@7884000 {
>  			compatible = "qcom,bam-v1.7.0";
>  			reg = <0x0 0x07884000 0x0 0x2b000>;
> -- 
> 2.25.1
> 

Re: [PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Chukun Pan]

Hi, Bjorn
> Subject and commit message says "enable", but this says disable. Could
> you change this to "Add" instead?

Thanks for your suggestion, I will change this to "Add".

> Do you have a patch for any board where this is actually enabled?
> Perhaps you missed a 3rd patch that enables this and uses the ipq6018_l2
> regulator you add in patch 1?

Some ipq6000 devices do not have pmic chips, resulting in l2 being
unavailable. So vqmmc-supply should be configured in the dts of each
specific device. As Robert suggested, the ipq6018_l2 node is used for
the device dts reference.

Thanks,
Chukun

-- 
2.25.1

Re: Re: [PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Bjorn Andersson]

On Mon, Jan 29, 2024 at 10:40:06AM +0800, Chukun Pan wrote:
> Hi, Bjorn
> > Subject and commit message says "enable", but this says disable. Could
> > you change this to "Add" instead?
> 
> Thanks for your suggestion, I will change this to "Add".
> 

Thanks

> > Do you have a patch for any board where this is actually enabled?
> > Perhaps you missed a 3rd patch that enables this and uses the ipq6018_l2
> > regulator you add in patch 1?
> 
> Some ipq6000 devices do not have pmic chips, resulting in l2 being
> unavailable. So vqmmc-supply should be configured in the dts of each
> specific device. As Robert suggested, the ipq6018_l2 node is used for
> the device dts reference.
> 

That sounds good, but do we have any one of those boards that should
reference &ipq6018_l2? Could make plug it into the sdhci node on some
board?

Essentially, why is it needed upstream, when there are no user?

Regards,
Bjorn

Re: [PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Chukun Pan]

Hi, Bjorn
> That sounds good, but do we have any one of those boards that should
> reference &ipq6018_l2? Could make plug it into the sdhci node on some
> board?

Actually I have an ipq6010 sdcard device with pmic, which needs to
reference ipq6018_l2. Also on the downstream qsdk kernel, the sdhc
node writes 'vqmmc-supply = <&ipq6018_l2>;' by default.

> Essentially, why is it needed upstream, when there are no user?

Most ipq60xx devices have pmic chips, including some ipq6000 devices,
while another ipq6000 devices do not have the pmic chips. So it does not
mean there are no users but the supply is board specific. Maybe we should
move the mp5496 node outside of ipq6018.dtsi.

Thanks,
Chukun

-- 
2.25.1

Re: [PATCH v3 2/2] arm64: dts: qcom: ipq6018: enable sdhci node

[Dmitry Baryshkov]

On Sat, 3 Feb 2024 at 08:00, Chukun Pan <amadeus@jmu.edu.cn> wrote:
>
> Hi, Bjorn
> > That sounds good, but do we have any one of those boards that should
> > reference &ipq6018_l2? Could make plug it into the sdhci node on some
> > board?
>
> Actually I have an ipq6010 sdcard device with pmic, which needs to
> reference ipq6018_l2. Also on the downstream qsdk kernel, the sdhc
> node writes 'vqmmc-supply = <&ipq6018_l2>;' by default.
>
> > Essentially, why is it needed upstream, when there are no user?
>
> Most ipq60xx devices have pmic chips, including some ipq6000 devices,
> while another ipq6000 devices do not have the pmic chips. So it does not
> mean there are no users but the supply is board specific. Maybe we should
> move the mp5496 node outside of ipq6018.dtsi.

Yes, please. In the end, mp5496 is not a part of the SoC.

-- 
With best wishes
Dmitry