2019-07-22 06:33:22

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

This patch add support for Fintek PCIE to 2 CAN controller support

Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
Changelog:
v2:
1: Fix comment on the spinlock with write access.
2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
3: Check the strap pin outside the loop.
4: Fix the cleanup issue in f81601_pci_add_card().
5: Remove unused "channels" in struct f81601_pci_card.

drivers/net/can/sja1000/Kconfig | 8 ++
drivers/net/can/sja1000/Makefile | 1 +
drivers/net/can/sja1000/f81601.c | 215 +++++++++++++++++++++++++++++++++++++++
3 files changed, 224 insertions(+)
create mode 100644 drivers/net/can/sja1000/f81601.c

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index f6dc89927ece..8588323c5138 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -101,4 +101,12 @@ config CAN_TSCAN1
IRQ numbers are read from jumpers JP4 and JP5,
SJA1000 IO base addresses are chosen heuristically (first that works).

+config CAN_F81601
+ tristate "Fintek F81601 PCIE to 2 CAN Controller"
+ depends on PCI
+ help
+ This driver adds support for Fintek F81601 PCIE to 2 CAN Controller.
+ It had internal 24MHz clock source, but it can be changed by
+ manufacturer. We can use modinfo to get usage for parameters.
+ Visit http://www.fintek.com.tw to get more information.
endif
diff --git a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
index 9253aaf9e739..6f6268543bd9 100644
--- a/drivers/net/can/sja1000/Makefile
+++ b/drivers/net/can/sja1000/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_CAN_PEAK_PCMCIA) += peak_pcmcia.o
obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
obj-$(CONFIG_CAN_TSCAN1) += tscan1.o
+obj-$(CONFIG_CAN_F81601) += f81601.o
diff --git a/drivers/net/can/sja1000/f81601.c b/drivers/net/can/sja1000/f81601.c
new file mode 100644
index 000000000000..3c378de8764d
--- /dev/null
+++ b/drivers/net/can/sja1000/f81601.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Fintek F81601 PCIE to 2 CAN controller driver
+ *
+ * Copyright (C) 2019 Peter Hong <[email protected]>
+ * Copyright (C) 2019 Linux Foundation
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/can/dev.h>
+#include <linux/io.h>
+#include <linux/version.h>
+
+#include "sja1000.h"
+
+#define F81601_PCI_MAX_CHAN 2
+
+#define F81601_DECODE_REG 0x209
+#define F81601_IO_MODE BIT(7)
+#define F81601_MEM_MODE BIT(6)
+#define F81601_CFG_MODE BIT(5)
+#define F81601_CAN2_INTERNAL_CLK BIT(3)
+#define F81601_CAN1_INTERNAL_CLK BIT(2)
+#define F81601_CAN2_EN BIT(1)
+#define F81601_CAN1_EN BIT(0)
+
+#define F81601_TRAP_REG 0x20a
+#define F81601_CAN2_HAS_EN BIT(4)
+
+struct f81601_pci_card {
+ void __iomem *addr;
+ spinlock_t lock; /* use this spin lock only for write access */
+ struct pci_dev *dev;
+ struct net_device *net_dev[F81601_PCI_MAX_CHAN];
+};
+
+static const struct pci_device_id f81601_pci_tbl[] = {
+ { PCI_DEVICE(0x1c29, 0x1703) },
+ {},
+};
+
+MODULE_DEVICE_TABLE(pci, f81601_pci_tbl);
+
+static bool internal_clk = 1;
+module_param(internal_clk, bool, 0444);
+MODULE_PARM_DESC(internal_clk, "Use internal clock, default 1 (24MHz)");
+
+static unsigned int external_clk;
+module_param(external_clk, uint, 0444);
+MODULE_PARM_DESC(external_clk, "External Clock, must spec when internal_clk = 0");
+
+static u8 f81601_pci_read_reg(const struct sja1000_priv *priv, int port)
+{
+ return readb(priv->reg_base + port);
+}
+
+static void f81601_pci_write_reg(const struct sja1000_priv *priv, int port,
+ u8 val)
+{
+ struct f81601_pci_card *card = priv->priv;
+ unsigned long flags;
+
+ spin_lock_irqsave(&card->lock, flags);
+ writeb(val, priv->reg_base + port);
+ readb(priv->reg_base);
+ spin_unlock_irqrestore(&card->lock, flags);
+}
+
+static void f81601_pci_del_card(struct pci_dev *pdev)
+{
+ struct f81601_pci_card *card = pci_get_drvdata(pdev);
+ struct net_device *dev;
+ int i = 0;
+
+ for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
+ dev = card->net_dev[i];
+ if (!dev)
+ continue;
+
+ dev_info(&pdev->dev, "%s: Removing %s\n", __func__, dev->name);
+
+ unregister_sja1000dev(dev);
+ free_sja1000dev(dev);
+ }
+
+ pcim_iounmap(pdev, card->addr);
+}
+
+/* Probe F81601 based device for the SJA1000 chips and register each
+ * available CAN channel to SJA1000 Socket-CAN subsystem.
+ */
+static int f81601_pci_add_card(struct pci_dev *pdev,
+ const struct pci_device_id *ent)
+{
+ struct sja1000_priv *priv;
+ struct net_device *dev;
+ struct f81601_pci_card *card;
+ int err, i, count;
+ u8 tmp;
+
+ if (pcim_enable_device(pdev) < 0) {
+ dev_err(&pdev->dev, "Failed to enable PCI device\n");
+ return -ENODEV;
+ }
+
+ dev_info(&pdev->dev, "Detected card at slot #%i\n",
+ PCI_SLOT(pdev->devfn));
+
+ card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
+ if (!card)
+ return -ENOMEM;
+
+ card->dev = pdev;
+ spin_lock_init(&card->lock);
+
+ pci_set_drvdata(pdev, card);
+
+ tmp = F81601_IO_MODE | F81601_MEM_MODE | F81601_CFG_MODE |
+ F81601_CAN2_EN | F81601_CAN1_EN;
+
+ if (internal_clk) {
+ tmp |= F81601_CAN2_INTERNAL_CLK | F81601_CAN1_INTERNAL_CLK;
+
+ dev_info(&pdev->dev,
+ "F81601 running with internal clock: 24Mhz\n");
+ } else {
+ dev_info(&pdev->dev,
+ "F81601 running with external clock: %dMhz\n",
+ external_clk / 1000000);
+ }
+
+ pci_write_config_byte(pdev, F81601_DECODE_REG, tmp);
+
+ card->addr = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
+
+ if (!card->addr) {
+ err = -ENOMEM;
+ dev_err(&pdev->dev, "%s: Failed to remap BAR\n", __func__);
+ goto failure_cleanup;
+ }
+
+ /* read CAN2_HW_EN strap pin to detect how many CANBUS do we have */
+ count = ARRAY_SIZE(card->net_dev);
+ pci_read_config_byte(pdev, F81601_TRAP_REG, &tmp);
+ if (!(tmp & F81601_CAN2_HAS_EN))
+ count = 1;
+
+ /* Detect available channels */
+ for (i = 0; i < count; i++) {
+ dev = alloc_sja1000dev(0);
+ if (!dev) {
+ err = -ENOMEM;
+ goto failure_cleanup;
+ }
+
+ priv = netdev_priv(dev);
+ priv->priv = card;
+ priv->irq_flags = IRQF_SHARED;
+ priv->reg_base = card->addr + 0x80 * i;
+ priv->read_reg = f81601_pci_read_reg;
+ priv->write_reg = f81601_pci_write_reg;
+
+ if (internal_clk)
+ priv->can.clock.freq = 24000000 / 2;
+ else
+ priv->can.clock.freq = external_clk / 2;
+
+ priv->ocr = OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL;
+ priv->cdr = CDR_CBP;
+
+ SET_NETDEV_DEV(dev, &pdev->dev);
+ dev->dev_id = i;
+ dev->irq = pdev->irq;
+
+ /* Register SJA1000 device */
+ err = register_sja1000dev(dev);
+ if (err) {
+ dev_err(&pdev->dev,
+ "%s: Registering device failed: %x\n", __func__,
+ err);
+ free_sja1000dev(dev);
+ goto failure_cleanup;
+ }
+
+ card->net_dev[i] = dev;
+ dev_info(&pdev->dev, "Channel #%d, %s at 0x%p, irq %d\n", i,
+ dev->name, priv->reg_base, dev->irq);
+ }
+
+ return 0;
+
+failure_cleanup:
+ dev_err(&pdev->dev, "%s: failed: %d. Cleaning Up.\n", __func__, err);
+ f81601_pci_del_card(pdev);
+
+ return err;
+}
+
+static struct pci_driver f81601_pci_driver = {
+ .name = "f81601",
+ .id_table = f81601_pci_tbl,
+ .probe = f81601_pci_add_card,
+ .remove = f81601_pci_del_card,
+};
+
+MODULE_DESCRIPTION("Fintek F81601 PCIE to 2 CANBUS adaptor driver");
+MODULE_AUTHOR("Peter Hong <[email protected]>");
+MODULE_LICENSE("GPL v2");
+
+module_pci_driver(f81601_pci_driver);
--
2.7.4


