diff mbox series

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

Message ID 20240118133022.553339-3-amadeus@jmu.edu.cn
State New
Headers
Series arm64: dts: qcom: ipq6018: enable sdhci node |

Commit Message

Chukun Pan Jan. 18, 2024, 1:30 p.m. UTC 
  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(+)

Comments

Bjorn Andersson Jan. 27, 2024, 11:19 p.m. UTC | #1 
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
>
Chukun Pan Jan. 29, 2024, 2:40 a.m. UTC | #2 
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
Bjorn Andersson Feb. 2, 2024, 4:15 a.m. UTC | #3 
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
Chukun Pan Feb. 3, 2024, 7 a.m. UTC | #4 
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
Dmitry Baryshkov Feb. 3, 2024, 1:43 p.m. UTC | #5 
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.
diff mbox series

Patch

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