On Wed, Oct 06, 2010 at 11:49:28AM +0100, Andre B. Oliveira wrote: > Sorry, again! Here is the patch, inlined:
Thanks! BTW, don't be shy; you don't have to have a "name" on netdev or some other lists. Good code is enough, and from a glimpse, your code is not far away from inclusion :) My first comments: > diff -uprN -X linux-2.6.35.4/Documentation/dontdiff > linux-2.6.35.4/drivers/net/can/sja1000/Kconfig > linux-2.6.35.4-tscan1/drivers/net/can/sja1000/Kconfig > --- linux-2.6.35.4/drivers/net/can/sja1000/Kconfig 2010-08-27 > 00:47:12.000000000 +0100 > +++ linux-2.6.35.4-tscan1/drivers/net/can/sja1000/Kconfig 2010-09-15 > 15:59:46.000000000 +0100 > @@ -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 -uprN -X linux-2.6.35.4/Documentation/dontdiff > linux-2.6.35.4/drivers/net/can/sja1000/Makefile > linux-2.6.35.4-tscan1/drivers/net/can/sja1000/Makefile > --- linux-2.6.35.4/drivers/net/can/sja1000/Makefile 2010-08-27 > 00:47:12.000000000 +0100 > +++ linux-2.6.35.4-tscan1/drivers/net/can/sja1000/Makefile 2010-09-06 > 11:08:23.000000000 +0100 > @@ -9,5 +9,6 @@ obj-$(CONFIG_CAN_SJA1000_OF_PLATFORM) += > 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 -uprN -X linux-2.6.35.4/Documentation/dontdiff > linux-2.6.35.4/drivers/net/can/sja1000/tscan1.c > linux-2.6.35.4-tscan1/drivers/net/can/sja1000/tscan1.c > --- linux-2.6.35.4/drivers/net/can/sja1000/tscan1.c 1970-01-01 > 01:00:00.000000000 +0100 > +++ linux-2.6.35.4-tscan1/drivers/net/can/sja1000/tscan1.c 2010-09-15 > 16:10:26.000000000 +0100 > @@ -0,0 +1,184 @@ > +/* > + * linux/drivers/net/can/sja1000/tscan1.c - driver for 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 3 of the License, or > + * (at your option) any later version. Uuuh, is GPL V3+ accepted in the kernel? Surely, if you could relicense it to GPL V2+, it will make a lot of things easier. > + * > + * 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 <asm/io.h> Please place this after the linux/ ones > +#include <linux/init.h> > +#include <linux/ioport.h> > +#include <linux/isa.h> > +#include <linux/module.h> > +#include <linux/netdevice.h> > +#include "sja1000.h" > + > +MODULE_DESCRIPTION("Driver for TS-CAN1 PC104 CAN interface boards"); > +MODULE_AUTHOR("Andre B. Oliveira <[email protected]>"); > +MODULE_LICENSE("GPL"); > + > +/* PLD register space size */ > +#define TSCAN1_PLD_SIZE 8 > + > +/* SJA1000 register space size */ > +#define TSCAN1_SJA1000_SIZE 32 > + > +/* 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 selection ID */ > +static int tscan1_match(struct device *dev, unsigned id) > +{ > + int pld, sja1000, irq, i; > + struct net_device *netdev; > + struct sja1000_priv *priv; > + > + /* Request PLD IO register region */ > + pld = 0x150 + id * 8; > + if (!request_region(pld, TSCAN1_PLD_SIZE, "tscan1")) > + return -EBUSY; > + > + /* Check magic values in PLD board identifier bytes */ > + if (inb(pld) != 0xf6 || inb(pld + 1) != 0xb9) I'd prefer #defines for the register names instead of "magic" numbers (like pld + 1). > + goto err1; Maybe something more descriptive than err1? > + > + /* Read JP4:JP5 jumper settings to find out the selected IRQ */ > + switch (inb(pld + 6) & 0x30) { > + case 0x10: irq = 6; break; > + case 0x20: irq = 7; break; > + case 0x30: irq = 5; break; > + default: goto err1; > + } > + > + netdev = alloc_sja1000dev(0); > + if (!netdev) > + goto err1; > + > + dev_set_drvdata(dev, netdev); > + SET_NETDEV_DEV(netdev, dev); > + > + netdev->base_addr = pld; > + netdev->irq = irq; > + > + priv = netdev_priv(netdev); > + priv->read_reg = tscan1_read; > + priv->write_reg = tscan1_write; > + priv->can.clock.freq = 16000000 / 2; > + priv->cdr = 0x48; /* CBP | CLKOFF */ > + priv->ocr = 0x18; /* TP0 | TN0 */ > + > + /* Select the first SJA1000 IO address that is free and that works */ > + for (i = 0; i < 8; i++) { > + const unsigned short addr[] = { > + 0x100, 0x120, 0x180, 0x1a0, 0x200, 0x240, 0x280, 0x320 > + }; > + > + sja1000 = addr[i]; > + > + if (!request_region(sja1000, TSCAN1_SJA1000_SIZE, "tscan1")) > + continue; > + > + /* Set the SJA1000 IO base address and enable it */ > + outb(0x40 | i, pld + 5); > + > + /* 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? > + > + /* The probe for an SJA1000 at this address failed: release */ > + outb(0, pld + 5); > + release_region(sja1000, TSCAN1_SJA1000_SIZE); > + } > + if (i == 8) > + goto err2; I somehow have the feeling that the above loop should be reworked. That 'if (!register) break;' is opposite to what one is used to. > + > + /* Turn the LED off */ > + outb(0, pld + 3); > + > + printk(KERN_INFO "%s: TS-CAN1 at 0x%x 0x%x irq %d\n", > + netdev->name, pld, sja1000, irq); (net)dev_info? > + > + return 0; > + > +err2: dev_set_drvdata(dev, NULL); labels for goto in a seperate line, please. > + free_sja1000dev(netdev); > +err1: release_region(pld, TSCAN1_PLD_SIZE); > + return -ENODEV; > +} > + > +/* Remove TSCAN1 device */ Obvious comment :) > +static int __devexit tscan1_remove(struct device *dev, unsigned id > /*unused*/) I wonder (don't know ISA) if match could bei __devinit if this one can be __devexit? > +{ > + struct net_device *netdev; > + struct sja1000_priv *priv; > + int pld, sja1000; > + > + netdev = dev_get_drvdata(dev); > + unregister_sja1000dev(netdev); > + dev_set_drvdata(dev, NULL); > + > + priv = netdev_priv(netdev); > + pld = netdev->base_addr; > + sja1000 = (int)priv->reg_base; > + > + /* Release SJA1000 IO space */ > + outb(0, pld + 5); > + release_region(sja1000, TSCAN1_SJA1000_SIZE); > + > + /* Release PLD IO space */ > + release_region(pld, 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", > + }, > +}; > + > +/* Register 4 devices, one for each JP1:JP2 jumper selection of IO address */ > +static int __init tscan1_init(void) > +{ > + return isa_register_driver(&tscan1_isa_driver, 4); > +} > +module_init(tscan1_init); > + > +static void __exit tscan1_exit(void) > +{ > + isa_unregister_driver(&tscan1_isa_driver); > +} > +module_exit(tscan1_exit); > _______________________________________________ > Socketcan-core mailing list > [email protected] > https://lists.berlios.de/mailman/listinfo/socketcan-core -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ |
signature.asc
Description: Digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
