On 14.09.2023 16:12, Geert Uytterhoeven wrote:

Hi Claudiu,

On Tue, Sep 12, 2023 at 6:52 AM Claudiu [off-list ref] wrote:

quoted

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

spinlock in rzg2l_mod_clock_endisable() is intended to protect the accesses
to hardware register. There is no need to protect the instructions that set
temporary variable which will be then written to register. Thus limit the
spinlock only to the hardware register access.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

quoted

--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c

@@ -912,13 +912,13 @@ static int rzg2l_mod_clock_endisable(struct clk_hw *hw, bool enable)

        dev_dbg(dev, "CLK_ON %u/%pC %s\n", CLK_ON_R(reg), hw->clk,
                enable ? "ON" : "OFF");
-       spin_lock_irqsave(&priv->rmw_lock, flags);

        value = bitmask << 16;
        if (enable)
                value |= bitmask;
-       writel(value, priv->base + CLK_ON_R(reg));

+       spin_lock_irqsave(&priv->rmw_lock, flags);
+       writel(value, priv->base + CLK_ON_R(reg));
        spin_unlock_irqrestore(&priv->rmw_lock, flags);

After this, it becomes obvious there is nothing to protect at all,
so the locking can just be removed from this function?

I tend to be paranoid when writing to hardware resources thus I kept it.
Would you prefer to remove it at all?

Gr{oetje,eeting}s,

                        Geert

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel