Re: [PATCH v4 5/7] serial: exar: add CTI cards to exar_get_nr_ports

From: Ilpo Järvinen
Date: Thu Apr 18 2024 - 07:43:46 EST


On Wed, 17 Apr 2024, Parker Newman wrote:

> From: Parker Newman <pnewman@xxxxxxxxxxxxxxx>
>
> Add code for getting number of ports of CTI cards to
> exar_get_nr_ports().
>
> Signed-off-by: Parker Newman <pnewman@xxxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - moved to separate patch
> - added spaces to single line comments
>
> drivers/tty/serial/8250/8250_exar.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index e68029a59122..197f45e306ff 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -711,12 +711,28 @@ static unsigned int exar_get_nr_ports(struct exar8250_board *board,
> {
> unsigned int nr_ports = 0;
>
> - if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO)
> + if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO) {

You can add the braces while you moved the code around so you don't need
to play with them again here and this patch can be more to the point.

> nr_ports = BIT(((pcidev->device & 0x38) >> 3) - 1);
> - else if (board->num_ports)
> + } else if (board->num_ports > 0) {
> + // Check if board struct overrides number of ports
> nr_ports = board->num_ports;

The comment just tells what the code does, IMO that comment doesn't add
any value.

> - else
> + } else if (pcidev->vendor == PCI_VENDOR_ID_EXAR) {
> + // Exar encodes # ports in last nibble of PCI Device ID ex. 0358

This comment you can also add while you moved the code around (or make
another patch out of it after moving).

--
i.

> nr_ports = pcidev->device & 0x0f;
> + } else if (pcidev->vendor == PCI_VENDOR_ID_CONNECT_TECH) {
> + // Handle CTI FPGA cards
> + switch (pcidev->device) {
> + case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X:
> + case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X:
> + nr_ports = 12;
> + break;
> + case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16:
> + nr_ports = 16;
> + break;
> + default:
> + break;
> + }
> + }
>
> return nr_ports;
> }
> --
> 2.43.2
>
>