From cdbc9d055fc7540e6a88ec2bdd7fee46e026156c Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:18 +0200 Subject: [PATCH 1/7] can: kvaser_pciefd: Group #defines together Increases readability Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-2-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index 7b5028b67cd5..fa205091aafe 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -29,10 +29,10 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices"); #define KVASER_PCIEFD_CAN_TX_MAX_COUNT 17U #define KVASER_PCIEFD_MAX_CAN_CHANNELS 8UL #define KVASER_PCIEFD_DMA_COUNT 2U - #define KVASER_PCIEFD_DMA_SIZE (4U * 1024U) #define KVASER_PCIEFD_VENDOR 0x1a07 + /* Altera based devices */ #define KVASER_PCIEFD_4HS_DEVICE_ID 0x000d #define KVASER_PCIEFD_2HS_V2_DEVICE_ID 0x000e From ac765219c2c4e44f29063724c8d36435a3e61985 Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:19 +0200 Subject: [PATCH 2/7] can: kvaser_pciefd: Skip redundant NULL pointer check in ISR This check is already done at the creation of the net devices in kvaser_pciefd_setup_can_ctrls called from kvaser_pciefd_probe. If it fails, the driver won't load, so there should be no need to repeat the check inside the ISR. The number of channels is read from the FPGA and should be trusted. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-3-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index fa205091aafe..4832a93d34de 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1701,12 +1701,6 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev) kvaser_pciefd_receive_irq(pcie); for (i = 0; i < pcie->nr_channels; i++) { - if (!pcie->can[i]) { - dev_err(&pcie->pci->dev, - "IRQ mask points to unallocated controller\n"); - break; - } - /* Check that mask matches channel (i) IRQ mask */ if (board_irq & irq_mask->kcan_tx[i]) kvaser_pciefd_transmit_irq(pcie->can[i]); From 11d186697ceb10b68c6a1fd505635346b1ccd055 Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:20 +0200 Subject: [PATCH 3/7] can: kvaser_pciefd: Remove unnecessary comment The code speaks for itself. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-4-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index 4832a93d34de..8c9abc702b24 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1701,7 +1701,6 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev) kvaser_pciefd_receive_irq(pcie); for (i = 0; i < pcie->nr_channels; i++) { - /* Check that mask matches channel (i) IRQ mask */ if (board_irq & irq_mask->kcan_tx[i]) kvaser_pciefd_transmit_irq(pcie->can[i]); } From 0132a05df1e0119286f51bf757759a7f8e6edaa5 Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:21 +0200 Subject: [PATCH 4/7] can: kvaser_pciefd: Add inline Make the short function kvaser_pciefd_set_tx_irq inline. This function is effectively three lines long and therefore inlining it should be OK according to rule #15 of the Linux kernel coding style. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-5-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index 8c9abc702b24..3d40d7b3d64c 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -550,7 +550,7 @@ static void kvaser_pciefd_disable_err_gen(struct kvaser_pciefd_can *can) spin_unlock_irqrestore(&can->lock, irq); } -static void kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can) +static inline void kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can) { u32 msk; From cebfebefaa012afbe49ad5e4d6bb97d573beb32d Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:22 +0200 Subject: [PATCH 5/7] can: kvaser_pciefd: Add unlikely Use unlikely for some unexpected errors. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-6-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index 3d40d7b3d64c..b08084b0a95b 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1619,7 +1619,7 @@ static int kvaser_pciefd_read_packet(struct kvaser_pciefd *pcie, int *start_pos, /* Position does not point to the end of the package, * corrupted packet size? */ - if ((*start_pos + size) != pos) + if (unlikely((*start_pos + size) != pos)) return -EIO; /* Point to the next packet header, if any */ @@ -1658,10 +1658,10 @@ static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie) KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CMD_REG); } - if (irq & KVASER_PCIEFD_SRB_IRQ_DOF0 || - irq & KVASER_PCIEFD_SRB_IRQ_DOF1 || - irq & KVASER_PCIEFD_SRB_IRQ_DUF0 || - irq & KVASER_PCIEFD_SRB_IRQ_DUF1) + if (unlikely(irq & KVASER_PCIEFD_SRB_IRQ_DOF0 || + irq & KVASER_PCIEFD_SRB_IRQ_DOF1 || + irq & KVASER_PCIEFD_SRB_IRQ_DUF0 || + irq & KVASER_PCIEFD_SRB_IRQ_DUF1)) dev_err(&pcie->pci->dev, "DMA IRQ error 0x%08X\n", irq); iowrite32(irq, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IRQ_REG); From cbf88a6ba7bb6ce0d3131b119298f73bd7b18459 Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:23 +0200 Subject: [PATCH 6/7] can: kvaser_pciefd: Rename board_irq to pci_irq Rename the variable name board_irq in the ISR to pci_irq to be more specific and to match the macro by which it is read. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-7-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index b08084b0a95b..8b2c18f2f23b 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1691,17 +1691,17 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev) { struct kvaser_pciefd *pcie = (struct kvaser_pciefd *)dev; const struct kvaser_pciefd_irq_mask *irq_mask = pcie->driver_data->irq_mask; - u32 board_irq = ioread32(KVASER_PCIEFD_PCI_IRQ_ADDR(pcie)); + u32 pci_irq = ioread32(KVASER_PCIEFD_PCI_IRQ_ADDR(pcie)); int i; - if (!(board_irq & irq_mask->all)) + if (!(pci_irq & irq_mask->all)) return IRQ_NONE; - if (board_irq & irq_mask->kcan_rx0) + if (pci_irq & irq_mask->kcan_rx0) kvaser_pciefd_receive_irq(pcie); for (i = 0; i < pcie->nr_channels; i++) { - if (board_irq & irq_mask->kcan_tx[i]) + if (pci_irq & irq_mask->kcan_tx[i]) kvaser_pciefd_transmit_irq(pcie->can[i]); } From 26a1b0fe3f62ba4a853f9d8ba3aa91f27e13f0ff Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Fri, 14 Jun 2024 17:15:24 +0200 Subject: [PATCH 7/7] can: kvaser_pciefd: Change name of return code variable Replace the variable name err used for return codes with the more generic name ret. An upcoming patch series for adding MSI interrupts will introduce code which also returns values other than return codes. Renaming the variable to ret enables using it for both purposes. This is applied to the whole file to make it consistent. Signed-off-by: Martin Jocic Link: https://lore.kernel.org/all/20240614151524.2718287-8-martin.jocic@kvaser.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 56 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index 8b2c18f2f23b..24871c276b31 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -711,17 +711,17 @@ static void kvaser_pciefd_pwm_start(struct kvaser_pciefd_can *can) static int kvaser_pciefd_open(struct net_device *netdev) { - int err; + int ret; struct kvaser_pciefd_can *can = netdev_priv(netdev); - err = open_candev(netdev); - if (err) - return err; + ret = open_candev(netdev); + if (ret) + return ret; - err = kvaser_pciefd_bus_on(can); - if (err) { + ret = kvaser_pciefd_bus_on(can); + if (ret) { close_candev(netdev); - return err; + return ret; } return 0; @@ -1032,15 +1032,15 @@ static int kvaser_pciefd_reg_candev(struct kvaser_pciefd *pcie) int i; for (i = 0; i < pcie->nr_channels; i++) { - int err = register_candev(pcie->can[i]->can.dev); + int ret = register_candev(pcie->can[i]->can.dev); - if (err) { + if (ret) { int j; /* Unregister all successfully registered devices. */ for (j = 0; j < i; j++) unregister_candev(pcie->can[j]->can.dev); - return err; + return ret; } } @@ -1726,7 +1726,7 @@ static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie) static int kvaser_pciefd_probe(struct pci_dev *pdev, const struct pci_device_id *id) { - int err; + int ret; struct kvaser_pciefd *pcie; const struct kvaser_pciefd_irq_mask *irq_mask; void __iomem *irq_en_base; @@ -1740,37 +1740,37 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, pcie->driver_data = (const struct kvaser_pciefd_driver_data *)id->driver_data; irq_mask = pcie->driver_data->irq_mask; - err = pci_enable_device(pdev); - if (err) - return err; + ret = pci_enable_device(pdev); + if (ret) + return ret; - err = pci_request_regions(pdev, KVASER_PCIEFD_DRV_NAME); - if (err) + ret = pci_request_regions(pdev, KVASER_PCIEFD_DRV_NAME); + if (ret) goto err_disable_pci; pcie->reg_base = pci_iomap(pdev, 0, 0); if (!pcie->reg_base) { - err = -ENOMEM; + ret = -ENOMEM; goto err_release_regions; } - err = kvaser_pciefd_setup_board(pcie); - if (err) + ret = kvaser_pciefd_setup_board(pcie); + if (ret) goto err_pci_iounmap; - err = kvaser_pciefd_setup_dma(pcie); - if (err) + ret = kvaser_pciefd_setup_dma(pcie); + if (ret) goto err_pci_iounmap; pci_set_master(pdev); - err = kvaser_pciefd_setup_can_ctrls(pcie); - if (err) + ret = kvaser_pciefd_setup_can_ctrls(pcie); + if (ret) goto err_teardown_can_ctrls; - err = request_irq(pcie->pci->irq, kvaser_pciefd_irq_handler, + ret = request_irq(pcie->pci->irq, kvaser_pciefd_irq_handler, IRQF_SHARED, KVASER_PCIEFD_DRV_NAME, pcie); - if (err) + if (ret) goto err_teardown_can_ctrls; iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1, @@ -1790,8 +1790,8 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CMD_REG); - err = kvaser_pciefd_reg_candev(pcie); - if (err) + ret = kvaser_pciefd_reg_candev(pcie); + if (ret) goto err_free_irq; return 0; @@ -1815,7 +1815,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, err_disable_pci: pci_disable_device(pdev); - return err; + return ret; } static void kvaser_pciefd_remove_all_ctrls(struct kvaser_pciefd *pcie)