2019-07-22 08:42:51

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

Hi Marc,

Marc Kleine-Budde 於 2019/7/22 下午 04:15 寫道:
> On 7/22/19 8:22 AM, Ji-Ze Hong (Peter Hong) wrote: >> +/* Probe F81601 based device for the SJA1000 chips and register each
>> + * available CAN channel to SJA1000 Socket-CAN subsystem.
>> + */
>> +static int f81601_pci_add_card(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> +{
>> + struct sja1000_priv *priv;
>> + struct net_device *dev;
>> + struct f81601_pci_card *card;
>> + int err, i, count;
>> + u8 tmp;
>> +
>> + if (pcim_enable_device(pdev) < 0) {
>
> I'm missing a corresponding disable_device().
>
I'm using managed pcim_enable_device(), Does it need call
pci_disable_device() ??

Thanks
--
With Best Regards,
Peter Hong

2019-07-22 09:08:57

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

On 7/22/19 8:22 AM, Ji-Ze Hong (Peter Hong) wrote:
> This patch add support for Fintek PCIE to 2 CAN controller support
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
> ---
> Changelog:
> v2:
> 1: Fix comment on the spinlock with write access.
> 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
> 3: Check the strap pin outside the loop.
> 4: Fix the cleanup issue in f81601_pci_add_card().
> 5: Remove unused "channels" in struct f81601_pci_card.
>
> drivers/net/can/sja1000/Kconfig | 8 ++
> drivers/net/can/sja1000/Makefile | 1 +
> drivers/net/can/sja1000/f81601.c | 215 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 224 insertions(+)
> create mode 100644 drivers/net/can/sja1000/f81601.c
>
> diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
> index f6dc89927ece..8588323c5138 100644
> --- a/drivers/net/can/sja1000/Kconfig
> +++ b/drivers/net/can/sja1000/Kconfig
> @@ -101,4 +101,12 @@ config CAN_TSCAN1
> IRQ numbers are read from jumpers JP4 and JP5,
> SJA1000 IO base addresses are chosen heuristically (first that works).
>
> +config CAN_F81601
> + tristate "Fintek F81601 PCIE to 2 CAN Controller"
> + depends on PCI
> + help
> + This driver adds support for Fintek F81601 PCIE to 2 CAN Controller.
> + It had internal 24MHz clock source, but it can be changed by
> + manufacturer. We can use modinfo to get usage for parameters.
> + Visit http://www.fintek.com.tw to get more information.
> endif
> diff --git a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
> index 9253aaf9e739..6f6268543bd9 100644
> --- a/drivers/net/can/sja1000/Makefile
> +++ b/drivers/net/can/sja1000/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_CAN_PEAK_PCMCIA) += peak_pcmcia.o
> obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
> obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
> obj-$(CONFIG_CAN_TSCAN1) += tscan1.o
> +obj-$(CONFIG_CAN_F81601) += f81601.o
> diff --git a/drivers/net/can/sja1000/f81601.c b/drivers/net/can/sja1000/f81601.c
> new file mode 100644
> index 000000000000..3c378de8764d
> --- /dev/null
> +++ b/drivers/net/can/sja1000/f81601.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Fintek F81601 PCIE to 2 CAN controller driver
> + *
> + * Copyright (C) 2019 Peter Hong <[email protected]>
> + * Copyright (C) 2019 Linux Foundation
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/netdevice.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +#include <linux/can/dev.h>
> +#include <linux/io.h>
> +#include <linux/version.h>
> +
> +#include "sja1000.h"
> +
> +#define F81601_PCI_MAX_CHAN 2
> +
> +#define F81601_DECODE_REG 0x209
> +#define F81601_IO_MODE BIT(7)
> +#define F81601_MEM_MODE BIT(6)
> +#define F81601_CFG_MODE BIT(5)
> +#define F81601_CAN2_INTERNAL_CLK BIT(3)
> +#define F81601_CAN1_INTERNAL_CLK BIT(2)
> +#define F81601_CAN2_EN BIT(1)
> +#define F81601_CAN1_EN BIT(0)
> +
> +#define F81601_TRAP_REG 0x20a
> +#define F81601_CAN2_HAS_EN BIT(4)
> +
> +struct f81601_pci_card {
> + void __iomem *addr;
> + spinlock_t lock; /* use this spin lock only for write access */
> + struct pci_dev *dev;
> + struct net_device *net_dev[F81601_PCI_MAX_CHAN];
> +};
> +
> +static const struct pci_device_id f81601_pci_tbl[] = {
> + { PCI_DEVICE(0x1c29, 0x1703) },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(pci, f81601_pci_tbl);
> +
> +static bool internal_clk = 1;

true

> +module_param(internal_clk, bool, 0444);
> +MODULE_PARM_DESC(internal_clk, "Use internal clock, default 1 (24MHz)");
> +
> +static unsigned int external_clk;
> +module_param(external_clk, uint, 0444);
> +MODULE_PARM_DESC(external_clk, "External Clock, must spec when internal_clk = 0");
> +
> +static u8 f81601_pci_read_reg(const struct sja1000_priv *priv, int port)
> +{
> + return readb(priv->reg_base + port);
> +}
> +
> +static void f81601_pci_write_reg(const struct sja1000_priv *priv, int port,
> + u8 val)
> +{
> + struct f81601_pci_card *card = priv->priv;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&card->lock, flags);
> + writeb(val, priv->reg_base + port);
> + readb(priv->reg_base);
> + spin_unlock_irqrestore(&card->lock, flags);
> +}
> +
> +static void f81601_pci_del_card(struct pci_dev *pdev)
> +{
> + struct f81601_pci_card *card = pci_get_drvdata(pdev);
> + struct net_device *dev;
> + int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
> + dev = card->net_dev[i];
> + if (!dev)
> + continue;
> +
> + dev_info(&pdev->dev, "%s: Removing %s\n", __func__, dev->name);
> +
> + unregister_sja1000dev(dev);
> + free_sja1000dev(dev);
> + }
> +
> + pcim_iounmap(pdev, card->addr);
> +}
> +
> +/* Probe F81601 based device for the SJA1000 chips and register each
> + * available CAN channel to SJA1000 Socket-CAN subsystem.
> + */
> +static int f81601_pci_add_card(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct sja1000_priv *priv;
> + struct net_device *dev;
> + struct f81601_pci_card *card;
> + int err, i, count;
> + u8 tmp;
> +
> + if (pcim_enable_device(pdev) < 0) {

I'm missing a corresponding disable_device().

> + dev_err(&pdev->dev, "Failed to enable PCI device\n");
> + return -ENODEV;
> + }
> +
> + dev_info(&pdev->dev, "Detected card at slot #%i\n",
> + PCI_SLOT(pdev->devfn));
> +
> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> + if (!card)
> + return -ENOMEM;
> +
> + card->dev = pdev;
> + spin_lock_init(&card->lock);
> +
> + pci_set_drvdata(pdev, card);
> +
> + tmp = F81601_IO_MODE | F81601_MEM_MODE | F81601_CFG_MODE |
> + F81601_CAN2_EN | F81601_CAN1_EN;
> +
> + if (internal_clk) {
> + tmp |= F81601_CAN2_INTERNAL_CLK | F81601_CAN1_INTERNAL_CLK;
> +
> + dev_info(&pdev->dev,
> + "F81601 running with internal clock: 24Mhz\n");
> + } else {
> + dev_info(&pdev->dev,
> + "F81601 running with external clock: %dMhz\n",
> + external_clk / 1000000);
> + }
> +
> + pci_write_config_byte(pdev, F81601_DECODE_REG, tmp);
> +
> + card->addr = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
> +
> + if (!card->addr) {
> + err = -ENOMEM;
> + dev_err(&pdev->dev, "%s: Failed to remap BAR\n", __func__);
> + goto failure_cleanup;
> + }
> +
> + /* read CAN2_HW_EN strap pin to detect how many CANBUS do we have */
> + count = ARRAY_SIZE(card->net_dev);
> + pci_read_config_byte(pdev, F81601_TRAP_REG, &tmp);
> + if (!(tmp & F81601_CAN2_HAS_EN))
> + count = 1;
> +
> + /* Detect available channels */
> + for (i = 0; i < count; i++) {
> + dev = alloc_sja1000dev(0);
> + if (!dev) {
> + err = -ENOMEM;
> + goto failure_cleanup;
> + }
> +
> + priv = netdev_priv(dev);
> + priv->priv = card;
> + priv->irq_flags = IRQF_SHARED;
> + priv->reg_base = card->addr + 0x80 * i;
> + priv->read_reg = f81601_pci_read_reg;
> + priv->write_reg = f81601_pci_write_reg;
> +
> + if (internal_clk)
> + priv->can.clock.freq = 24000000 / 2;
> + else
> + priv->can.clock.freq = external_clk / 2;
> +
> + priv->ocr = OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL;
> + priv->cdr = CDR_CBP;
> +
> + SET_NETDEV_DEV(dev, &pdev->dev);
> + dev->dev_id = i;
> + dev->irq = pdev->irq;
> +
> + /* Register SJA1000 device */
> + err = register_sja1000dev(dev);
> + if (err) {
> + dev_err(&pdev->dev,
> + "%s: Registering device failed: %x\n", __func__,
> + err);
> + free_sja1000dev(dev);
> + goto failure_cleanup;
> + }
> +
> + card->net_dev[i] = dev;
> + dev_info(&pdev->dev, "Channel #%d, %s at 0x%p, irq %d\n", i,
> + dev->name, priv->reg_base, dev->irq);
> + }
> +
> + return 0;
> +
> +failure_cleanup:
> + dev_err(&pdev->dev, "%s: failed: %d. Cleaning Up.\n", __func__, err);
> + f81601_pci_del_card(pdev);
> +
> + return err;
> +}
> +
> +static struct pci_driver f81601_pci_driver = {
> + .name = "f81601",
> + .id_table = f81601_pci_tbl,
> + .probe = f81601_pci_add_card,
> + .remove = f81601_pci_del_card,
> +};
> +
> +MODULE_DESCRIPTION("Fintek F81601 PCIE to 2 CANBUS adaptor driver");
> +MODULE_AUTHOR("Peter Hong <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +
> +module_pci_driver(f81601_pci_driver);
>

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2019-07-22 11:08:45

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

On 7/22/19 10:36 AM, Ji-Ze Hong (Peter Hong) wrote:
> Hi Marc,
>
> Marc Kleine-Budde 於 2019/7/22 下午 04:15 寫道:
>> On 7/22/19 8:22 AM, Ji-Ze Hong (Peter Hong) wrote: >> +/* Probe F81601 based device for the SJA1000 chips and register each
>>> + * available CAN channel to SJA1000 Socket-CAN subsystem.
>>> + */
>>> +static int f81601_pci_add_card(struct pci_dev *pdev,
>>> + const struct pci_device_id *ent)
>>> +{
>>> + struct sja1000_priv *priv;
>>> + struct net_device *dev;
>>> + struct f81601_pci_card *card;
>>> + int err, i, count;
>>> + u8 tmp;
>>> +
>>> + if (pcim_enable_device(pdev) < 0) {
>>
>> I'm missing a corresponding disable_device().
>>
> I'm using managed pcim_enable_device(), Does it need call
> pci_disable_device() ??

Right.

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2019-07-22 11:56:59

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

On 7/22/19 8:22 AM, Ji-Ze Hong (Peter Hong) wrote:
> This patch add support for Fintek PCIE to 2 CAN controller support
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
> ---
> Changelog:
> v2:
> 1: Fix comment on the spinlock with write access.
> 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
> 3: Check the strap pin outside the loop.
> 4: Fix the cleanup issue in f81601_pci_add_card().
> 5: Remove unused "channels" in struct f81601_pci_card.
>
> drivers/net/can/sja1000/Kconfig | 8 ++
> drivers/net/can/sja1000/Makefile | 1 +
> drivers/net/can/sja1000/f81601.c | 215 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 224 insertions(+)
> create mode 100644 drivers/net/can/sja1000/f81601.c

> diff --git a/drivers/net/can/sja1000/f81601.c b/drivers/net/can/sja1000/f81601.c
> new file mode 100644
> index 000000000000..3c378de8764d
> --- /dev/null
> +++ b/drivers/net/can/sja1000/f81601.c

[...]

> +static void f81601_pci_del_card(struct pci_dev *pdev)
> +{
> + struct f81601_pci_card *card = pci_get_drvdata(pdev);
> + struct net_device *dev;
> + int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
> + dev = card->net_dev[i];
> + if (!dev)
> + continue;
> +
> + dev_info(&pdev->dev, "%s: Removing %s\n", __func__, dev->name);
> +
> + unregister_sja1000dev(dev);
> + free_sja1000dev(dev);
> + }
> +
> + pcim_iounmap(pdev, card->addr);

Why do you need iounmap()?

> +}

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2019-07-24 02:32:18

by Saeed Mahameed

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

On Mon, 2019-07-22 at 14:22 +0800, Ji-Ze Hong (Peter Hong) wrote:
> This patch add support for Fintek PCIE to 2 CAN controller support
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]
> >
> ---
> Changelog:
> v2:
> 1: Fix comment on the spinlock with write access.
> 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
> 3: Check the strap pin outside the loop.
> 4: Fix the cleanup issue in f81601_pci_add_card().
> 5: Remove unused "channels" in struct f81601_pci_card.
>
> drivers/net/can/sja1000/Kconfig | 8 ++
> drivers/net/can/sja1000/Makefile | 1 +
> drivers/net/can/sja1000/f81601.c | 215
> +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 224 insertions(+)
> create mode 100644 drivers/net/can/sja1000/f81601.c
>
> diff --git a/drivers/net/can/sja1000/Kconfig
> b/drivers/net/can/sja1000/Kconfig
> index f6dc89927ece..8588323c5138 100644
> --- a/drivers/net/can/sja1000/Kconfig
> +++ b/drivers/net/can/sja1000/Kconfig
> @@ -101,4 +101,12 @@ config CAN_TSCAN1
> IRQ numbers are read from jumpers JP4 and JP5,
> SJA1000 IO base addresses are chosen heuristically (first
> that works).
>
> +config CAN_F81601
> + tristate "Fintek F81601 PCIE to 2 CAN Controller"
> + depends on PCI
> + help
> + This driver adds support for Fintek F81601 PCIE to 2 CAN
> Controller.
> + It had internal 24MHz clock source, but it can be changed by
> + manufacturer. We can use modinfo to get usage for parameters.
> + Visit http://www.fintek.com.tw to get more information.
> endif
> diff --git a/drivers/net/can/sja1000/Makefile
> b/drivers/net/can/sja1000/Makefile
> index 9253aaf9e739..6f6268543bd9 100644
> --- a/drivers/net/can/sja1000/Makefile
> +++ b/drivers/net/can/sja1000/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_CAN_PEAK_PCMCIA) += peak_pcmcia.o
> obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
> obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
> obj-$(CONFIG_CAN_TSCAN1) += tscan1.o
> +obj-$(CONFIG_CAN_F81601) += f81601.o
> diff --git a/drivers/net/can/sja1000/f81601.c
> b/drivers/net/can/sja1000/f81601.c
> new file mode 100644
> index 000000000000..3c378de8764d
> --- /dev/null
> +++ b/drivers/net/can/sja1000/f81601.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Fintek F81601 PCIE to 2 CAN controller driver
> + *
> + * Copyright (C) 2019 Peter Hong <[email protected]>
> + * Copyright (C) 2019 Linux Foundation
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/netdevice.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/pci.h>
> +#include <linux/can/dev.h>
> +#include <linux/io.h>
> +#include <linux/version.h>
> +
> +#include "sja1000.h"
> +
> +#define F81601_PCI_MAX_CHAN 2
> +
> +#define F81601_DECODE_REG 0x209
> +#define F81601_IO_MODE BIT(7)
> +#define F81601_MEM_MODE BIT(6)
> +#define F81601_CFG_MODE BIT(5)
> +#define F81601_CAN2_INTERNAL_CLK BIT(3)
> +#define F81601_CAN1_INTERNAL_CLK BIT(2)
> +#define F81601_CAN2_EN BIT(1)
> +#define F81601_CAN1_EN BIT(0)
> +
> +#define F81601_TRAP_REG 0x20a
> +#define F81601_CAN2_HAS_EN BIT(4)
> +
> +struct f81601_pci_card {
> + void __iomem *addr;
> + spinlock_t lock; /* use this spin lock only for write access
> */
> + struct pci_dev *dev;
> + struct net_device *net_dev[F81601_PCI_MAX_CHAN];
> +};
> +
> +static const struct pci_device_id f81601_pci_tbl[] = {
> + { PCI_DEVICE(0x1c29, 0x1703) },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(pci, f81601_pci_tbl);
> +
> +static bool internal_clk = 1;
> +module_param(internal_clk, bool, 0444);
> +MODULE_PARM_DESC(internal_clk, "Use internal clock, default 1
> (24MHz)");
> +
> +static unsigned int external_clk;
> +module_param(external_clk, uint, 0444);
> +MODULE_PARM_DESC(external_clk, "External Clock, must spec when
> internal_clk = 0");
> +
> +static u8 f81601_pci_read_reg(const struct sja1000_priv *priv, int
> port)
> +{
> + return readb(priv->reg_base + port);
> +}
> +
> +static void f81601_pci_write_reg(const struct sja1000_priv *priv,
> int port,
> + u8 val)
> +{
> + struct f81601_pci_card *card = priv->priv;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&card->lock, flags);
> + writeb(val, priv->reg_base + port);
> + readb(priv->reg_base);
> + spin_unlock_irqrestore(&card->lock, flags);
> +}
> +
> +static void f81601_pci_del_card(struct pci_dev *pdev)
> +{
> + struct f81601_pci_card *card = pci_get_drvdata(pdev);
> + struct net_device *dev;
> + int i = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
> + dev = card->net_dev[i];
> + if (!dev)
> + continue;
> +
> + dev_info(&pdev->dev, "%s: Removing %s\n", __func__,
> dev->name);
> +
> + unregister_sja1000dev(dev);
> + free_sja1000dev(dev);
> + }
> +
> + pcim_iounmap(pdev, card->addr);
> +}
> +
> +/* Probe F81601 based device for the SJA1000 chips and register each
> + * available CAN channel to SJA1000 Socket-CAN subsystem.
> + */
> +static int f81601_pci_add_card(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct sja1000_priv *priv;
> + struct net_device *dev;
> + struct f81601_pci_card *card;

nit, reverse xmas tree.

> + int err, i, count;
> + u8 tmp;
> +
> + if (pcim_enable_device(pdev) < 0) {
> + dev_err(&pdev->dev, "Failed to enable PCI device\n");
> + return -ENODEV;
> + }
> +
> + dev_info(&pdev->dev, "Detected card at slot #%i\n",
> + PCI_SLOT(pdev->devfn));
> +
> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> + if (!card)
> + return -ENOMEM;
> +
> + card->dev = pdev;
> + spin_lock_init(&card->lock);
> +
> + pci_set_drvdata(pdev, card);
> +
> + tmp = F81601_IO_MODE | F81601_MEM_MODE | F81601_CFG_MODE |
> + F81601_CAN2_EN | F81601_CAN1_EN;
> +
> + if (internal_clk) {
> + tmp |= F81601_CAN2_INTERNAL_CLK |
> F81601_CAN1_INTERNAL_CLK;
> +
> + dev_info(&pdev->dev,
> + "F81601 running with internal clock:
> 24Mhz\n");
> + } else {
> + dev_info(&pdev->dev,
> + "F81601 running with external clock: %dMhz\n",
> + external_clk / 1000000);
> + }
> +
> + pci_write_config_byte(pdev, F81601_DECODE_REG, tmp);
> +
> + card->addr = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
> +
> + if (!card->addr) {
> + err = -ENOMEM;
> + dev_err(&pdev->dev, "%s: Failed to remap BAR\n",
> __func__);
> + goto failure_cleanup;
> + }
> +
> + /* read CAN2_HW_EN strap pin to detect how many CANBUS do we
> have */
> + count = ARRAY_SIZE(card->net_dev);
> + pci_read_config_byte(pdev, F81601_TRAP_REG, &tmp);
> + if (!(tmp & F81601_CAN2_HAS_EN))
> + count = 1;
> +
> + /* Detect available channels */
> + for (i = 0; i < count; i++) {
> + dev = alloc_sja1000dev(0);
> + if (!dev) {
> + err = -ENOMEM;
> + goto failure_cleanup;
> + }
> +

don't you need to rollback and cleanup/unregister previously allocated
devs ?

> + priv = netdev_priv(dev);
> + priv->priv = card;
> + priv->irq_flags = IRQF_SHARED;
> + priv->reg_base = card->addr + 0x80 * i;
> + priv->read_reg = f81601_pci_read_reg;
> + priv->write_reg = f81601_pci_write_reg;
> +
> + if (internal_clk)
> + priv->can.clock.freq = 24000000 / 2;
> + else
> + priv->can.clock.freq = external_clk / 2;
> +
> + priv->ocr = OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL;
> + priv->cdr = CDR_CBP;
> +
> + SET_NETDEV_DEV(dev, &pdev->dev);
> + dev->dev_id = i;
> + dev->irq = pdev->irq;
> +
> + /* Register SJA1000 device */
> + err = register_sja1000dev(dev);
> + if (err) {
> + dev_err(&pdev->dev,
> + "%s: Registering device failed: %x\n",
> __func__,
> + err);
> + free_sja1000dev(dev);
> + goto failure_cleanup;
> + }
> +
> + card->net_dev[i] = dev;
> + dev_info(&pdev->dev, "Channel #%d, %s at 0x%p, irq
> %d\n", i,
> + dev->name, priv->reg_base, dev->irq);
> + }
> +
> + return 0;
> +
> +failure_cleanup:
> + dev_err(&pdev->dev, "%s: failed: %d. Cleaning Up.\n", __func__,
> err);
> + f81601_pci_del_card(pdev);
> +
> + return err;
> +}
> +
> +static struct pci_driver f81601_pci_driver = {
> + .name = "f81601",
> + .id_table = f81601_pci_tbl,
> + .probe = f81601_pci_add_card,
> + .remove = f81601_pci_del_card,
> +};
> +
> +MODULE_DESCRIPTION("Fintek F81601 PCIE to 2 CANBUS adaptor driver");
> +MODULE_AUTHOR("Peter Hong <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> +
> +module_pci_driver(f81601_pci_driver);

2019-07-24 05:20:31

by Ji-Ze Hong (Peter Hong)

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

Hi,

Saeed Mahameed 於 2019/7/24 上午 05:38 寫道:
> On Mon, 2019-07-22 at 14:22 +0800, Ji-Ze Hong (Peter Hong) wrote:
>> This patch add support for Fintek PCIE to 2 CAN controller support
>>
>> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]
>>>
>> ---
>> Changelog:
>> v2:
>> 1: Fix comment on the spinlock with write access.
>> 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
>> 3: Check the strap pin outside the loop.
>> 4: Fix the cleanup issue in f81601_pci_add_card().
>> 5: Remove unused "channels" in struct f81601_pci_card.
>>
>> drivers/net/can/sja1000/Kconfig | 8 ++
>> drivers/net/can/sja1000/Makefile | 1 +
>> drivers/net/can/sja1000/f81601.c | 215
>> +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 224 insertions(+)
>> create mode 100644 drivers/net/can/sja1000/f81601.c
>>
>> diff --git a/drivers/net/can/sja1000/Kconfig
>> b/drivers/net/can/sja1000/Kconfig
>> index f6dc89927ece..8588323c5138 100644
>> --- a/drivers/net/can/sja1000/Kconfig
>> +++ b/drivers/net/can/sja1000/Kconfig
>> @@ -101,4 +101,12 @@ config CAN_TSCAN1
>> IRQ numbers are read from jumpers JP4 and JP5,
>> SJA1000 IO base addresses are chosen heuristically (first
>> that works).
>>
>> +config CAN_F81601
>> + tristate "Fintek F81601 PCIE to 2 CAN Controller"
>> + depends on PCI
>> + help
>> + This driver adds support for Fintek F81601 PCIE to 2 CAN
>> Controller.
>> + It had internal 24MHz clock source, but it can be changed by
>> + manufacturer. We can use modinfo to get usage for parameters.
>> + Visit http://www.fintek.com.tw to get more information.
>> endif
>> diff --git a/drivers/net/can/sja1000/Makefile
>> b/drivers/net/can/sja1000/Makefile
>> index 9253aaf9e739..6f6268543bd9 100644
>> --- a/drivers/net/can/sja1000/Makefile
>> +++ b/drivers/net/can/sja1000/Makefile
>> @@ -13,3 +13,4 @@ obj-$(CONFIG_CAN_PEAK_PCMCIA) += peak_pcmcia.o
>> obj-$(CONFIG_CAN_PEAK_PCI) += peak_pci.o
>> obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
>> obj-$(CONFIG_CAN_TSCAN1) += tscan1.o
>> +obj-$(CONFIG_CAN_F81601) += f81601.o
>> diff --git a/drivers/net/can/sja1000/f81601.c
>> b/drivers/net/can/sja1000/f81601.c
>> new file mode 100644
>> index 000000000000..3c378de8764d
>> --- /dev/null
>> +++ b/drivers/net/can/sja1000/f81601.c
>> @@ -0,0 +1,215 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Fintek F81601 PCIE to 2 CAN controller driver
>> + *
>> + * Copyright (C) 2019 Peter Hong <[email protected]>
>> + * Copyright (C) 2019 Linux Foundation
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/delay.h>
>> +#include <linux/slab.h>
>> +#include <linux/pci.h>
>> +#include <linux/can/dev.h>
>> +#include <linux/io.h>
>> +#include <linux/version.h>
>> +
>> +#include "sja1000.h"
>> +
>> +#define F81601_PCI_MAX_CHAN 2
>> +
>> +#define F81601_DECODE_REG 0x209
>> +#define F81601_IO_MODE BIT(7)
>> +#define F81601_MEM_MODE BIT(6)
>> +#define F81601_CFG_MODE BIT(5)
>> +#define F81601_CAN2_INTERNAL_CLK BIT(3)
>> +#define F81601_CAN1_INTERNAL_CLK BIT(2)
>> +#define F81601_CAN2_EN BIT(1)
>> +#define F81601_CAN1_EN BIT(0)
>> +
>> +#define F81601_TRAP_REG 0x20a
>> +#define F81601_CAN2_HAS_EN BIT(4)
>> +
>> +struct f81601_pci_card {
>> + void __iomem *addr;
>> + spinlock_t lock; /* use this spin lock only for write access
>> */
>> + struct pci_dev *dev;
>> + struct net_device *net_dev[F81601_PCI_MAX_CHAN];
>> +};
>> +
>> +static const struct pci_device_id f81601_pci_tbl[] = {
>> + { PCI_DEVICE(0x1c29, 0x1703) },
>> + {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(pci, f81601_pci_tbl);
>> +
>> +static bool internal_clk = 1;
>> +module_param(internal_clk, bool, 0444);
>> +MODULE_PARM_DESC(internal_clk, "Use internal clock, default 1
>> (24MHz)");
>> +
>> +static unsigned int external_clk;
>> +module_param(external_clk, uint, 0444);
>> +MODULE_PARM_DESC(external_clk, "External Clock, must spec when
>> internal_clk = 0");
>> +
>> +static u8 f81601_pci_read_reg(const struct sja1000_priv *priv, int
>> port)
>> +{
>> + return readb(priv->reg_base + port);
>> +}
>> +
>> +static void f81601_pci_write_reg(const struct sja1000_priv *priv,
>> int port,
>> + u8 val)
>> +{
>> + struct f81601_pci_card *card = priv->priv;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&card->lock, flags);
>> + writeb(val, priv->reg_base + port);
>> + readb(priv->reg_base);
>> + spin_unlock_irqrestore(&card->lock, flags);
>> +}
>> +
>> +static void f81601_pci_del_card(struct pci_dev *pdev)
>> +{
>> + struct f81601_pci_card *card = pci_get_drvdata(pdev);
>> + struct net_device *dev;
>> + int i = 0;
>> +
>> + for (i = 0; i < ARRAY_SIZE(card->net_dev); i++) {
>> + dev = card->net_dev[i];
>> + if (!dev)
>> + continue;
>> +
>> + dev_info(&pdev->dev, "%s: Removing %s\n", __func__,
>> dev->name);
>> +
>> + unregister_sja1000dev(dev);
>> + free_sja1000dev(dev);
>> + }
>> +
>> + pcim_iounmap(pdev, card->addr);
>> +}
>> +
>> +/* Probe F81601 based device for the SJA1000 chips and register each
>> + * available CAN channel to SJA1000 Socket-CAN subsystem.
>> + */
>> +static int f81601_pci_add_card(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> +{
>> + struct sja1000_priv *priv;
>> + struct net_device *dev;
>> + struct f81601_pci_card *card;
>
> nit, reverse xmas tree.
>
>> + int err, i, count;
>> + u8 tmp;
>> +
>> + if (pcim_enable_device(pdev) < 0) {
>> + dev_err(&pdev->dev, "Failed to enable PCI device\n");
>> + return -ENODEV;
>> + }
>> +
>> + dev_info(&pdev->dev, "Detected card at slot #%i\n",
>> + PCI_SLOT(pdev->devfn));
>> +
>> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
>> + if (!card)
>> + return -ENOMEM;
>> +
>> + card->dev = pdev;
>> + spin_lock_init(&card->lock);
>> +
>> + pci_set_drvdata(pdev, card);
>> +
>> + tmp = F81601_IO_MODE | F81601_MEM_MODE | F81601_CFG_MODE |
>> + F81601_CAN2_EN | F81601_CAN1_EN;
>> +
>> + if (internal_clk) {
>> + tmp |= F81601_CAN2_INTERNAL_CLK |
>> F81601_CAN1_INTERNAL_CLK;
>> +
>> + dev_info(&pdev->dev,
>> + "F81601 running with internal clock:
>> 24Mhz\n");
>> + } else {
>> + dev_info(&pdev->dev,
>> + "F81601 running with external clock: %dMhz\n",
>> + external_clk / 1000000);
>> + }
>> +
>> + pci_write_config_byte(pdev, F81601_DECODE_REG, tmp);
>> +
>> + card->addr = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
>> +
>> + if (!card->addr) {
>> + err = -ENOMEM;
>> + dev_err(&pdev->dev, "%s: Failed to remap BAR\n",
>> __func__);
>> + goto failure_cleanup;
>> + }
>> +
>> + /* read CAN2_HW_EN strap pin to detect how many CANBUS do we
>> have */
>> + count = ARRAY_SIZE(card->net_dev);
>> + pci_read_config_byte(pdev, F81601_TRAP_REG, &tmp);
>> + if (!(tmp & F81601_CAN2_HAS_EN))
>> + count = 1;
>> +
>> + /* Detect available channels */
>> + for (i = 0; i < count; i++) {
>> + dev = alloc_sja1000dev(0);
>> + if (!dev) {
>> + err = -ENOMEM;
>> + goto failure_cleanup;
>> + }
>> +
>
> don't you need to rollback and cleanup/unregister previously allocated
> devs ?
>

I'll do cleanup when errors jump to failure_cleanup label and do
f81601_pci_del_card().

>> + priv = netdev_priv(dev);
>> + priv->priv = card;
>> + priv->irq_flags = IRQF_SHARED;
>> + priv->reg_base = card->addr + 0x80 * i;
>> + priv->read_reg = f81601_pci_read_reg;
>> + priv->write_reg = f81601_pci_write_reg;
>> +
>> + if (internal_clk)
>> + priv->can.clock.freq = 24000000 / 2;
>> + else
>> + priv->can.clock.freq = external_clk / 2;
>> +
>> + priv->ocr = OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL;
>> + priv->cdr = CDR_CBP;
>> +
>> + SET_NETDEV_DEV(dev, &pdev->dev);
>> + dev->dev_id = i;
>> + dev->irq = pdev->irq;
>> +
>> + /* Register SJA1000 device */
>> + err = register_sja1000dev(dev);
>> + if (err) {
>> + dev_err(&pdev->dev,
>> + "%s: Registering device failed: %x\n",
>> __func__,
>> + err);
>> + free_sja1000dev(dev);
>> + goto failure_cleanup;
>> + }
>> +
>> + card->net_dev[i] = dev;
>> + dev_info(&pdev->dev, "Channel #%d, %s at 0x%p, irq
>> %d\n", i,
>> + dev->name, priv->reg_base, dev->irq);
>> + }
>> +
>> + return 0;
>> +
>> +failure_cleanup:
>> + dev_err(&pdev->dev, "%s: failed: %d. Cleaning Up.\n", __func__,
>> err);
>> + f81601_pci_del_card(pdev);

--
With Best Regards,
Peter Hong

2019-07-24 19:48:26

by Saeed Mahameed

[permalink] [raw]
Subject: Re: [PATCH V2 1/1] can: sja1000: f81601: add Fintek F81601 support

On Wed, 2019-07-24 at 13:19 +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi,
>
> Saeed Mahameed 於 2019/7/24 上午 05:38 寫道:
> > On Mon, 2019-07-22 at 14:22 +0800, Ji-Ze Hong (Peter Hong) wrote:
> > > This patch add support for Fintek PCIE to 2 CAN controller
> > > support
> > >
> > > Signed-off-by: Ji-Ze Hong (Peter Hong) <
> > > [email protected]
> > > ---
> > > Changelog:
> > > v2:
> > > 1: Fix comment on the spinlock with write access.
> > > 2: Use ARRAY_SIZE instead of F81601_PCI_MAX_CHAN.
> > > 3: Check the strap pin outside the loop.
> > > 4: Fix the cleanup issue in f81601_pci_add_card().
> > > 5: Remove unused "channels" in struct f81601_pci_card.
> > >
> > > drivers/net/can/sja1000/Kconfig | 8 ++
> > > drivers/net/can/sja1000/Makefile | 1 +
> > > drivers/net/can/sja1000/f81601.c | 215
> > >

[...]

> > >
> > > + /* Detect available channels */
> > > + for (i = 0; i < count; i++) {
> > > + dev = alloc_sja1000dev(0);
> > > + if (!dev) {
> > > + err = -ENOMEM;
> > > + goto failure_cleanup;
> > > + }
> > > +
> >
> > don't you need to rollback and cleanup/unregister previously
> > allocated
> > devs ?
> >
>
> I'll do cleanup when errors jump to failure_cleanup label and do
> f81601_pci_del_card().
>

Right !, thanks for the clarification.