Date:   Mon, 3 Sep 2018 17:07:44 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Janusz Krzysztofik <jmkrzyszt@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Jonathan Corbet <corbet@....net>,
        Miguel Ojeda Sandonis <miguel.ojeda.sandonis@...il.com>,
        peter.korsgaard@...co.com, Peter Rosin <peda@...ntia.se>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Greg KH <gregkh@...uxfoundation.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Peter Meerwald <pmeerw@...erw.net>,
        Jiri Slaby <jslaby@...e.com>, Willy Tarreau <w@....eu>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux I2C <linux-i2c@...r.kernel.org>,
        Linux MMC List <linux-mmc@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>, linux-iio@...r.kernel.org,
        driverdevel <devel@...verdev.osuosl.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        sebastien.bourdelin@...oirfairelinux.com,
        Lukas Wunner <lukas@...ner.de>,
        Rojhalat Ibrahim <imr@...chenk.de>,
        Russell King <rmk+kernel@...linux.org.uk>,
        ext Tony Lindgren <tony@...mide.com>,
        Yegor Yefremov <yegorslists@...glemail.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH v7 1/4] gpiolib: Pass bitmaps, not integer arrays, to
 get/set array

Hi Janusz,

On Sun, Sep 2, 2018 at 2:01 PM Janusz Krzysztofik <jmkrzyszt@...il.com> wrote:
> Most users of get/set array functions iterate consecutive bits of data,
> usually a single integer, while processing array of results obtained
> from, or building an array of values to be passed to those functions.
> Save time wasted on those iterations by changing the functions' API to
> accept bitmaps.
>
> All current users are updated as well.
>
> More benefits from the change are expected as soon as planned support
> for accepting/passing those bitmaps directly from/to respective GPIO
> chip callbacks if applicable is implemented.
>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@...il.com>
> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: Sebastien Bourdelin <sebastien.bourdelin@...oirfairelinux.com>
> Cc: Lukas Wunner <lukas@...ner.de>
> Cc: Peter Korsgaard <peter.korsgaard@...co.com>
> Cc: Peter Rosin <peda@...ntia.se>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Rojhalat Ibrahim <imr@...chenk.de>
> Cc: Dominik Brodowski <linux@...inikbrodowski.net>
> Cc: Russell King <rmk+kernel@...linux.org.uk>
> Cc: Kishon Vijay Abraham I <kishon@...com>
> Cc: Tony Lindgren <tony@...mide.com>
> Cc: Lars-Peter Clausen <lars@...afoo.de>
> Cc: Michael Hennerich <Michael.Hennerich@...log.com>
> Cc: Jonathan Cameron <jic23@...nel.org>
> Cc: Hartmut Knaack <knaack.h@....de>
> Cc: Peter Meerwald-Stadler <pmeerw@...erw.net>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Jiri Slaby <jslaby@...e.com>
> Cc: Yegor Yefremov <yegorslists@...glemail.com>
> Cc: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@...il.com>
> Acked-by: Ulf Hansson <ulf.hansson@...aro.org>

> --- a/drivers/auxdisplay/hd44780.c
> +++ b/drivers/auxdisplay/hd44780.c
> @@ -62,17 +62,12 @@ static void hd44780_strobe_gpio(struct hd44780 *hd)
>  /* write to an LCD panel register in 8 bit GPIO mode */
>  static void hd44780_write_gpio8(struct hd44780 *hd, u8 val, unsigned int rs)
>  {
> -       int values[10]; /* for DATA[0-7], RS, RW */
> -       unsigned int i, n;
> -
> -       for (i = 0; i < 8; i++)
> -               values[PIN_DATA0 + i] = !!(val & BIT(i));
> -       values[PIN_CTRL_RS] = rs;
> -       n = 9;
> -       if (hd->pins[PIN_CTRL_RW]) {
> -               values[PIN_CTRL_RW] = 0;
> -               n++;
> -       }
> +       DECLARE_BITMAP(values, 10); /* for DATA[0-7], RS, RW */
> +       unsigned int n;
> +
> +       *values = val;

Given DECLARE_BITMAP() creates an array, the above line looks a bit funny now.

IMHO, either you use

    unsigned long values;
    values = val;
    __assign_bit(8, &values, rs);

or

    DECLARE_BITMAP(values, 10);
    values[0] = val;
    __assign_bit(8, values, rs);

Nevertheless, for hd44780.c:
Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists