diff mbox series

[v5,1/1] arm64: dts: qcom: ipq6018: add sdhci node

Message ID 20240306123006.724934-2-amadeus@jmu.edu.cn (mailing list archive)
State Changes Requested
Headers show
Series arm64: dts: qcom: ipq6018: add sdhci node | expand

Commit Message

Chukun Pan March 6, 2024, 12:30 p.m. UTC
Add node to support mmc controller inside of IPQ6018.
This controller 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

Dmitry Baryshkov March 6, 2024, 7:43 p.m. UTC | #1
On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
>
> Add node to support mmc controller inside of IPQ6018.
> This controller 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 0x07804000 0x0 0x1000>,
> +                             <0x0 0x07805000 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>;

If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?

> +                       status = "disabled";
> +               };
> +
>                 blsp_dma: dma-controller@7884000 {
>                         compatible = "qcom,bam-v1.7.0";
>                         reg = <0x0 0x07884000 0x0 0x2b000>;
> --
> 2.25.1
>
>
Robert Marko March 6, 2024, 8:35 p.m. UTC | #2
On 06. 03. 2024. 20:43, Dmitry Baryshkov wrote:
> On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
>> Add node to support mmc controller inside of IPQ6018.
>> This controller 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 0x07804000 0x0 0x1000>,
>> +                             <0x0 0x07805000 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>;
> If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
> to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?
>
> I am not sure that 384MHz is actually supported as IPQ6018 datasheet
> clearly indicates that HS400 mode is not supported.
>
> Regards,
> Robert
>
>> +                       status = "disabled";
>> +               };
>> +
>>                  blsp_dma: dma-controller@7884000 {
>>                          compatible = "qcom,bam-v1.7.0";
>>                          reg = <0x0 0x07884000 0x0 0x2b000>;
>> --
>> 2.25.1
>>
>>
>
Dmitry Baryshkov March 7, 2024, 7:27 a.m. UTC | #3
On Wed, 6 Mar 2024 at 22:35, Robert Marko <robimarko@gmail.com> wrote:
>
>
> On 06. 03. 2024. 20:43, Dmitry Baryshkov wrote:
> > On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
> >> Add node to support mmc controller inside of IPQ6018.
> >> This controller 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 0x07804000 0x0 0x1000>,
> >> +                             <0x0 0x07805000 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>;
> > If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
> > to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?
> >
> > I am not sure that 384MHz is actually supported as IPQ6018 datasheet
> > clearly indicates that HS400 mode is not supported.

I didn't check the datasheet, I opened the gcc-ipq6018.c

> >
> > Regards,
> > Robert
> >
> >> +                       status = "disabled";
> >> +               };
> >> +
> >>                  blsp_dma: dma-controller@7884000 {
> >>                          compatible = "qcom,bam-v1.7.0";
> >>                          reg = <0x0 0x07884000 0x0 0x2b000>;
> >> --
> >> 2.25.1
> >>
> >>
> >
Robert Marko March 7, 2024, 7:38 a.m. UTC | #4
On Thu, 7 Mar 2024 at 08:28, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Wed, 6 Mar 2024 at 22:35, Robert Marko <robimarko@gmail.com> wrote:
> >
> >
> > On 06. 03. 2024. 20:43, Dmitry Baryshkov wrote:
> > > On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
> > >> Add node to support mmc controller inside of IPQ6018.
> > >> This controller 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 0x07804000 0x0 0x1000>,
> > >> +                             <0x0 0x07805000 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>;
> > > If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
> > > to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?
> > >
> > > I am not sure that 384MHz is actually supported as IPQ6018 datasheet
> > > clearly indicates that HS400 mode is not supported.
>
> I didn't check the datasheet, I opened the gcc-ipq6018.c

I understand that, I just pointed it out, it wouldn't surprise me if
the frequency table
was just copy/pasted from IPQ8074.

Regards,
Robert
>
> > >
> > > Regards,
> > > Robert
> > >
> > >> +                       status = "disabled";
> > >> +               };
> > >> +
> > >>                  blsp_dma: dma-controller@7884000 {
> > >>                          compatible = "qcom,bam-v1.7.0";
> > >>                          reg = <0x0 0x07884000 0x0 0x2b000>;
> > >> --
> > >> 2.25.1
> > >>
> > >>
> > >
>
>
>
> --
> With best wishes
> Dmitry
Dmitry Baryshkov March 7, 2024, 7:50 a.m. UTC | #5
On Thu, 7 Mar 2024 at 09:38, Robert Marko <robimarko@gmail.com> wrote:
>
> On Thu, 7 Mar 2024 at 08:28, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Wed, 6 Mar 2024 at 22:35, Robert Marko <robimarko@gmail.com> wrote:
> > >
> > >
> > > On 06. 03. 2024. 20:43, Dmitry Baryshkov wrote:
> > > > On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
> > > >> Add node to support mmc controller inside of IPQ6018.
> > > >> This controller 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 0x07804000 0x0 0x1000>,
> > > >> +                             <0x0 0x07805000 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>;
> > > > If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
> > > > to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?
> > > >
> > > > I am not sure that 384MHz is actually supported as IPQ6018 datasheet
> > > > clearly indicates that HS400 mode is not supported.
> >
> > I didn't check the datasheet, I opened the gcc-ipq6018.c
>
> I understand that, I just pointed it out, it wouldn't surprise me if
> the frequency table
> was just copy/pasted from IPQ8074.

Then it might be fixed instead, making the max-frequency property unnecessary.

>
> Regards,
> Robert
> >
> > > >
> > > > Regards,
> > > > Robert
> > > >
> > > >> +                       status = "disabled";
> > > >> +               };
> > > >> +
> > > >>                  blsp_dma: dma-controller@7884000 {
> > > >>                          compatible = "qcom,bam-v1.7.0";
> > > >>                          reg = <0x0 0x07884000 0x0 0x2b000>;
> > > >> --
> > > >> 2.25.1
> > > >>
> > > >>
> > > >
> >
> >
> >
> > --
> > With best wishes
> > Dmitry
Konrad Dybcio March 7, 2024, 11:50 a.m. UTC | #6
On 3/7/24 08:50, Dmitry Baryshkov wrote:
> On Thu, 7 Mar 2024 at 09:38, Robert Marko <robimarko@gmail.com> wrote:
>>
>> On Thu, 7 Mar 2024 at 08:28, Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>>>
>>> On Wed, 6 Mar 2024 at 22:35, Robert Marko <robimarko@gmail.com> wrote:
>>>>
>>>>
>>>> On 06. 03. 2024. 20:43, Dmitry Baryshkov wrote:
>>>>> On Wed, 6 Mar 2024 at 14:31, Chukun Pan <amadeus@jmu.edu.cn> wrote:
>>>>>> Add node to support mmc controller inside of IPQ6018.
>>>>>> This controller 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 0x07804000 0x0 0x1000>,
>>>>>> +                             <0x0 0x07805000 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>;
>>>>> If I understand correctly, GCC_SDCC1_APPS_CLK support frequencies up
>>>>> to 384 MHz, but here you are limiting it to 192 MHz. Why is it so?
>>>>>
>>>>> I am not sure that 384MHz is actually supported as IPQ6018 datasheet
>>>>> clearly indicates that HS400 mode is not supported.
>>>
>>> I didn't check the datasheet, I opened the gcc-ipq6018.c
>>
>> I understand that, I just pointed it out, it wouldn't surprise me if
>> the frequency table
>> was just copy/pasted from IPQ8074.
> 
> Then it might be fixed instead, making the max-frequency property unnecessary.

The clock driver contains clock settings that were either autogenerated
or manually included, or copypasted. These settings, and particularly
downstream, only describe the known-working clock rates and the minimum
required voltage setting just to keep them ticking nicely (think running
a car with the clutch pressed, no real load), a subset (which may be an
improper subset) of which gets translated into the device OPPs (or frequency
levels downstream). We should have an OPP table here.

Konrad
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 0x07804000 0x0 0x1000>,
+			      <0x0 0x07805000 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>;