linux-kernel - Re: [PATCH v4 2/7] serial: exar: added a 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]

Date: Thu, 18 Apr 2024 14:32:01 +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>, LKML <linux-kernel@...r.kernel.org>, 
    linux-serial <linux-serial@...r.kernel.org>, 
    Parker Newman <pnewman@...necttech.com>
Subject: Re: [PATCH v4 2/7] serial: exar: added a exar_get_nr_ports
 function

On Wed, 17 Apr 2024, Parker Newman wrote:

> From: Parker Newman <pnewman@...necttech.com>
> 
> Moved code for getting number of ports from exar_pci_probe() to a
> separate exar_get_nr_ports() function. CTI specific code will be added
> in another patch in this series.
> 
> Signed-off-by: Parker Newman <pnewman@...necttech.com>
> ---
> Changes in v3:
> - Only moved existing code in this patch, will add CTI code in subsequent
>   patch
> 
>  drivers/tty/serial/8250/8250_exar.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> index 04ce5e8ddb24..72385c7d2eda 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -704,6 +704,21 @@ static irqreturn_t exar_misc_handler(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
> 
> +static unsigned int exar_get_nr_ports(struct exar8250_board *board,
> +					struct pci_dev *pcidev)
> +{
> +	unsigned int nr_ports = 0;

It's always set, so no need to initialize.

> +	if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO)
> +		nr_ports = BIT(((pcidev->device & 0x38) >> 3) - 1);
> +	else if (board->num_ports)
> +		nr_ports = board->num_ports;
> +	else
> +		nr_ports = pcidev->device & 0x0f;
> +
> +	return nr_ports;
> +}
> +
>  static int
>  exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
>  {
> @@ -723,12 +738,12 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
> 
>  	maxnr = pci_resource_len(pcidev, bar) >> (board->reg_shift + 3);
> 
> -	if (pcidev->vendor == PCI_VENDOR_ID_ACCESSIO)
> -		nr_ports = BIT(((pcidev->device & 0x38) >> 3) - 1);
> -	else if (board->num_ports)
> -		nr_ports = board->num_ports;
> -	else
> -		nr_ports = pcidev->device & 0x0f;
> +	nr_ports = exar_get_nr_ports(board, pcidev);
> +	if (nr_ports == 0) {
> +		dev_err_probe(&pcidev->dev, -ENODEV,
> +				"failed to get number of ports\n");
> +		return -ENODEV;

		return dev_err_probe(&pcidev->dev, -ENODEV,
				     "failed to get number of ports\n");

Other than those two small things,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

-- 
 i.