Re: [PATCH 06/11] gpio: Add NVIDIA LSTP GPIO driver

[Adrian Ambrozewicz]

Hi Linus,

Thanks for the review.

On Wed, Oct 14, 2026 at 1:25 AM Linus Walleij <linusw@xxxxxxxxxx> wrote:
>> +config GPIO_LSTP
>
> GPIO_USBIO ... GPIO_LSTP?
>
> These should be in alphabetical order.

Will fix, and check the other Kconfig hunks in the series.

>> +#include <uapi/linux/gpio.h>
>
> Do you really need all this?
> Certainly <uapi/linux/gpio.h> is a mistake, look over all of these.

It was there for GPIO_MAX_NAME_SIZE, for a name truncation warning the
driver does not need. Both go, and I'll trim the rest of the includes
here and in the other drivers.

>> +/* clang-format off */
>
> I beg your pardon? What is this?

Markers that stopped clang-format from reflowing hand-formatted blocks.
They don't belong in posted code; they are gone in the next version.

> This  looks very dense too..
>
> At least a newline between the enums?

Will do.

>> +struct lstp_gpio_priv {
>
> Just call it lstp_gpio

Will do.

>> +       struct lstp_channel *ch;
>> +       struct gpio_chip gc;
>
> Why? Normally drivers get the state container from the
> chip not the other way around.

The gpio_chip callbacks do get it from the chip, with
gpiochip_get_data(). The channel pointer is the handle every request
to the device goes through. The only lookup the other way round is the
interrupt event callback: the LSTP core calls it with the channel, so
it takes the state from the channel's private pointer.

> It seems some pins are only input and some are only output.
>
> Have you considered using serialized register access over
> regmap and use regmap-gpio with fixed direction bitmaps?

Yes, the device configuration fixes each pin's direction. I looked at
gpio-regmap: a regmap bus with one register per pin would cover get,
set and the fixed directions. Two parts don't fit well, though.
gpio-regmap has no get_multiple/set_multiple, while the device takes
several pins in one request, so an array access would become one USB
round trip per pin. And interrupts arrive as event packets the device
sends on its own, not as status registers to read, so regmap-irq does
not apply and the irq_chip would stay as it is.

What I would take from it is reading the directions once at probe into
bitmaps, instead of asking the device on every call. Would that be
acceptable, or would you still prefer gpio-regmap despite the lost
batching?

Thanks,
Adrian