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

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]

Message-ID: <CAHp75VdQ0mK_qJ9Cm1d+woWYFJVCJb0-Uwj1tKtG2QpW-7vQ1A@mail.gmail.com>
Date: Mon, 1 Dec 2025 04:13:04 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Crescent Hsieh <crescentcy.hsieh@...a.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, 
	ilpo.jarvinen@...ux.intel.com, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v1 25/31] serial: 8250_mxpcie: add basic GPIO helper functions

On Sun, Nov 30, 2025 at 12:45 PM Crescent Hsieh
<crescentcy.hsieh@...a.com> 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