| Message ID | eef5ee23-311a-4367-43b5-b793357f06ef@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Renesas R8A77980 CPG/MSSR RPC clock support | expand |
On Tue, Dec 04, 2018 at 10:49:44PM +0300, Sergei Shtylyov wrote: > Protect the CPG register read-modify-write sequence with a spinlock. Spinlocks are expensive, I think an explanation of why this is necessary would be worthwhile. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > Changes in version 2: > - new patch. > > drivers/clk/renesas/rcar-gen3-cpg.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > Index: renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c > =================================================================== > --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.c > +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c > @@ -30,14 +30,19 @@ > > #define CPG_RCKCR_CKSEL BIT(15) /* RCLK Clock Source Select */ > > +static spinlock_t cpg_lock; > + > static void cpg_reg_modify(void __iomem *reg, u32 clear, u32 set) > { > + unsigned long flags; > u32 val; > > + spin_lock_irqsave(&cpg_lock, flags); > val = readl(reg); > val &= ~clear; > val |= set; > writel(val, reg); > + spin_unlock_irqrestore(&cpg_lock, flags); > }; > > struct cpg_simple_notifier { > @@ -604,5 +609,8 @@ int __init rcar_gen3_cpg_init(const stru > if (attr) > cpg_quirks = (uintptr_t)attr->data; > pr_debug("%s: mode = 0x%x quirks = 0x%x\n", __func__, mode, cpg_quirks); > + > + spin_lock_init(&cpg_lock); > + > return 0; > } >
Hi Simon, On Fri, Dec 14, 2018 at 4:33 PM Simon Horman <horms@verge.net.au> wrote: > On Tue, Dec 04, 2018 at 10:49:44PM +0300, Sergei Shtylyov wrote: > > Protect the CPG register read-modify-write sequence with a spinlock. > > Spinlocks are expensive, I think an explanation of why this is > necessary would be worthwhile. Some clock registers contain bits for multiple clocks. Hence read-modify-write cycles on these registers should be protected against concurrent accesses for multiple clocks. Gr{oetje,eeting}s, Geert
On Tue, Dec 4, 2018 at 8:49 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Protect the CPG register read-modify-write sequence with a spinlock. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On 12/14/2018 06:33 PM, Simon Horman wrote: > On Tue, Dec 04, 2018 at 10:49:44PM +0300, Sergei Shtylyov wrote: >> Protect the CPG register read-modify-write sequence with a spinlock. > > Spinlocks are expensive, I think an explanation of why this is > necessary would be worthwhile. The driver using a spinlock to protect the hardware registers is such a common scenario, I didn't think it was worth a special explanation. :-) > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> [...] MBR, Sergei
Index: renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c =================================================================== --- renesas-drivers.orig/drivers/clk/renesas/rcar-gen3-cpg.c +++ renesas-drivers/drivers/clk/renesas/rcar-gen3-cpg.c @@ -30,14 +30,19 @@ #define CPG_RCKCR_CKSEL BIT(15) /* RCLK Clock Source Select */ +static spinlock_t cpg_lock; + static void cpg_reg_modify(void __iomem *reg, u32 clear, u32 set) { + unsigned long flags; u32 val; + spin_lock_irqsave(&cpg_lock, flags); val = readl(reg); val &= ~clear; val |= set; writel(val, reg); + spin_unlock_irqrestore(&cpg_lock, flags); }; struct cpg_simple_notifier { @@ -604,5 +609,8 @@ int __init rcar_gen3_cpg_init(const stru if (attr) cpg_quirks = (uintptr_t)attr->data; pr_debug("%s: mode = 0x%x quirks = 0x%x\n", __func__, mode, cpg_quirks); + + spin_lock_init(&cpg_lock); + return 0; }
Protect the CPG register read-modify-write sequence with a spinlock. Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 2: - new patch. drivers/clk/renesas/rcar-gen3-cpg.c | 8 ++++++++ 1 file changed, 8 insertions(+)