diff mbox series

[v2,3/4] clk: renesas: cpg-mssr: add register pointers into struct cpg_mssr_priv

Message ID 1599657211-17504-4-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series clk: renesas: cpg-mssr: add support for V3U | expand

Commit Message

Yoshihiro Shimoda Sept. 9, 2020, 1:13 p.m. UTC 
To support other register layout in the future, add register pointers
of control_regs and status_regs into struct cpg_mssr_priv. After that,
we can remove unused macros like MSTPSR(). No behavior changes.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/clk/renesas/renesas-cpg-mssr.c | 56 ++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 26 deletions(-)

Comments

Geert Uytterhoeven Sept. 10, 2020, 1:11 p.m. UTC | #1 
Hi Shimoda-san,

On Wed, Sep 9, 2020 at 3:13 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> To support other register layout in the future, add register pointers
> of control_regs and status_regs into struct cpg_mssr_priv. After that,
> we can remove unused macros like MSTPSR(). No behavior changes.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c

> @@ -137,6 +130,8 @@ struct cpg_mssr_priv {
>         unsigned int last_dt_core_clk;
>
>         struct raw_notifier_head notifiers;
> +       const u16 *status_regs;
> +       const u16 *control_regs;

Please add new fields to the struct's kerneldoc comments above.

Note that you need to use a similar dispatching for the Software Reset
and Software Reset Clearing Registers, as their offsets differ from
previous SoCs.

The reset looks good to me, thanks!

Gr{oetje,eeting}s,

                        Geert
Yoshihiro Shimoda Sept. 11, 2020, 4:16 a.m. UTC | #2 
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, September 10, 2020 10:11 PM
> 
> Hi Shimoda-san,
> 
> On Wed, Sep 9, 2020 at 3:13 PM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > To support other register layout in the future, add register pointers
> > of control_regs and status_regs into struct cpg_mssr_priv. After that,
> > we can remove unused macros like MSTPSR(). No behavior changes.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> 
> > @@ -137,6 +130,8 @@ struct cpg_mssr_priv {
> >         unsigned int last_dt_core_clk;
> >
> >         struct raw_notifier_head notifiers;
> > +       const u16 *status_regs;
> > +       const u16 *control_regs;
> 
> Please add new fields to the struct's kerneldoc comments above.

Oops. I'll add such comments.

> Note that you need to use a similar dispatching for the Software Reset
> and Software Reset Clearing Registers, as their offsets differ from
> previous SoCs.

Indeed. I'll add these pointers.

> The reset looks good to me, thanks!

Thank you for your review!

Best regards,
Yoshihiro Shimoda
diff mbox series

Patch

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index 1b289c8..b3a84cd 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -57,9 +57,6 @@  static const u16 mstpsr[] = {
 	0x9A0, 0x9A4, 0x9A8, 0x9AC,
 };
 
-#define	MSTPSR(i)	mstpsr[i]
-
-
 /*
  * System Module Stop Control Register offsets
  */
@@ -69,8 +66,6 @@  static const u16 smstpcr[] = {
 	0x990, 0x994, 0x998, 0x99C,
 };
 
-#define	SMSTPCR(i)	smstpcr[i]
-
 /*
  * Standby Control Register offsets (RZ/A)
  * Base address is FRQCR register
@@ -81,8 +76,6 @@  static const u16 stbcr[] = {
 	0x424, 0x428, 0x42C,
 };
 
-#define	STBCR(i)	stbcr[i]
-
 /*
  * Software Reset Register offsets
  */
@@ -137,6 +130,8 @@  struct cpg_mssr_priv {
 	unsigned int last_dt_core_clk;
 
 	struct raw_notifier_head notifiers;
+	const u16 *status_regs;
+	const u16 *control_regs;
 	struct {
 		u32 mask;
 		u32 val;
@@ -178,23 +173,23 @@  static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
 	spin_lock_irqsave(&priv->rmw_lock, flags);
 
 	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A) {
-		value = readb(priv->base + STBCR(reg));
+		value = readb(priv->base + priv->control_regs[reg]);
 		if (enable)
 			value &= ~bitmask;
 		else
 			value |= bitmask;
-		writeb(value, priv->base + STBCR(reg));
+		writeb(value, priv->base + priv->control_regs[reg]);
 
 		/* dummy read to ensure write has completed */
-		readb(priv->base + STBCR(reg));
-		barrier_data(priv->base + STBCR(reg));
+		readb(priv->base + priv->control_regs[reg]);
+		barrier_data(priv->base + priv->control_regs[reg]);
 	} else {
-		value = readl(priv->base + SMSTPCR(reg));
+		value = readl(priv->base + priv->control_regs[reg]);
 		if (enable)
 			value &= ~bitmask;
 		else
 			value |= bitmask;
-		writel(value, priv->base + SMSTPCR(reg));
+		writel(value, priv->base + priv->control_regs[reg]);
 	}
 
 	spin_unlock_irqrestore(&priv->rmw_lock, flags);
@@ -203,14 +198,14 @@  static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
 		return 0;
 
 	for (i = 1000; i > 0; --i) {
-		if (!(readl(priv->base + MSTPSR(reg)) & bitmask))
+		if (!(readl(priv->base + priv->status_regs[reg]) & bitmask))
 			break;
 		cpu_relax();
 	}
 
 	if (!i) {
 		dev_err(dev, "Failed to enable SMSTP %p[%d]\n",
-			priv->base + SMSTPCR(reg), bit);
+			priv->base + priv->control_regs[reg], bit);
 		return -ETIMEDOUT;
 	}
 
@@ -234,9 +229,9 @@  static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
 	u32 value;
 
 	if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A)
-		value = readb(priv->base + STBCR(clock->index / 32));
+		value = readb(priv->base + priv->control_regs[clock->index / 32]);
 	else
-		value = readl(priv->base + MSTPSR(clock->index / 32));
+		value = readl(priv->base + priv->status_regs[clock->index / 32]);
 
 	return !(value & BIT(clock->index % 32));
 }
