Re: [PATCH v5 01/14] serial: 8250: split Moxa PCIe serial board support out of 8250_pci

[Andy Shevchenko]

On Fri, Jul 31, 2026 at 10:49 AM Crescent Hsieh
<crescentcy.hsieh@xxxxxxxx> wrote:
>
> The Moxa PCIe multiport serial boards are currently handled as part of
> 8250_pci.c. In preparation for adding Moxa-specific UART features and
> optimizations, move the Moxa PCIe implementation into a dedicated
> driver.
>
> This introduces drivers/tty/serial/8250/8250_mxpcie.c and wires it up
> via Kconfig and Makefile, while preserving the existing probe flow and
> device IDs.
>
> This change was suggested during earlier reviews by Andy Shevchenko [1][2].
>
> No functional change intended.

...


> +static unsigned int mxpcie8250_get_supp_rs(unsigned short device)
> +{
> +       switch (device & MOXA_DEV_ID_IFACE_MASK) {

For the sake of consistency this can be FIELD_GET() as well.

> +       case 0x0000:
> +       case 0x0600:
> +               return MOXA_SUPP_RS232;
> +       case 0x0100:
> +               return MOXA_SUPP_RS232 | MOXA_SUPP_RS422 | MOXA_SUPP_RS485;
> +       case 0x0300:
> +               return MOXA_SUPP_RS422 | MOXA_SUPP_RS485;
> +       default:
> +               return 0;
> +       }
> +}

...

> +static void mxpcie8250_init_board(struct pci_dev *pdev, struct mxpcie8250 *priv)
> +{
> +       void __iomem *bar2_base = priv->bar2_base;
> +       unsigned short device = pdev->device;
> +       u8 cval;
> +
> +       /* Initial terminator */
> +       if (device == PCI_DEVICE_ID_MOXA_CP114EL ||
> +           device == PCI_DEVICE_ID_MOXA_CP118EL_A) {
> +               iowrite8(0xff, bar2_base + MOXA_GPIO_DIRECTION);
> +               iowrite8(0x00, bar2_base + MOXA_GPIO_OUTPUT);
> +       }
> +       /*
> +        * Enable hardware buffer to prevent break signal output when system boots up.
> +        * This hardware buffer is only supported on Mini PCIe series.
> +        */
> +       if (mxpcie8250_is_mini_pcie(device)) {
> +               /* Set GPIO direction */
> +               cval = ioread8(bar2_base + MOXA_GPIO_DIRECTION);
> +               cval |= MOXA_GPIO_PIN2;

And FIELD_MODIFY() here...

> +               iowrite8(cval, bar2_base + MOXA_GPIO_DIRECTION);
> +               /* Enable low GPIO */
> +               cval = ioread8(bar2_base + MOXA_GPIO_OUTPUT);
> +               cval &= ~MOXA_GPIO_PIN2;

...and here.

> +               iowrite8(cval, bar2_base + MOXA_GPIO_OUTPUT);
> +       }
> +}

...

But I'm okay with this, the above can be amended later on.

-- 
With Best Regards,
Andy Shevchenko