2021-06-10 07:14:55

by Shaik Sajida Bhanu

[permalink] [raw]
Subject: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

Added xo clock for eMMC and Sd card.

Signed-off-by: Shaik Sajida Bhanu <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 295844e..5bb6bd4 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -701,8 +701,9 @@
interrupt-names = "hc_irq", "pwr_irq";

clocks = <&gcc GCC_SDCC1_APPS_CLK>,
- <&gcc GCC_SDCC1_AHB_CLK>;
- clock-names = "core", "iface";
+ <&gcc GCC_SDCC1_AHB_CLK>,
+ <&rpmhcc RPMH_CXO_CLK>;
+ clock-names = "core", "iface","xo";
interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
interconnect-names = "sdhc-ddr","cpu-sdhc";
@@ -2666,8 +2667,9 @@
interrupt-names = "hc_irq", "pwr_irq";

clocks = <&gcc GCC_SDCC2_APPS_CLK>,
- <&gcc GCC_SDCC2_AHB_CLK>;
- clock-names = "core", "iface";
+ <&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 &config_noc SLAVE_SDCC_2 0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2021-06-10 07:57:59

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

Hi,


> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -701,8 +701,9 @@
> interrupt-names = "hc_irq", "pwr_irq";
>
> clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> - <&gcc GCC_SDCC1_AHB_CLK>;
> - clock-names = "core", "iface";
> + <&gcc GCC_SDCC1_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>;

Don't these clocks fit in 100 chars?



> + clock-names = "core", "iface","xo";

A space is missing before "xo".



> interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
> <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
> interconnect-names = "sdhc-ddr","cpu-sdhc";
> @@ -2666,8 +2667,9 @@
> interrupt-names = "hc_irq", "pwr_irq";
>
> clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> - <&gcc GCC_SDCC2_AHB_CLK>;
> - clock-names = "core", "iface";
> + <&gcc GCC_SDCC2_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>;

Ditto



Konrad

2021-06-11 03:35:46

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On Thu 10 Jun 02:54 CDT 2021, Konrad Dybcio wrote:

> Hi,
>
>
> > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > @@ -701,8 +701,9 @@
> > interrupt-names = "hc_irq", "pwr_irq";
> >
> > clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> > - <&gcc GCC_SDCC1_AHB_CLK>;
> > - clock-names = "core", "iface";
> > + <&gcc GCC_SDCC1_AHB_CLK>,
> > + <&rpmhcc RPMH_CXO_CLK>;
>
> Don't these clocks fit in 100 chars?
>

We typically list them on one line each, so I'm fine with that.

But all three clocks should be aligned.

>
>
> > + clock-names = "core", "iface","xo";
>
> A space is missing before "xo".
>

Thanks for pointing this out.

I fixed up the indentation and applied the patch.

Regards,
Bjorn

>
>
> > interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
> > <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
> > interconnect-names = "sdhc-ddr","cpu-sdhc";
> > @@ -2666,8 +2667,9 @@
> > interrupt-names = "hc_irq", "pwr_irq";
> >
> > clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> > - <&gcc GCC_SDCC2_AHB_CLK>;
> > - clock-names = "core", "iface";
> > + <&gcc GCC_SDCC2_AHB_CLK>,
> > + <&rpmhcc RPMH_CXO_CLK>;
>
> Ditto
>
>
>
> Konrad
>

2021-06-11 04:34:34

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On Thu 10 Jun 02:11 CDT 2021, Shaik Sajida Bhanu wrote:

> Added xo clock for eMMC and Sd card.

Was about to push out my branch of patches, but before I do. Can you
please describe WHY this is needed?

Regards,
Bjorn

>
> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 295844e..5bb6bd4 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -701,8 +701,9 @@
> interrupt-names = "hc_irq", "pwr_irq";
>
> clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> - <&gcc GCC_SDCC1_AHB_CLK>;
> - clock-names = "core", "iface";
> + <&gcc GCC_SDCC1_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>;
> + clock-names = "core", "iface","xo";
> interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
> <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
> interconnect-names = "sdhc-ddr","cpu-sdhc";
> @@ -2666,8 +2667,9 @@
> interrupt-names = "hc_irq", "pwr_irq";
>
> clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> - <&gcc GCC_SDCC2_AHB_CLK>;
> - clock-names = "core", "iface";
> + <&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 &config_noc SLAVE_SDCC_2 0>;
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>

