RE: [PATCH v2 2/5] gpio: loongson1: Use readl() & writel()

[David Laight <David.Laight@ACULAB.COM>]

From: Keguang Zhang
> Sent: 07 March 2023 03:46
> 
> On Mon, Mar 6, 2023 at 5:30 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
> > >
> > > This patch replace __raw_readl() & __raw_writel() with readl() & writel().
> > >
> >
> > Please say WHY you're doing this.
> >
> readl & writel contain memory barriers which can guarantee access order.

So what...

There is a data dependency between the read and write.
The read can't be scheduled before the lock is acquired.
The write can't be scheduled after the lock is released.

So any barriers in readl()/writel() aren't needed.

If they are only compile barriers they'll have no real effect.
OTOH if the cpu needs actual synchronising instructions (as some
ppc do) then they will slow things down.

	David

> 
> > Bart
> >
> > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > > ---
> > > V1 -> V2: Split this change to a separate patch
> > > ---
> > >  drivers/gpio/gpio-loongson1.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-loongson1.c b/drivers/gpio/gpio-loongson1.c
> > > index 8862c9ea0d41..b6c11caa3ade 100644
> > > --- a/drivers/gpio/gpio-loongson1.c
> > > +++ b/drivers/gpio/gpio-loongson1.c
> > > @@ -23,8 +23,8 @@ static int ls1x_gpio_request(struct gpio_chip *gc, unsigned int offset)
> > >         unsigned long flags;
> > >
> > >         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> > > -       __raw_writel(__raw_readl(gpio_reg_base + GPIO_CFG) | BIT(offset),
> > > -                    gpio_reg_base + GPIO_CFG);
> > > +       writel(readl(gpio_reg_base + GPIO_CFG) | BIT(offset),
> > > +              gpio_reg_base + GPIO_CFG);
> > >         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > >
> > >         return 0;
> > > @@ -35,8 +35,8 @@ static void ls1x_gpio_free(struct gpio_chip *gc, unsigned int offset)
> > >         unsigned long flags;
> > >
> > >         raw_spin_lock_irqsave(&gc->bgpio_lock, flags);
> > > -       __raw_writel(__raw_readl(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> > > -                    gpio_reg_base + GPIO_CFG);
> > > +       writel(readl(gpio_reg_base + GPIO_CFG) & ~BIT(offset),
> > > +              gpio_reg_base + GPIO_CFG);
> > >         raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > >  }
> > >
> > > --
> > > 2.34.1
> > >
> 
> 
> 
> --
> Best regards,
> 
> Kelvin Cheung

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)