>> 2010/10/6 Wolfram Sang:
> 2010/10/6 Wolfgang Grandegger:

Thank you very much for your reviews!
Here is the new patch with your suggestions, down below.

> +     outb(val, (int)priv->reg_base + reg);
> Is the cast really needed?

Yes. :-(
priv->reg_base is of type void __iomem * and outb/inb expect an int.
Without the cast, gcc generates a warning: passing argument 2 of
‘outb’ makes integer from pointer without a cast

>> +             /* Probe and register SJA1000 */
>> +             priv->reg_base = (void __iomem *)sja1000;
>> +             if (!register_sja1000dev(netdev))
>> +                     break;
>> Hmm, we lose the return value of that function here... Do we really need
>> to continue if the region was succesfully requested and the sja1000dev
>> not?

Yes, the region may be free in Linux but may be disabled in hardware
(for example, the BIOS of the PC104 CPU board I tested this driver
with only enables a short few I/O port address ranges). The idea is to
keep trying all 8 possible addresses.

> Hm, does the simple probing of sja1000_probe_chip() work reliably?

Yes! :-)

> You can find a more intelligent test in the plx_pci driver.

I agree, but to avoid duplicating code, maybe we should create one
good probe function in sja1000.c that all modules could use, or maybe
simply replace sja1000_probe_chip() by plx_pci_check_sja1000() ?

> Anyway, why do you need to probe the IO port?
> Can't it be derived from the JP1:JP2 jumper settings?

I don't think so. There are 4x8=32 possible IO address configurations,
and only 2, maybe 3 or 4 max, jumpers available. And I didn't want to
use module parameters, I'd like this driver to be completely
automatic.

Thanks!
Here is the patch:

>From be8727e70f13198589cc1676f48560aa614ea1d0 Mon Sep 17 00:00:00 2001
From: Andre B. Oliveira <[email protected]>
Date: Sat, 9 Oct 2010 19:28:02 +0100
Subject: [PATCH net-next-2.6] can: tscan1: add driver for TS-CAN1 boards


Signed-off-by: Andre B. Oliveira <[email protected]>
---
 drivers/net/can/sja1000/Kconfig  |   12 ++
 drivers/net/can/sja1000/Makefile |    1 +
 drivers/net/can/sja1000/tscan1.c |  215 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 228 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/sja1000/tscan1.c

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index ae3505a..6fdc031 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -58,4 +58,16 @@ config CAN_PLX_PCI
           - esd CAN-PCIe/2000
           - Marathon CAN-bus-PCI card (http://www.marathon.ru/)
           - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
+
+config CAN_TSCAN1
+       tristate "TS-CAN1 PC104 boards"
+       depends on ISA
+       help
+       This driver is for Technologic Systems' TSCAN-1 PC104 boards.
+       http://www.embeddedarm.com/products/board-detail.php?product=TS-CAN1
+       The driver supports multiple boards and automatically configures them:
+       PLD IO base addresses are read from jumpers JP1 and JP2,
+       IRQ numbers are read from jumpers JP4 and JP5,
+       SJA1000 IO base addresses are chosen heuristically (first that works).
+
 endif
diff --git a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
index ce92455..2c591eb 100644
--- a/drivers/net/can/sja1000/Makefile
+++ b/drivers/net/can/sja1000/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_CAN_SJA1000_OF_PLATFORM) += sja1000_of_platform.o
 obj-$(CONFIG_CAN_EMS_PCI) += ems_pci.o
 obj-$(CONFIG_CAN_KVASER_PCI) += kvaser_pci.o
 obj-$(CONFIG_CAN_PLX_PCI) += plx_pci.o
+obj-$(CONFIG_CAN_TSCAN1) += tscan1.o

 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/sja1000/tscan1.c b/drivers/net/can/sja1000/tscan1.c