2021-06-14 11:47:28

by Shaik Sajida Bhanu

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On 2021-06-10 13:24, Konrad Dybcio wrote:
> Hi,
>
>
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -701,8 +701,9 @@
>> interrupt-names = "hc_irq", "pwr_irq";
>>
>> clocks = <&gcc GCC_SDCC1_APPS_CLK>,
>> - <&gcc GCC_SDCC1_AHB_CLK>;
>> - clock-names = "core", "iface";
>> + <&gcc GCC_SDCC1_AHB_CLK>,
>> + <&rpmhcc RPMH_CXO_CLK>;
>
> Don't these clocks fit in 100 chars?
These two clocks can fit in 100 chars but we have 3 clocks they don't
fit in 100 chars.
>
>
>
>> + clock-names = "core", "iface","xo";
>
> A space is missing before "xo".
Sure
>
>
>
>> interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
>> <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
>> interconnect-names = "sdhc-ddr","cpu-sdhc";
>> @@ -2666,8 +2667,9 @@
>> interrupt-names = "hc_irq", "pwr_irq";
>>
>> clocks = <&gcc GCC_SDCC2_APPS_CLK>,
>> - <&gcc GCC_SDCC2_AHB_CLK>;
>> - clock-names = "core", "iface";
>> + <&gcc GCC_SDCC2_AHB_CLK>,
>> + <&rpmhcc RPMH_CXO_CLK>;
>
> Ditto
Same as above
>
>
>
> Konrad

2021-06-14 11:52:42

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card


On 14.06.2021 13:44, [email protected] wrote:
> On 2021-06-10 13:24, Konrad Dybcio wrote:
>> Hi,
>>
>>
>>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>>> @@ -701,8 +701,9 @@
>>>              interrupt-names = "hc_irq", "pwr_irq";
>>>
>>>              clocks = <&gcc GCC_SDCC1_APPS_CLK>,
>>> -                    <&gcc GCC_SDCC1_AHB_CLK>;
>>> -            clock-names = "core", "iface";
>>> +                    <&gcc GCC_SDCC1_AHB_CLK>,
>>> +                    <&rpmhcc RPMH_CXO_CLK>;
>>
>> Don't these clocks fit in 100 chars?
> These two clocks can fit in 100 chars but we have 3 clocks they don't fit in 100 chars.

You're right.


With the space before xo,

Reviewed-by: Konrad Dybcio <[email protected]>


Konrad

2021-06-14 11:59:33

by Shaik Sajida Bhanu

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On 2021-06-11 10:00, Bjorn Andersson wrote:
> On Thu 10 Jun 02:11 CDT 2021, Shaik Sajida Bhanu wrote:
>
>> Added xo clock for eMMC and Sd card.
>
> Was about to push out my branch of patches, but before I do. Can you
> please describe WHY this is needed?
>
> Regards,
> Bjorn

We are making use of this clock in dll register value calculation,
The default PoR value is also same as calculated value for
HS200/HS400/SDR104 modes.
But just not to rely on default register values we need this entry.

>
>>
>> Signed-off-by: Shaik Sajida Bhanu <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> index 295844e..5bb6bd4 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> @@ -701,8 +701,9 @@
>> interrupt-names = "hc_irq", "pwr_irq";
>>
>> clocks = <&gcc GCC_SDCC1_APPS_CLK>,
>> - <&gcc GCC_SDCC1_AHB_CLK>;
>> - clock-names = "core", "iface";
>> + <&gcc GCC_SDCC1_AHB_CLK>,
>> + <&rpmhcc RPMH_CXO_CLK>;
>> + clock-names = "core", "iface","xo";
>> interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
>> <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
>> interconnect-names = "sdhc-ddr","cpu-sdhc";
>> @@ -2666,8 +2667,9 @@
>> interrupt-names = "hc_irq", "pwr_irq";
>>
>> clocks = <&gcc GCC_SDCC2_APPS_CLK>,
>> - <&gcc GCC_SDCC2_AHB_CLK>;
>> - clock-names = "core", "iface";
>> + <&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 &config_noc SLAVE_SDCC_2 0>;
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>

2021-06-14 16:35:03

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On Mon 14 Jun 06:55 CDT 2021, [email protected] wrote:

