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

[André Przywara]

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 at 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
>