Re: [PATCH] gpio: sifive: use the correct register to read output values

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Tue, Feb 08, 2022 at 10:44:41AM +0100, Bartosz Golaszewski wrote:
> On Sat, Feb 5, 2022 at 12:33 AM Damien Le Moal
> <damien.lemoal@opensource.wdc.com> wrote:
> >
> > On 2/4/22 22:02, Niklas Cassel wrote:
> > > From: Niklas Cassel <niklas.cassel@wdc.com>
> > >
> > > Setting the output of a GPIO to 1 using gpiod_set_value(), followed by
> > > reading the same GPIO using gpiod_get_value(), will currently yield an
> > > incorrect result.
> > >
> > > This is because the SiFive GPIO device stores the output values in reg_set,
> > > not reg_dat.
> > >
> > > Supply the flag BGPIOF_READ_OUTPUT_REG_SET to bgpio_init() so that the
> > > generic driver reads the correct register.
> > >
> > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> >
> > This probably needs a Fixes tag. I have not checked which patch though.
> >
> 
> I added the tag and queued this for fixes, thanks!

Hello Bart,

Thank you for queueing and amending the Fixes tag!


Kind regards,
Niklas


> 
> Bart
> 
> > > ---
> > > The patch was tested on a canaan,k210 board (canaan,k210-gpiohs compatible
> > > string). It would be nice with a Tested-by from someone with a SiFive board.
> > >
> > > However, the u-boot driver for this device already behaves exactly the same
> > > as this driver does after my patch, for all platforms using the driver.
> > >
> > >  drivers/gpio/gpio-sifive.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpio-sifive.c b/drivers/gpio/gpio-sifive.c
> > > index 403f9e833d6a..7d82388b4ab7 100644
> > > --- a/drivers/gpio/gpio-sifive.c
> > > +++ b/drivers/gpio/gpio-sifive.c
> > > @@ -223,7 +223,7 @@ static int sifive_gpio_probe(struct platform_device *pdev)
> > >                        NULL,
> > >                        chip->base + SIFIVE_GPIO_OUTPUT_EN,
> > >                        chip->base + SIFIVE_GPIO_INPUT_EN,
> > > -                      0);
> > > +                      BGPIOF_READ_OUTPUT_REG_SET);
> > >       if (ret) {
> > >               dev_err(dev, "unable to init generic GPIO\n");
> > >               return ret;
> >
> >
> > --
> > Damien Le Moal
> > Western Digital Research