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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2d5521ff21ea4b99be3dd2e449f53934@AcuMS.aculab.com>
Date:   Wed, 8 Mar 2023 09:42:43 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Keguang Zhang' <keguang.zhang@...il.com>,
        Bartosz Golaszewski <brgl@...ev.pl>
CC:     "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>
Subject: RE: [PATCH v2 2/5] gpio: loongson1: Use readl() & writel()

From: Keguang Zhang
> Sent: 07 March 2023 03:46
> 
> On Mon, Mar 6, 2023 at 5:30 PM Bartosz Golaszewski <brgl@...ev.pl> wrote:
> >
> > On Thu, Mar 2, 2023 at 1:52 PM Keguang Zhang <keguang.zhang@...il.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@...il.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)