linux-kernel - Re: [PATCH 3/3] gpio: syscon: Add support for the Xylon LogiCVC GPIOs

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]

Date:   Mon, 23 Sep 2019 15:33:25 +0200
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 3/3] gpio: syscon: Add support for the Xylon LogiCVC GPIOs

Hi,

On Thu 12 Sep 19, 10:17, Linus Walleij wrote:
> On Tue, Sep 10, 2019 at 4:29 PM Paul Kocialkowski
> <paul.kocialkowski@...tlin.com> wrote:
> 
> > The LogiCVC display hardware block comes with GPIO capabilities
> > that must be exposed separately from the main driver (as GPIOs) for
> > use with regulators and panels. A syscon is used to share the same
> > regmap across the two drivers.
> >
> > Since the GPIO capabilities are pretty simple, add them to the syscon
> > GPIO driver.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> 
> I'm fine with this for now, but the gpio-syscon driver is now growing
> big and when you use it you are getting support for a whole bunch
> of systems you're not running on included in your binary.
> 
> We need to think about possibly creating drivers/gpio/syscon
> and split subdrivers into separate files and config options
> so that people can slim down to what they actually need.

Thanks for the review!

I understand your concern about more devices getting pulled-in and built
unconditionally. Though I do like the idea of having a single driver for only
exposing the GPIO part of bigger components instead of specific drivers with
< 100 lines of useful code.

Maybe a first step would be to introduce Kconfig options for each device and
ifdef around in the code, as to solve the "built unconditionally" aspect?

Either way, I'll send v2 still adding my driver to gpio-syscon but feel free to
have me in the loop when that driver needs to be changed.

> > +       *bit = 1 << offset;
> 
> Please do this:
> 
> #include <linux/bits.h>
> 
> *bit = BIT(offset);

Sure thing and sorry I missed that, thanks!

Cheers,

Paul

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

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