linux-kernel - Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get operation

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 3 Oct 2019 07:26:10 -0400
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Bartosz Golaszewski <bgolaszewski@...libre.com>
Cc:     linux-gpio <linux-gpio@...r.kernel.org>,
        linux-devicetree <devicetree@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Lee Jones <lee.jones@...aro.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v3 3/5] gpio: syscon: Add support for a custom get
 operation

Hi,

On Thu 03 Oct 19, 10:24, Bartosz Golaszewski wrote:
> pt., 27 wrz 2019 o 12:04 Paul Kocialkowski
> <paul.kocialkowski@...tlin.com> napisał(a):
> >
> > Some drivers might need a custom get operation to match custom
> > behavior implemented in the set operation.
> >
> > Add plumbing for supporting that.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> > ---
> >  drivers/gpio/gpio-syscon.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-syscon.c b/drivers/gpio/gpio-syscon.c
> > index 31f332074d7d..05c537ed73f1 100644
> > --- a/drivers/gpio/gpio-syscon.c
> > +++ b/drivers/gpio/gpio-syscon.c
> > @@ -43,8 +43,9 @@ struct syscon_gpio_data {
> >         unsigned int    bit_count;
> >         unsigned int    dat_bit_offset;
> >         unsigned int    dir_bit_offset;
> > -       void            (*set)(struct gpio_chip *chip,
> > -                              unsigned offset, int value);
> > +       int             (*get)(struct gpio_chip *chip, unsigned offset);
> > +       void            (*set)(struct gpio_chip *chip, unsigned offset,
> > +                              int value);
> 
> Why did you change this line? Doesn't seem necessary and pollutes the history.

This is for consistency since both the "chip" and "offset" arguments can fit
in a single line. Since I want the "get" addition to fit in a single line,
bringing back "offset" on the previous line of "set" makes things consistent.
There's probably no particular reason for the split in the first place.

Do you think it needs a separate cosmetic commit only for that?
I'd rather add a note in the commit message and keep the change as-is.

Cheers,

Paul

> Bart
> 
> >  };
> >
> >  struct syscon_gpio_priv {
> > @@ -252,7 +253,7 @@ static int syscon_gpio_probe(struct platform_device *pdev)
> >         priv->chip.label = dev_name(dev);
> >         priv->chip.base = -1;
> >         priv->chip.ngpio = priv->data->bit_count;
> > -       priv->chip.get = syscon_gpio_get;
> > +       priv->chip.get = priv->data->get ? : syscon_gpio_get;
> >         if (priv->data->flags & GPIO_SYSCON_FEAT_IN)
> >                 priv->chip.direction_input = syscon_gpio_dir_in;
> >         if (priv->data->flags & GPIO_SYSCON_FEAT_OUT) {
> > --
> > 2.23.0
> >

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)