2024-04-17 20:44:12

by Parker Newman

Subject: [PATCH v4 2/7] serial: exar: added a exar_get_nr_ports function

From: Parker Newman <[email protected]>

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 <[email protected]>
---
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;
+
+ 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;
+ }

priv = devm_kzalloc(&pcidev->dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
if (!priv)
--
2.43.2

2024-04-18 11:32:16

by Ilpo Järvinen

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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>

--
i.