@@ -827,8 +822,8 @@  static int cpg_mssr_suspend_noirq(struct device *dev)
 		if (priv->smstpcr_saved[reg].mask)
 			priv->smstpcr_saved[reg].val =
 				priv->reg_layout == CLK_REG_LAYOUT_RZ_A ?
-				readb(priv->base + STBCR(reg)) :
-				readl(priv->base + SMSTPCR(reg));
+				readb(priv->base + priv->control_regs[reg]) :
+				readl(priv->base + priv->control_regs[reg]);
 	}
 
 	/* Save core clocks */
@@ -857,22 +852,22 @@  static int cpg_mssr_resume_noirq(struct device *dev)
 			continue;
 
 		if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A)
-			oldval = readb(priv->base + STBCR(reg));
+			oldval = readb(priv->base + priv->control_regs[reg]);
 		else
-			oldval = readl(priv->base + SMSTPCR(reg));
+			oldval = readl(priv->base + priv->control_regs[reg]);
 		newval = oldval & ~mask;
 		newval |= priv->smstpcr_saved[reg].val & mask;
 		if (newval == oldval)
 			continue;
 
 		if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A) {
-			writeb(newval, priv->base + STBCR(reg));
+			writeb(newval, priv->base + priv->control_regs[reg]);
 			/* dummy read to ensure write has completed */
-			readb(priv->base + STBCR(reg));
-			barrier_data(priv->base + STBCR(reg));
+			readb(priv->base + priv->control_regs[reg]);
+			barrier_data(priv->base + priv->control_regs[reg]);
 			continue;
 		} else
-			writel(newval, priv->base + SMSTPCR(reg));
+			writel(newval, priv->base + priv->control_regs[reg]);
 
 		/* Wait until enabled clocks are really enabled */
 		mask &= ~priv->smstpcr_saved[reg].val;
@@ -880,7 +875,7 @@  static int cpg_mssr_resume_noirq(struct device *dev)
 			continue;
 
 		for (i = 1000; i > 0; --i) {
-			oldval = readl(priv->base + MSTPSR(reg));
+			oldval = readl(priv->base + priv->status_regs[reg]);
 			if (!(oldval & mask))
 				break;
 			cpu_relax();
@@ -939,6 +934,15 @@  static int __init cpg_mssr_common_init(struct device *dev,
 	priv->last_dt_core_clk = info->last_dt_core_clk;
 	RAW_INIT_NOTIFIER_HEAD(&priv->notifiers);
 	priv->reg_layout = info->reg_layout;
+	if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN2_AND_GEN3) {
+		priv->status_regs = mstpsr;
+		priv->control_regs = smstpcr;
+	} else if (priv->reg_layout == CLK_REG_LAYOUT_RZ_A) {
+		priv->control_regs = stbcr;
+	} else {
+		error = -EINVAL;
+		goto out_err;
+	}
 
 	for (i = 0; i < nclks; i++)
 		priv->clks[i] = ERR_PTR(-ENOENT);