linux-kernel - Re: [PATCH v4 5/7] serial: exar: add CTI cards to exar_get_nr

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: <60790bd3-d35a-026d-ef38-ae3b12160fbe@linux.intel.com>
Date: Thu, 18 Apr 2024 14:43:32 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Parker Newman <parker@...est.io>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    Jiri Slaby <jirislaby@...nel.org>, linux-kernel@...r.kernel.org, 
    linux-serial@...r.kernel.org, Parker Newman <pnewman@...necttech.com>
Subject: Re: [PATCH v4 5/7] serial: exar: add CTI cards to
 exar_get_nr_ports

On Wed, 17 Apr 2024, Parker Newman wrote:

> From: Parker Newman <pnewman@...necttech.com>
> 
> Add code for getting number of ports of CTI cards to
> exar_get_nr_ports().
> 
> Signed-off-by: Parker Newman <pnewman@...necttech.com>
> ---
> 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
> 
>