new file mode 100644
index 0000000..7754176
--- /dev/null
+++ b/drivers/net/can/sja1000/tscan1.c
@@ -0,0 +1,215 @@
+/*
+ * tscan1.c: driver for Technologic Systems TS-CAN1 PC104 boards
+ *
+ * Copyright 2010 Andre B. Oliveira
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * References:
+ * - Getting started with TS-CAN1, Technologic Systems, Jun 2009
+ *     http://www.embeddedarm.com/documentation/ts-can1-manual.pdf
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/isa.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include "sja1000.h"
+
+MODULE_DESCRIPTION("Driver for Technologic Systems TS-CAN1 PC104 boards");
+MODULE_AUTHOR("Andre B. Oliveira <[email protected]>");
+MODULE_LICENSE("GPL");
+
+/* Maximum number of boards (one in each JP1:JP2 setting of I/O address) */
+#define TSCAN1_MAXDEV 4
+
+/* PLD registers address offsets */
+#define TSCAN1_ID1     0
+#define TSCAN1_ID2     1
+#define TSCAN1_VERSION 2
+#define TSCAN1_LED     3
+#define TSCAN1_PAGE    4
+#define TSCAN1_MODE    5
+#define TSCAN1_JUMPERS 6
+
+/* PLD board identifier registers magic values */
+#define TSCAN1_ID1_VALUE 0xf6
+#define TSCAN1_ID2_VALUE 0xb9
+
+/* PLD mode register SJA1000 I/O enable bit */
+#define TSCAN1_MODE_ENABLE 0x40
+
+/* PLD jumpers register bits */
+#define TSCAN1_JP4 0x10
+#define TSCAN1_JP5 0x20
+
+/* PLD I/O base addresses start */
+#define TSCAN1_PLD_ADDRESS 0x150
+
+/* PLD register space size */
+#define TSCAN1_PLD_SIZE 8
+
+/* SJA1000 register space size */
+#define TSCAN1_SJA1000_SIZE 32
+
+/* SJA1000 crystal frequency (16MHz) */
+#define TSCAN1_SJA1000_XTAL 16000000
+
+/* SJA1000 indirect I/O base addresses */
+static const unsigned short tscan1_sja1000_addresses[] __devinitconst = {
+       0x100, 0x120, 0x180, 0x1a0, 0x200, 0x240, 0x280, 0x320
+};
+
+/* Read SJA1000 register */
+static u8 tscan1_read(const struct sja1000_priv *priv, int reg)
+{
+       return inb((int)priv->reg_base + reg);
+}
+
+/* Write SJA1000 register */
+static void tscan1_write(const struct sja1000_priv *priv, int reg, u8 val)
+{
+       outb(val, (int)priv->reg_base + reg);
+}
+
+/* Probe for a TSCAN1 device with JP1:JP2 jumper setting ID */
+static int __devinit tscan1_match(struct device *dev, unsigned id)
+{
+       int pld_base, sja1000_base, irq, i;
+       struct net_device *netdev;
+       struct sja1000_priv *priv;
+
+       /* Request PLD I/O register region */
+       pld_base = TSCAN1_PLD_ADDRESS + id * TSCAN1_PLD_SIZE;
+       if (!request_region(pld_base, TSCAN1_PLD_SIZE, "tscan1"))
+               return -EBUSY;
+
+       /* Check magic values in PLD board identifier registers */
+       if (inb(pld_base + TSCAN1_ID1) != TSCAN1_ID1_VALUE ||
+           inb(pld_base + TSCAN1_ID2) != TSCAN1_ID2_VALUE)
+               goto out_release;
+
+       /* Read JP4:JP5 jumper settings to find out the selected IRQ */
+       switch (inb(pld_base + TSCAN1_JUMPERS) & (TSCAN1_JP4 | TSCAN1_JP5)) {
+       case TSCAN1_JP4:
+               irq = 6;
+               break;
+       case TSCAN1_JP5:
+               irq = 7;
+               break;
+       case TSCAN1_JP4 | TSCAN1_JP5:
+               irq = 5;
+               break;
+       default:
+               goto out_release;
+       }
+
+       netdev = alloc_sja1000dev(0);
+       if (!netdev)
+               goto out_release;
+
+       dev_set_drvdata(dev, netdev);
+       SET_NETDEV_DEV(netdev, dev);
+
+       netdev->base_addr = pld_base;
+       netdev->irq = irq;
+
+       priv = netdev_priv(netdev);
+       priv->read_reg = tscan1_read;
+       priv->write_reg = tscan1_write;
+       priv->can.clock.freq = TSCAN1_SJA1000_XTAL / 2;
+       priv->cdr = CDR_CBP | CDR_CLK_OFF;
+       priv->ocr = OCR_TX0_PUSHPULL;
+
+       /* Select the first SJA1000 I/O address that is free and that works */
+       for (i = 0; i < ARRAY_SIZE(tscan1_sja1000_addresses); i++) {
+               sja1000_base = tscan1_sja1000_addresses[i];
+               if (!request_region(sja1000_base, TSCAN1_SJA1000_SIZE,
+                                                               "tscan1"))
+                       continue;
+
+               /* Set the SJA1000 I/O base address and enable it */
+               outb(TSCAN1_MODE_ENABLE | i, pld_base + TSCAN1_MODE);
+
+               /* Probe and register SJA1000 */
+               priv->reg_base = (void __iomem *)sja1000_base;
+               if (!register_sja1000dev(netdev)) {
+                       outb(0, pld_base + TSCAN1_LED); /* turn the LED off */
+                       netdev_info(netdev, "TS-CAN1 at 0x%x 0x%x irq %d\n",
+                                               pld_base, sja1000_base, irq);
+                       return 0;
+               }
+
+               /* The probe for an SJA1000 at this address failed: release */
+               outb(0, pld_base + TSCAN1_MODE);
+               release_region(sja1000_base, TSCAN1_SJA1000_SIZE);
+       }
+
+       dev_set_drvdata(dev, NULL);
+       free_sja1000dev(netdev);
+out_release:
+       release_region(pld_base, TSCAN1_PLD_SIZE);
+       return -ENODEV;
+}
+
+static int __devexit tscan1_remove(struct device *dev, unsigned id /*unused*/)
+{
+       struct net_device *netdev;
+       struct sja1000_priv *priv;
+       int pld_base, sja1000_base;
+
+       netdev = dev_get_drvdata(dev);
+       unregister_sja1000dev(netdev);
+       dev_set_drvdata(dev, NULL);
+
+       priv = netdev_priv(netdev);
+       pld_base = netdev->base_addr;
+       sja1000_base = (int)priv->reg_base;
+
+       /* Release SJA1000 I/O space */
+       outb(0, pld_base + TSCAN1_MODE);
+       release_region(sja1000_base, TSCAN1_SJA1000_SIZE);
+
+       /* Release PLD I/O space */
+       release_region(pld_base, TSCAN1_PLD_SIZE);
+
+       /* Free net device */
+       free_sja1000dev(netdev);
+
+       return 0;
+}
+
+static struct isa_driver tscan1_isa_driver = {
+       .match = tscan1_match,
+       .remove = __devexit_p(tscan1_remove),
+       .driver = {
+               .name = "tscan1",
+       },
+};
+
+static int __init tscan1_init(void)
+{
+       return isa_register_driver(&tscan1_isa_driver, TSCAN1_MAXDEV);
+}
+module_init(tscan1_init);
+
+static void __exit tscan1_exit(void)
+{
+       isa_unregister_driver(&tscan1_isa_driver);
+}
+module_exit(tscan1_exit);
-- 
1.6.5

-- 
Andre B. Oliveira
http://clientes.netvisao.pt/anbadeol/
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to