2024-01-18 13:31:47

by Chukun Pan

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

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



2024-01-18 13:36:36

by Chukun Pan

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

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 <[email protected]>
---
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


2024-01-27 23:20:42

by Bjorn Andersson

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

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 <[email protected]>
> ---
> 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
>

2024-01-29 02:40:48

by Chukun Pan

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

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


2024-02-02 04:15:40

by Bjorn Andersson

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

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

2024-02-03 07:00:44

by Chukun Pan

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

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


2024-02-03 13:44:02

by Dmitry Baryshkov

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

On Sat, 3 Feb 2024 at 08:00, Chukun Pan <[email protected]> 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