> On 2021-06-11 10:00, Bjorn Andersson wrote:
> > On Thu 10 Jun 02:11 CDT 2021, Shaik Sajida Bhanu wrote:
> >
> > > Added xo clock for eMMC and Sd card.
> >
> > Was about to push out my branch of patches, but before I do. Can you
> > please describe WHY this is needed?
> >
> > Regards,
> > Bjorn
>
> We are making use of this clock in dll register value calculation,
> The default PoR value is also same as calculated value for
> HS200/HS400/SDR104 modes.
> But just not to rely on default register values we need this entry.
>

That is the perfect thing to include in a commit message!

I rewrote yours and applied the change, but please next time do describe
the "why" of your change.

Regards,
Bjorn

> >
> > >
> > > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > index 295844e..5bb6bd4 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > > @@ -701,8 +701,9 @@
> > > interrupt-names = "hc_irq", "pwr_irq";
> > >
> > > clocks = <&gcc GCC_SDCC1_APPS_CLK>,
> > > - <&gcc GCC_SDCC1_AHB_CLK>;
> > > - clock-names = "core", "iface";
> > > + <&gcc GCC_SDCC1_AHB_CLK>,
> > > + <&rpmhcc RPMH_CXO_CLK>;
> > > + clock-names = "core", "iface","xo";
> > > interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
> > > <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
> > > interconnect-names = "sdhc-ddr","cpu-sdhc";
> > > @@ -2666,8 +2667,9 @@
> > > interrupt-names = "hc_irq", "pwr_irq";
> > >
> > > clocks = <&gcc GCC_SDCC2_APPS_CLK>,
> > > - <&gcc GCC_SDCC2_AHB_CLK>;
> > > - clock-names = "core", "iface";
> > > + <&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 &config_noc SLAVE_SDCC_2 0>;
> > > --
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> > > member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > >

2021-06-15 10:02:12

by Shaik Sajida Bhanu

[permalink] [raw]
Subject: Re: [PATCH V1] arm64: dts: qcom: sc7180: Added xo clock for eMMC and Sd card

On 2021-06-14 22:00, Bjorn Andersson wrote:
> On Mon 14 Jun 06:55 CDT 2021, [email protected] wrote:
>
>> On 2021-06-11 10:00, Bjorn Andersson wrote:
>> > On Thu 10 Jun 02:11 CDT 2021, Shaik Sajida Bhanu wrote:
>> >
>> > > Added xo clock for eMMC and Sd card.
>> >
>> > Was about to push out my branch of patches, but before I do. Can you
>> > please describe WHY this is needed?
>> >
>> > Regards,
>> > Bjorn
>>
>> We are making use of this clock in dll register value calculation,
>> The default PoR value is also same as calculated value for
>> HS200/HS400/SDR104 modes.
>> But just not to rely on default register values we need this entry.
>>
>
> That is the perfect thing to include in a commit message!
>
> I rewrote yours and applied the change, but please next time do
> describe
> the "why" of your change.
>
> Regards,
> Bjorn
Sure
>
>> >
>> > >
>> > > Signed-off-by: Shaik Sajida Bhanu <[email protected]>
>> > > ---
>> > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++----
>> > > 1 file changed, 6 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> > > b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> > > index 295844e..5bb6bd4 100644
>> > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
>> > > @@ -701,8 +701,9 @@
>> > > interrupt-names = "hc_irq", "pwr_irq";
>> > >
>> > > clocks = <&gcc GCC_SDCC1_APPS_CLK>,
>> > > - <&gcc GCC_SDCC1_AHB_CLK>;
>> > > - clock-names = "core", "iface";
>> > > + <&gcc GCC_SDCC1_AHB_CLK>,
>> > > + <&rpmhcc RPMH_CXO_CLK>;
>> > > + clock-names = "core", "iface","xo";
>> > > interconnects = <&aggre1_noc MASTER_EMMC 0 &mc_virt SLAVE_EBI1 0>,
>> > > <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_EMMC_CFG 0>;
>> > > interconnect-names = "sdhc-ddr","cpu-sdhc";
>> > > @@ -2666,8 +2667,9 @@
>> > > interrupt-names = "hc_irq", "pwr_irq";
>> > >
>> > > clocks = <&gcc GCC_SDCC2_APPS_CLK>,
>> > > - <&gcc GCC_SDCC2_AHB_CLK>;
>> > > - clock-names = "core", "iface";
>> > > + <&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 &config_noc SLAVE_SDCC_2 0>;
>> > > --
>> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> > > member
>> > > of Code Aurora Forum, hosted by The Linux Foundation
>> > >