[PATCH v3 4/6] clk: renesas: rzv2h: Switch MSTOP configuration to per-bit basis

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>

Switch MSTOP handling from group-based to per-bit configuration to
address issues with shared dependencies between module clocks. In the
current group-based configuration, multiple module clocks may rely on
a single MSTOP bit. When both clocks are turned ON and one is
subsequently turned OFF, the shared MSTOP bit will still be set, which
is incorrect since the other dependent module clock remains ON. By
switching to a per-bit configuration, we ensure precise control over
individual MSTOP bits, preventing such conflicts.

Replace the refcount API with atomic operations for managing MSTOP bit
counters. The refcount API requires explicitly setting the counter to
`1` before calling `refcount_inc()`, which introduces potential edge
cases and unnecessary complexity. Using atomic operations simplifies
the logic and avoids such issues, resulting in cleaner and more
maintainable code.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
---
v2->v3
- Dropped unnecessary parentheses
- Switched using to devm_kcalloc() instead of devm_kmalloc_array()
- Optimized check in rzv2h_mod_clock_endisable() if the states are same
- Dropped GET_MSTOP_IDX() macro and handled indexing in the code
- Made mstop_mask to unsigned long to avoid casting

v1->v2
- None
---
 drivers/clk/renesas/r9a09g047-cpg.c |   2 +
 drivers/clk/renesas/r9a09g057-cpg.c |   2 +
 drivers/clk/renesas/rzv2h-cpg.c     | 186 ++++++++++++++--------------
 drivers/clk/renesas/rzv2h-cpg.h     |   5 +
 4 files changed, 104 insertions(+), 91 deletions(-)

diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
index 7945b9f95b95..536d922bed70 100644
--- a/drivers/clk/renesas/r9a09g047-cpg.c
+++ b/drivers/clk/renesas/r9a09g047-cpg.c
@@ -145,4 +145,6 @@ const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
 	/* Resets */
 	.resets = r9a09g047_resets,
 	.num_resets = ARRAY_SIZE(r9a09g047_resets),
+
+	.num_mstop_bits = 208,
 };
diff --git a/drivers/clk/renesas/r9a09g057-cpg.c b/drivers/clk/renesas/r9a09g057-cpg.c
index 59dadedb2217..a45b4020996b 100644
--- a/drivers/clk/renesas/r9a09g057-cpg.c
+++ b/drivers/clk/renesas/r9a09g057-cpg.c
@@ -275,4 +275,6 @@ const struct rzv2h_cpg_info r9a09g057_cpg_info __initconst = {
 	/* Resets */
 	.resets = r9a09g057_resets,
 	.num_resets = ARRAY_SIZE(r9a09g057_resets),
+
+	.num_mstop_bits = 192,
 };
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 23fb209d3232..a4c1e92e1fd7 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -68,6 +68,7 @@
  * @resets: Array of resets
  * @num_resets: Number of Module Resets in info->resets[]
  * @last_dt_core_clk: ID of the last Core Clock exported to DT
