On 13/12/2020 09:44, Amit Singh Tomar wrote:

Hi,

> This commit adds SD/MMC clocks, and provides .set/get_rate callbacks
> for SD/MMC device present on Actions OWL S700 SoCs.
> 
> Signed-off-by: Amit Singh Tomar <amittome...@gmail.com>
> ---
>  drivers/clk/owl/clk_owl.c | 66 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/owl/clk_owl.h |  2 ++
>  2 files changed, 68 insertions(+)
> 
> diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c
> index c9bc5c2..49a492c 100644
> --- a/drivers/clk/owl/clk_owl.c
> +++ b/drivers/clk/owl/clk_owl.c
> @@ -92,6 +92,9 @@ int owl_clk_enable(struct clk *clk)
>               setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
>               setbits_le32(priv->base + CMU_ETHERNETPLL, 5);
>               break;
> +     case CLK_SD0:
> +             setbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
> +             break;
>       default:
>               return -EINVAL;
>       }
> @@ -121,6 +124,9 @@ int owl_clk_disable(struct clk *clk)
>       case CLK_ETHERNET:
>               clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
>               break;
> +     case CLK_SD0:
> +             clrbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
> +             break;
>       default:
>               return -EINVAL;
>       }
> @@ -128,12 +134,69 @@ int owl_clk_disable(struct clk *clk)
>       return 0;
>  }
>  
> +static ulong owl_get_sd_clk_rate(struct owl_clk_priv *priv, int sd_index)
> +{
> +     ulong parent_rate;
> +     uint div = 1;
> +
> +     /* Clock output of DEV_PLL
> +      * Range: 48M ~ 756M
> +      * Frequency= DEVPLLCLK * 6
> +      */
> +     parent_rate = readl(priv->base + CMU_DEVPLL) & 0x7f;

According to the datasheet the clock source could also be NAND_PLL,
depending on bit 9.
Both PLLs use the same rate calculation, so it's just matter of the PLL
address offset to use for covering both.

> +     parent_rate *= 6000000;
> +
> +     div += readl(priv->base + (CMU_SD0CLK + sd_index*0x4)) & 0x1f;
> +
> +     return (parent_rate / div);
> +}
> +
> +static ulong owl_set_sd_clk_rate(struct owl_clk_priv *priv, ulong rate,
> +                              int sd_index)
> +{
> +     ulong parent_rate;
> +     uint div = 1, val;

Unneeded initialisation, you overwrite this below.

> +
> +     /* Clock output of DEV_PLL
> +      * Range: 48M ~ 756M
> +      * Frequency= DEVPLLCLK * 6
> +      */
> +     parent_rate = readl(priv->base + CMU_DEVPLL) & 0x7f;
> +     parent_rate *= 6000000;
> +
> +     rate *= 2;

Where does this come from?

> +     div = (parent_rate / rate) - 1;

Please check rate being not zero before you dividing by it. And I would
keep the real divider value for now, so drop the -1 here.

> +
> +     if (div >= 128)
> +             div |= 0x100;

This does not seem right. You would need to divide the value by 128,
once you decided to use the "divide-by-128" bit (which is bit 8).
What about moving this below, after you know the register value?
Then you can divide by 128 and set bit 8.

> +
> +     val = readl(priv->base + (CMU_SD0CLK + sd_index*0x4));
> +     /* Bits 4..0 is used to program div value */
> +     val &= ~0x1f;
> +     val |= div;

Please mask div before applying.

> +     /* As per Manuals Bits 31..10 are reserved but Bits 11 and
> +      * 10 needed to be set for proper operation
> +      */
> +     val |= 0xc00;

Where does this come from? I don't see the Linux driver doing that?

> +     /* Bit 9 and 8 must be cleared */

Bit 9 is the parent clock selection, which you hard code to DEV_CLK.
Bit 8 is the divide-by-128 bit.
Please split those two up and explain it in a comment, maybe.

Cheers,
Andre

> +     if (div < 128)
> +             val &= ~0x300;
> +     else
> +             val &= ~0x200;
> +     writel(val, priv->base + (CMU_SD0CLK + sd_index*0x4));
> +
> +     return owl_get_sd_clk_rate(priv, 0);
> +}
> +
>  static ulong owl_clk_get_rate(struct clk *clk)
>  {
>       struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>       ulong rate;
>  
>       switch (clk->id) {
> +     case CLK_SD0:
> +             rate = owl_get_sd_clk_rate(priv, 0);
> +             break;
>       default:
>               return -ENOENT;
>       }
> @@ -147,6 +210,9 @@ static ulong owl_clk_set_rate(struct clk *clk, ulong rate)
>       ulong new_rate;
>  
>       switch (clk->id) {
> +     case CLK_SD0:
> +             new_rate = owl_set_sd_clk_rate(priv, rate, 0);
> +             break;
>       default:
>               return -ENOENT;
>       }
> diff --git a/drivers/clk/owl/clk_owl.h b/drivers/clk/owl/clk_owl.h
> index a01f81a..ee5eba4 100644
> --- a/drivers/clk/owl/clk_owl.h
> +++ b/drivers/clk/owl/clk_owl.h
> @@ -62,4 +62,6 @@ struct owl_clk_priv {
>  #define CMU_DEVCLKEN1_UART5  BIT(21)
>  #define CMU_DEVCLKEN1_UART3  BIT(11)
>  
> +#define CMU_DEVCLKEN0_SD0    BIT(22)
> +
>  #endif
> 

Reply via email to