Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Ulf Hansson]

On 12 April 2016 at 00:11, Stephen Boyd  wrote:
> On 04/11, Ulf Hansson wrote:
>> + Adrian
>>
>> On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
>> > The DLL clock has to be enabled until the correct
>> > clock frequency is delivered to DLL
>> > '1'(default) - DLL clock is disabled
>> > '0' - dll clock has legacly clock enable.
>> >
>> > Signed-off-by: Varadarajan Narayanan 
>> > Signed-off-by: Sreedhar Sambangi 
>>
>> Adrian Hunter is the maintainer for sdhci, next time make sure to post to 
>> him.
>>
>> As this seems like fairly trivial change I decided to pick it up
>> anyway. So applied for next!
>>
>> Note, that I changed the prefix of the commit message header to "mmc".
>>
>
> I'm not sure this patch is actually right. In the downstream
> sources we do quite a few more reads and writes if we need to
> poke this second DLL configuration register. Furthermore, on
> msm8974 and apq8084 this register doesn't even exist so writing
> to it may cause problems if it isn't write ignored (I haven't
> checked).
>
> I think we should follow the downstream kernel design instead.
> Namely, reading the major/minor version registers to figure out
> if we should be touching this register in the first place, and
> then adding a clock property to the DT binding for the XO source
> so we can determine the XO frequency. It seems that we need this
> frequency to figure out how to program the second DLL
> configuration register appropriately.

Stephen,

Thanks for reviewing. I have dropped this patch for now.

Kind regards
Uffe

Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Ulf Hansson]

On 12 April 2016 at 00:11, Stephen Boyd  wrote:
> On 04/11, Ulf Hansson wrote:
>> + Adrian
>>
>> On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
>> > The DLL clock has to be enabled until the correct
>> > clock frequency is delivered to DLL
>> > '1'(default) - DLL clock is disabled
>> > '0' - dll clock has legacly clock enable.
>> >
>> > Signed-off-by: Varadarajan Narayanan 
>> > Signed-off-by: Sreedhar Sambangi 
>>
>> Adrian Hunter is the maintainer for sdhci, next time make sure to post to 
>> him.
>>
>> As this seems like fairly trivial change I decided to pick it up
>> anyway. So applied for next!
>>
>> Note, that I changed the prefix of the commit message header to "mmc".
>>
>
> I'm not sure this patch is actually right. In the downstream
> sources we do quite a few more reads and writes if we need to
> poke this second DLL configuration register. Furthermore, on
> msm8974 and apq8084 this register doesn't even exist so writing
> to it may cause problems if it isn't write ignored (I haven't
> checked).
>
> I think we should follow the downstream kernel design instead.
> Namely, reading the major/minor version registers to figure out
> if we should be touching this register in the first place, and
> then adding a clock property to the DT binding for the XO source
> so we can determine the XO frequency. It seems that we need this
> frequency to figure out how to program the second DLL
> configuration register appropriately.

Stephen,

Thanks for reviewing. I have dropped this patch for now.

Kind regards
Uffe

Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Stephen Boyd]

On 04/11, Ulf Hansson wrote:
> + Adrian
> 
> On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
> > The DLL clock has to be enabled until the correct
> > clock frequency is delivered to DLL
> > '1'(default) - DLL clock is disabled
> > '0' - dll clock has legacly clock enable.
> >
> > Signed-off-by: Varadarajan Narayanan 
> > Signed-off-by: Sreedhar Sambangi 
> 
> Adrian Hunter is the maintainer for sdhci, next time make sure to post to him.
> 
> As this seems like fairly trivial change I decided to pick it up
> anyway. So applied for next!
> 
> Note, that I changed the prefix of the commit message header to "mmc".
> 

I'm not sure this patch is actually right. In the downstream
sources we do quite a few more reads and writes if we need to
poke this second DLL configuration register. Furthermore, on
msm8974 and apq8084 this register doesn't even exist so writing
to it may cause problems if it isn't write ignored (I haven't
checked).

I think we should follow the downstream kernel design instead.
Namely, reading the major/minor version registers to figure out
if we should be touching this register in the first place, and
then adding a clock property to the DT binding for the XO source
so we can determine the XO frequency. It seems that we need this
frequency to figure out how to program the second DLL
configuration register appropriately.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Stephen Boyd]

On 04/11, Ulf Hansson wrote:
> + Adrian
> 
> On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
> > The DLL clock has to be enabled until the correct
> > clock frequency is delivered to DLL
> > '1'(default) - DLL clock is disabled
> > '0' - dll clock has legacly clock enable.
> >
> > Signed-off-by: Varadarajan Narayanan 
> > Signed-off-by: Sreedhar Sambangi 
> 
> Adrian Hunter is the maintainer for sdhci, next time make sure to post to him.
> 
> As this seems like fairly trivial change I decided to pick it up
> anyway. So applied for next!
> 
> Note, that I changed the prefix of the commit message header to "mmc".
> 

I'm not sure this patch is actually right. In the downstream
sources we do quite a few more reads and writes if we need to
poke this second DLL configuration register. Furthermore, on
msm8974 and apq8084 this register doesn't even exist so writing
to it may cause problems if it isn't write ignored (I haven't
checked).

I think we should follow the downstream kernel design instead.
Namely, reading the major/minor version registers to figure out
if we should be touching this register in the first place, and
then adding a clock property to the DT binding for the XO source
so we can determine the XO frequency. It seems that we need this
frequency to figure out how to program the second DLL
configuration register appropriately.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Ulf Hansson]

+ Adrian

On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
> The DLL clock has to be enabled until the correct
> clock frequency is delivered to DLL
> '1'(default) - DLL clock is disabled
> '0' - dll clock has legacly clock enable.
>
> Signed-off-by: Varadarajan Narayanan 
> Signed-off-by: Sreedhar Sambangi 

