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