Re: [PATCH v1 25/31] serial: 8250_mxpcie: add basic GPIO helper functions

[Andy Shevchenko]

On Sun, Nov 30, 2025 at 12:45 PM Crescent Hsieh
<crescentcy.hsieh@xxxxxxxx> wrote:
>
> Introduce a set of helper functions for accessing the on-board GPIO
> registers on Moxa PCIe serial devices. These cover:
>
> - Initializing all pins as outputs
> - Setting the direction of individual pins
> - Setting or clearing an output pin
> - Bulk set/get operations on OUTPUT and DIRECTION registers

> These functions do not change any existing behavior yet. They are added
> as a preparation step for follow-up patches that will make use of the
> GPIOs to control board-specific signals.

Then do not add them. We do not add the code without users. It might
be better for review (or worse ;-) but at the end of the day it needs
to be squashed with the user.

...

> +/**
> + * mxpcie8250_gpio_init() - GPIO initialization routine
> + * @iobar_addr:        The base address of the GPIO I/O region
> + *
> + * Initializes the GPIO direction. After calling this function, all GPIO
> + * pins will be set to output.

This is quite dangerous! Why not input? At bare minimum this needs a
good comment to explain why it's not a problem.

> + */
> +static void mxpcie8250_gpio_init(resource_size_t iobar_addr)
> +{
> +       /* Initialize all the GPIO pins into output state */
> +       outb(0xff, iobar_addr + MOXA_GPIO_DIRECTION);
> +}

...

Why is this not a drivers/gpio/gpio-moxa.c how it's done for Exar, for example?

-- 
With Best Regards,
Andy Shevchenko