+ * @mstop_count: Array of mstop values
  * @rcdev: Reset controller entity
  */
 struct rzv2h_cpg_priv {
@@ -82,17 +83,13 @@ struct rzv2h_cpg_priv {
 	unsigned int num_resets;
 	unsigned int last_dt_core_clk;
 
+	atomic_t *mstop_count;
+
 	struct reset_controller_dev rcdev;
 };
 
 #define rcdev_to_priv(x)	container_of(x, struct rzv2h_cpg_priv, rcdev)
 
-struct rzv2h_mstop {
-	u16 idx;
-	u16 mask;
-	refcount_t ref_cnt;
-};
-
 struct pll_clk {
 	struct rzv2h_cpg_priv *priv;
 	void __iomem *base;
@@ -107,7 +104,7 @@ struct pll_clk {
  * struct mod_clock - Module clock
  *
  * @priv: CPG private data
- * @mstop: handle to cpg bus mstop data
+ * @mstop_data: mstop data relating to module clock
  * @hw: handle between common and hardware-specific interfaces
  * @no_pm: flag to indicate PM is not supported
  * @on_index: register offset
@@ -117,7 +114,7 @@ struct pll_clk {
  */
 struct mod_clock {
 	struct rzv2h_cpg_priv *priv;
-	struct rzv2h_mstop *mstop;
+	unsigned int mstop_data;
 	struct clk_hw hw;
 	bool no_pm;
 	u8 on_index;
@@ -446,38 +443,70 @@ rzv2h_cpg_register_core_clk(const struct cpg_core_clk *core,
 }
 
 static void rzv2h_mod_clock_mstop_enable(struct rzv2h_cpg_priv *priv,
-					 struct mod_clock *clock)
+					 u32 mstop_data)
 {
+	unsigned long mstop_mask = FIELD_GET(BUS_MSTOP_BITS_MASK, mstop_data);
+	u16 mstop_index = FIELD_GET(BUS_MSTOP_IDX_MASK, mstop_data);
+	unsigned int index = (mstop_index - 1) * 16;
+	atomic_t *mstop = &priv->mstop_count[index];
 	unsigned long flags;
-	u32 val;
+	unsigned int i;
+	u32 val = 0;
 
 	spin_lock_irqsave(&priv->rmw_lock, flags);
-	if (!refcount_read(&clock->mstop->ref_cnt)) {
-		val = clock->mstop->mask << 16;
-		writel(val, priv->base + CPG_BUS_MSTOP(clock->mstop->idx));
-		refcount_set(&clock->mstop->ref_cnt, 1);
-	} else {
-		refcount_inc(&clock->mstop->ref_cnt);
+	for_each_set_bit(i, &mstop_mask, 16) {
+		if (!atomic_read(&mstop[i]))
+			val |= BIT(i) << 16;
+		atomic_inc(&mstop[i]);
 	}
+	if (val)
+		writel(val, priv->base + CPG_BUS_MSTOP(mstop_index));
 	spin_unlock_irqrestore(&priv->rmw_lock, flags);
 }
 
 static void rzv2h_mod_clock_mstop_disable(struct rzv2h_cpg_priv *priv,
-					  struct mod_clock *clock)
+					  u32 mstop_data)
 {
+	unsigned long mstop_mask = FIELD_GET(BUS_MSTOP_BITS_MASK, mstop_data);
+	u16 mstop_index = FIELD_GET(BUS_MSTOP_IDX_MASK, mstop_data);
+	unsigned int index = (mstop_index - 1) * 16;
+	atomic_t *mstop = &priv->mstop_count[index];
 	unsigned long flags;
-	u32 val;
+	unsigned int i;
+	u32 val = 0;
 
 	spin_lock_irqsave(&priv->rmw_lock, flags);
-	if (refcount_dec_and_test(&clock->mstop->ref_cnt)) {
-		val = clock->mstop->mask << 16 | clock->mstop->mask;
-		writel(val, priv->base + CPG_BUS_MSTOP(clock->mstop->idx));
+	for_each_set_bit(i, &mstop_mask, 16) {
+		if (!atomic_read(&mstop[i]) ||
+		    atomic_dec_and_test(&mstop[i]))
+			val |= BIT(i) << 16 | BIT(i);
 	}
+	if (val)
+		writel(val, priv->base + CPG_BUS_MSTOP(mstop_index));
 	spin_unlock_irqrestore(&priv->rmw_lock, flags);
 }
 
+static int rzv2h_mod_clock_is_enabled(struct clk_hw *hw)
+{
+	struct mod_clock *clock = to_mod_clock(hw);
+	struct rzv2h_cpg_priv *priv = clock->priv;
+	u32 bitmask;
+	u32 offset;
+
+	if (clock->mon_index >= 0) {
+		offset = GET_CLK_MON_OFFSET(clock->mon_index);
+		bitmask = BIT(clock->mon_bit);
+	} else {
+		offset = GET_CLK_ON_OFFSET(clock->on_index);
+		bitmask = BIT(clock->on_bit);
+	}
+
+	return readl(priv->base + offset) & bitmask;
+}
+
 static int rzv2h_mod_clock_endisable(struct clk_hw *hw, bool enable)
 {
+	bool enabled = rzv2h_mod_clock_is_enabled(hw);
 	struct mod_clock *clock = to_mod_clock(hw);
 	unsigned int reg = GET_CLK_ON_OFFSET(clock->on_index);
 	struct rzv2h_cpg_priv *priv = clock->priv;
@@ -489,15 +518,18 @@ static int rzv2h_mod_clock_endisable(struct clk_hw *hw, bool enable)
 	dev_dbg(dev, "CLK_ON 0x%x/%pC %s\n", reg, hw->clk,
 		enable ? "ON" : "OFF");
 
+	if (enabled == enable)
+		return 0;
+
 	value = bitmask << 16;
 	if (enable) {
 		value |= bitmask;
 		writel(value, priv->base + reg);
-		if (clock->mstop)
-			rzv2h_mod_clock_mstop_enable(priv, clock);
+		if (clock->mstop_data != BUS_MSTOP_NONE)
+			rzv2h_mod_clock_mstop_enable(priv, clock->mstop_data);
 	} else {
-		if (clock->mstop)
-			rzv2h_mod_clock_mstop_disable(priv, clock);
+		if (clock->mstop_data != BUS_MSTOP_NONE)
+			rzv2h_mod_clock_mstop_disable(priv, clock->mstop_data);
 		writel(value, priv->base + reg);
 	}
 
@@ -525,73 +557,12 @@ static void rzv2h_mod_clock_disable(struct clk_hw *hw)
 	rzv2h_mod_clock_endisable(hw, false);
 }
 
-static int rzv2h_mod_clock_is_enabled(struct clk_hw *hw)
-{
-	struct mod_clock *clock = to_mod_clock(hw);
-	struct rzv2h_cpg_priv *priv = clock->priv;
-	u32 bitmask;
-	u32 offset;
-
-	if (clock->mon_index >= 0) {
-		offset = GET_CLK_MON_OFFSET(clock->mon_index);
-		bitmask = BIT(clock->mon_bit);
-	} else {
-		offset = GET_CLK_ON_OFFSET(clock->on_index);
-		bitmask = BIT(clock->on_bit);
-	}
-
-	return readl(priv->base + offset) & bitmask;
-}
-
 static const struct clk_ops rzv2h_mod_clock_ops = {
 	.enable = rzv2h_mod_clock_enable,
 	.disable = rzv2h_mod_clock_disable,
 	.is_enabled = rzv2h_mod_clock_is_enabled,
 };
 
-static struct rzv2h_mstop
-*rzv2h_cpg_get_mstop(struct rzv2h_cpg_priv *priv, struct mod_clock *clock, u32 mstop_data)
-{
-	struct rzv2h_mstop *mstop;
-	unsigned int i;
-
-	for (i = 0; i < priv->num_mod_clks; i++) {
-		struct mod_clock *clk;
-		struct clk_hw *hw;
-
-		if (priv->clks[priv->num_core_clks + i] == ERR_PTR(-ENOENT))
-			continue;
-
-		hw = __clk_get_hw(priv->clks[priv->num_core_clks + i]);
-		clk = to_mod_clock(hw);
-		if (!clk->mstop)
-			continue;
-
-		if (BUS_MSTOP(clk->mstop->idx, clk->mstop->mask) == mstop_data) {
-			if (rzv2h_mod_clock_is_enabled(&clock->hw)) {
-				if (refcount_read(&clk->mstop->ref_cnt))
-					refcount_inc(&clk->mstop->ref_cnt);
-				else
-					refcount_set(&clk->mstop->ref_cnt, 1);
-			}
-			return clk->mstop;
-		}
-	}
-
-	mstop = devm_kzalloc(priv->dev, sizeof(*mstop), GFP_KERNEL);
-	if (!mstop)
-		return NULL;
-
-	mstop->idx = FIELD_GET(BUS_MSTOP_IDX_MASK, mstop_data);
-	mstop->mask = FIELD_GET(BUS_MSTOP_BITS_MASK, mstop_data);
-	if (rzv2h_mod_clock_is_enabled(&clock->hw))
-		refcount_set(&mstop->ref_cnt, 1);
-	else
-		refcount_set(&mstop->ref_cnt, 0);
-
-	return mstop;
-}
-
 static void __init
 rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 			   struct rzv2h_cpg_priv *priv)
@@ -638,6 +609,7 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 	clock->no_pm = mod->no_pm;
 	clock->priv = priv;
 	clock->hw.init = &init;
+	clock->mstop_data = mod->mstop_data;
 
 	ret = devm_clk_hw_register(dev, &clock->hw);
 	if (ret) {
@@ -647,12 +619,39 @@ rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
 
 	priv->clks[id] = clock->hw.clk;
 
-	if (mod->mstop_data != BUS_MSTOP_NONE) {
-		clock->mstop = rzv2h_cpg_get_mstop(priv, clock, mod->mstop_data);
-		if (!clock->mstop) {
-			clk = ERR_PTR(-ENOMEM);
-			goto fail;
+	/*
+	 * Ensure the module clocks and MSTOP bits are synchronized when they are
+	 * turned ON by the bootloader. Enable MSTOP bits for module clocks that were
+	 * turned ON in an earlier boot stage.
+	 */
+	if (clock->mstop_data != BUS_MSTOP_NONE &&
+	    !mod->critical && rzv2h_mod_clock_is_enabled(&clock->hw)) {
+		rzv2h_mod_clock_mstop_enable(priv, clock->mstop_data);
+	} else if (clock->mstop_data != BUS_MSTOP_NONE && mod->critical) {
+		unsigned long mstop_mask = FIELD_GET(BUS_MSTOP_BITS_MASK, clock->mstop_data);
+		u16 mstop_index = FIELD_GET(BUS_MSTOP_IDX_MASK, clock->mstop_data);
+		unsigned int index = (mstop_index - 1) * 16;
+		atomic_t *mstop = &priv->mstop_count[index];
+		unsigned long flags;
+		unsigned int i;
+		u32 val = 0;
+
+		/*
+		 * Critical clocks are turned ON immediately upon registration, and the
+		 * MSTOP counter is updated through the rzv2h_mod_clock_enable() path.
+		 * However, if the critical clocks were already turned ON by the initial
+		 * bootloader, synchronize the atomic counter here and clear the MSTOP bit.
+		 */
+		spin_lock_irqsave(&priv->rmw_lock, flags);
+		for_each_set_bit(i, &mstop_mask, 16) {
+			if (atomic_read(&mstop[i]))
+				continue;
+			val |= BIT(i) << 16;
+			atomic_inc(&mstop[i]);
 		}
+		if (val)
+			writel(val, priv->base + CPG_BUS_MSTOP(mstop_index));
+		spin_unlock_irqrestore(&priv->rmw_lock, flags);
 	}
 
 	return;
@@ -922,6 +921,11 @@ static int __init rzv2h_cpg_probe(struct platform_device *pdev)
 	if (!clks)
 		return -ENOMEM;
 
+	priv->mstop_count = devm_kcalloc(dev, info->num_mstop_bits,
+					 sizeof(*priv->mstop_count), GFP_KERNEL);
+	if (!priv->mstop_count)
+		return -ENOMEM;
+
 	priv->resets = devm_kmemdup(dev, info->resets, sizeof(*info->resets) *
 				    info->num_resets, GFP_KERNEL);
 	if (!priv->resets)
diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
index f918620c4650..a772304f9057 100644
--- a/drivers/clk/renesas/rzv2h-cpg.h
+++ b/drivers/clk/renesas/rzv2h-cpg.h
@@ -193,6 +193,9 @@ struct rzv2h_reset {
  *
  * @resets: Array of Module Reset definitions
  * @num_resets: Number of entries in resets[]
+ *
+ * @num_mstop_bits: Maximum number of MSTOP bits supported, equivalent to the
+ *		    number of CPG_BUS_m_MSTOP registers multiplied by 16.
  */
 struct rzv2h_cpg_info {
 	/* Core Clocks */
@@ -209,6 +212,8 @@ struct rzv2h_cpg_info {
 	/* Resets */
 	const struct rzv2h_reset *resets;
 	unsigned int num_resets;
+
+	unsigned int num_mstop_bits;
 };
 
 extern const struct rzv2h_cpg_info r9a09g047_cpg_info;
-- 
2.43.0





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux