[PATCH 2/6] clk: actions: Add SD/MMC clocks

André Przywara
2020-12-14T01:16:03Z

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 <amittomer25@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
>