Adrian Hunter is the maintainer for sdhci, next time make sure to post to him.

As this seems like fairly trivial change I decided to pick it up
anyway. So applied for next!

Note, that I changed the prefix of the commit message header to "mmc".

Thanks and kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-msm.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4695bee..95b8b70 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -43,6 +43,9 @@
>  #define CORE_DLL_CONFIG0x100
>  #define CORE_DLL_STATUS0x108
>
> +#define CORE_DLL_CONFIG2   0x1b4
> +#define CORE_DLL_CLK_DISABLE   BIT(21)
> +
>  #define CORE_VENDOR_SPEC   0x10c
>  #define CORE_CLK_PWRSAVE   BIT(1)
>
> @@ -326,6 +329,10 @@ static int msm_init_cm_dll(struct sdhci_host *host)
> writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
> | CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
>
> +   /* Write 0 to DLL_CLOCK_DISABLE bit of DLL_CONFIG_2 register */
> +   writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG2)
> +   & ~CORE_DLL_CLK_DISABLE), host->ioaddr + CORE_DLL_CONFIG2);
> +
> /* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
> while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
>  CORE_DLL_LOCK)) {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] qcom: sdhci-msm: enable the DLL clock

[Ulf Hansson]

+ Adrian

On 5 April 2016 at 09:46, Sreedhar Sambangi  wrote:
> The DLL clock has to be enabled until the correct
> clock frequency is delivered to DLL
> '1'(default) - DLL clock is disabled
> '0' - dll clock has legacly clock enable.
>
> Signed-off-by: Varadarajan Narayanan 
> Signed-off-by: Sreedhar Sambangi 

Adrian Hunter is the maintainer for sdhci, next time make sure to post to him.

As this seems like fairly trivial change I decided to pick it up
anyway. So applied for next!

Note, that I changed the prefix of the commit message header to "mmc".

Thanks and kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-msm.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 4695bee..95b8b70 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -43,6 +43,9 @@
>  #define CORE_DLL_CONFIG0x100
>  #define CORE_DLL_STATUS0x108
>
> +#define CORE_DLL_CONFIG2   0x1b4
> +#define CORE_DLL_CLK_DISABLE   BIT(21)
> +
>  #define CORE_VENDOR_SPEC   0x10c
>  #define CORE_CLK_PWRSAVE   BIT(1)
>
> @@ -326,6 +329,10 @@ static int msm_init_cm_dll(struct sdhci_host *host)
> writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
> | CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
>
> +   /* Write 0 to DLL_CLOCK_DISABLE bit of DLL_CONFIG_2 register */
> +   writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG2)
> +   & ~CORE_DLL_CLK_DISABLE), host->ioaddr + CORE_DLL_CONFIG2);
> +
> /* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
> while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
>  CORE_DLL_LOCK)) {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] qcom: sdhci-msm: enable the DLL clock

[Sreedhar Sambangi]

The DLL clock has to be enabled until the correct
clock frequency is delivered to DLL
'1'(default) - DLL clock is disabled
'0' - dll clock has legacly clock enable.

Signed-off-by: Varadarajan Narayanan 
Signed-off-by: Sreedhar Sambangi 
---
 drivers/mmc/host/sdhci-msm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4695bee..95b8b70 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -43,6 +43,9 @@
 #define CORE_DLL_CONFIG0x100
 #define CORE_DLL_STATUS0x108
 
+#define CORE_DLL_CONFIG2   0x1b4
+#define CORE_DLL_CLK_DISABLE   BIT(21)
+
 #define CORE_VENDOR_SPEC   0x10c
 #define CORE_CLK_PWRSAVE   BIT(1)
 
@@ -326,6 +329,10 @@ static int msm_init_cm_dll(struct sdhci_host *host)
writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
| CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
 
+   /* Write 0 to DLL_CLOCK_DISABLE bit of DLL_CONFIG_2 register */
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG2)
+   & ~CORE_DLL_CLK_DISABLE), host->ioaddr + CORE_DLL_CONFIG2);
+
/* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
 CORE_DLL_LOCK)) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] qcom: sdhci-msm: enable the DLL clock

[Sreedhar Sambangi]

The DLL clock has to be enabled until the correct
clock frequency is delivered to DLL
'1'(default) - DLL clock is disabled
'0' - dll clock has legacly clock enable.

Signed-off-by: Varadarajan Narayanan 
Signed-off-by: Sreedhar Sambangi 
---
 drivers/mmc/host/sdhci-msm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 4695bee..95b8b70 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -43,6 +43,9 @@
 #define CORE_DLL_CONFIG0x100
 #define CORE_DLL_STATUS0x108
 
+#define CORE_DLL_CONFIG2   0x1b4
+#define CORE_DLL_CLK_DISABLE   BIT(21)
+
 #define CORE_VENDOR_SPEC   0x10c
 #define CORE_CLK_PWRSAVE   BIT(1)
 
@@ -326,6 +329,10 @@ static int msm_init_cm_dll(struct sdhci_host *host)
writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG)
| CORE_CK_OUT_EN), host->ioaddr + CORE_DLL_CONFIG);
 
+   /* Write 0 to DLL_CLOCK_DISABLE bit of DLL_CONFIG_2 register */
+   writel_relaxed((readl_relaxed(host->ioaddr + CORE_DLL_CONFIG2)
+   & ~CORE_DLL_CLK_DISABLE), host->ioaddr + CORE_DLL_CONFIG2);
+
/* Wait until DLL_LOCK bit of DLL_STATUS register becomes '1' */
while (!(readl_relaxed(host->ioaddr + CORE_DLL_STATUS) &
 CORE_DLL_LOCK)) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project