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

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Fri, Jan 27, 2017 at 3:47 PM, Sebastian Reichel <sre@xxxxxxxxxx> 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@xxxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html