linux-kernel - Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support

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, 30 Jan 2017 16:08:01 +0100
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Alexandre Courbot <gnurou@...il.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] gpio: mcp23s08: add pinconf support

On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@...nel.org> wrote:

> mcp23xxx device have configurable 100k pullup resistors. This adds
> support for enabling them using pinctrl's pinconf interface.
>
> Signed-off-by: Sebastian Reichel <sre@...nel.org>

That's the right approach!

> diff --git a/drivers/gpio/gpio-mcp23s08.c b/drivers/gpio/gpio-mcp23s08.c

Can we move the patch to drivers/pinctrl/* like all other mixed drivers
doing combined pinctrl and GPIO?

Also: no Kconfig changes? Surely you must select the right pinctrl
things, I guess you're just lucky that some other pin controller on your
system selects your infrastructure. I think that is why the build robot
is complaining.

> +static int mcp_update_cache(struct mcp23s08 *mcp)
> +{
> +       int ret, reg, i;
> +
> +       for (i = 0; i < ARRAY_SIZE(mcp->cache); i++) {
> +               ret = mcp_read(mcp, i, &reg);
> +               if (ret < 0)
> +                       return ret;
> +               mcp->cache[i] = reg;
> +       }
> +
> +       return 0;
> +}

Why do you need a cache of register values when regmap already
supports this?

Instead I suggest: exploit the .volatile_reg() callback from
struct regmap_config and use regmap to maintain the cache.

Apart from this is is nice!

With the recent changes from Mika Westerberg scheduled for v4.11
the road is open to expose pullups all the way to userspace from
GPIOs chardev if we want to (some patches would be needed).

Yours,
Linus Walleij