can: sja1000: misused netdev->base_addr and OF platform driver
Subsystems (5)
cover can: sja1000: misused netdev->base_addr and OF platform driver
Hello,
this patch series fixes some minor issues with the initial version of the CAN device driver interface, corrects misusing netdev->base_addr and adds the OF platform driver for the SJA1000.
Wolfgang.
PATCH 1/3 can: some fixes and cleanups to the initial device driver interface
This patch fixes a few errors sneaked into the initial version of the device driver interface.
Signed-off-by Wolfgang Grandegger <[email protected]>
--- drivers/net/can/dev.c | 2 +- drivers/net/can/sja1000/ems_pci.c | 3 --- drivers/net/can/sja1000/sja1000.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-)
drivers/net/can/dev.c +1 -1
@@ -477,7 +477,7 @@return 0 ; } -EXPORT_SYMBOL (open_candev); +EXPORT_SYMBOL_GPL (open_candev);/* * Common close function for cleanup before the device gets closed.
drivers/net/can/sja1000/ems_pci.c +0 -3
@@ -218,14 +218,12 @@ card->conf_addr =pci_iomap (pdev,0 ,EMS_PCI_MEM_SIZE );if (card->conf_addr ==NULL ) { err = -ENOMEM ; - goto failure_cleanup; } card->base_addr =pci_iomap (pdev,1 ,EMS_PCI_MEM_SIZE );if (card->base_addr ==NULL ) { err = -ENOMEM ; - goto failure_cleanup; }
@@ -239,7 +237,6 @@ ems_pci_readb(card, 3) != 0xCB || ems_pci_readb(card, 4) != 0x11) {dev_err (&pdev->dev,"Not EMS Dr. Thomas Wuensche interface\n" ); - err = -ENODEV ; goto failure_cleanup; }
drivers/net/can/sja1000/sja1000.c +1 -1
@@ -532,8 +532,8 @@ err =request_irq (dev->irq, &sja1000_interrupt, priv->irq_flags, dev->name, (void *)dev);if (err) { -return -EAGAIN ;close_candev (dev); +return -EAGAIN ; } }
PATCH 2/3 can: sja1000: stop misusing member base_addr of struct net_device
In reply to Wolfgang GrandeggerAs discussed on the netdev mailing list, the member "base_addr" of "struct net_device" should not be (mis)used to store the virtual address to the SJA1000 register area. According to David Miller, it's only use is to allow ISA and similar primitive bus devices to have their I/O ports changed via ifconfig. The virtual address is now stored in the private data structure of the SJA1000 device and the callback functions use "struct sja1000_priv" instead of the unneeded "struct net_device".
Signed-off-by Wolfgang Grandegger <[email protected]>
--- drivers/net/can/sja1000/ems_pci.c | 36 +++------ drivers/net/can/sja1000/kvaser_pci.c | 21 ++--- drivers/net/can/sja1000/sja1000.c | 110 ++++++++++++++--------------- drivers/net/can/sja1000/sja1000.h | 9 +- drivers/net/can/sja1000/sja1000_platform.c | 19 ++--- 5 files changed, 96 insertions(+), 99 deletions(-)
drivers/net/can/sja1000/ems_pci.c +15 -21
@@ -99,25 +99,21 @@*/ static u8 ems_pci_readb (struct ems_pci_card *card,unsigned int port) { -return readb ((void __iomem *)card->base_addr - + (port *EMS_PCI_PORT_BYTES )); +return readb (card->base_addr + (port *EMS_PCI_PORT_BYTES )); } -static u8 ems_pci_read_reg (const struct net_device *dev,int port) +static u8 ems_pci_read_reg (const struct sja1000_priv *priv,int port) { -return readb ((void __iomem *)dev->base_addr - + (port *EMS_PCI_PORT_BYTES )); +return readb (priv->reg_base + (port *EMS_PCI_PORT_BYTES )); } -static void ems_pci_write_reg (const struct net_device *dev,int port,u8 val) +static void ems_pci_write_reg (const struct sja1000_priv *priv,int port,u8 val) { -writeb (val, (void __iomem *)dev->base_addr - + (port *EMS_PCI_PORT_BYTES )); +writeb (val, priv->reg_base + (port *EMS_PCI_PORT_BYTES )); } -static void ems_pci_post_irq (const struct net_device *dev) +static void ems_pci_post_irq (const struct sja1000_priv *priv) { -struct sja1000_priv *priv =netdev_priv (dev);struct ems_pci_card *card = (struct ems_pci_card *)priv->priv;/* reset int flag of pita */
@@ -129,17 +125,17 @@* Check if a CAN controller is present at the specified location * by trying to set 'em into the PeliCAN mode */ -static inline int ems_pci_check_chan (struct net_device *dev) +static inline int ems_pci_check_chan (const struct sja1000_priv *priv) {unsigned char res;/* Make sure SJA1000 is in reset mode */ -ems_pci_write_reg (dev,REG_MOD ,1 ); +ems_pci_write_reg (priv,REG_MOD ,1 ); -ems_pci_write_reg (dev,REG_CDR ,CDR_PELICAN ); +ems_pci_write_reg (priv,REG_CDR ,CDR_PELICAN );/* read reset-values */ - res =ems_pci_read_reg (dev,REG_CDR ); + res =ems_pci_read_reg (priv,REG_CDR );if (res ==CDR_PELICAN )return 1 ;
@@ -257,12 +253,11 @@ priv->irq_flags =IRQF_SHARED ; dev->irq = pdev->irq; - dev->base_addr = (unsigned long )(card->base_addr - +EMS_PCI_CAN_BASE_OFFSET - + (i *EMS_PCI_CAN_CTRL_SIZE )); + priv->reg_base = card->base_addr +EMS_PCI_CAN_BASE_OFFSET + + (i *EMS_PCI_CAN_CTRL_SIZE );/* Check if channel is present */ -if (ems_pci_check_chan (dev)) { +if (ems_pci_check_chan (priv)) { priv->read_reg = ems_pci_read_reg; priv->write_reg = ems_pci_write_reg; priv->post_irq = ems_pci_post_irq;
@@ -286,9 +281,8 @@ card->channels++; -dev_info (&pdev->dev,"Channel #%d at %#lX, irq %d\n" , - i +1 , dev->base_addr, - dev->irq); +dev_info (&pdev->dev,"Channel #%d at 0x%p, irq %d\n" , + i +1 , priv->reg_base, dev->irq); }else {free_sja1000dev (dev); }
drivers/net/can/sja1000/kvaser_pci.c +11 -10
@@ -117,14 +117,15 @@MODULE_DEVICE_TABLE (pci, kvaser_pci_tbl); -static u8 kvaser_pci_read_reg (const struct net_device *dev,int port) +static u8 kvaser_pci_read_reg (const struct sja1000_priv *priv,int port) { -return ioread8 ((void __iomem *)(dev->base_addr + port)); +return ioread8 (priv->reg_base + port); } -static void kvaser_pci_write_reg (const struct net_device *dev,int port,u8 val) +static void kvaser_pci_write_reg (const struct sja1000_priv *priv, +int port,u8 val) { -iowrite8 (val, (void __iomem *)(dev->base_addr + port)); +iowrite8 (val, priv->reg_base + port); }static void kvaser_pci_disable_irq (struct net_device *dev)
@@ -199,7 +200,7 @@ }unregister_sja1000dev (dev); -pci_iounmap (board->pci_dev, (void __iomem *)dev->base_addr); +pci_iounmap (board->pci_dev, priv->reg_base);pci_iounmap (board->pci_dev, board->conf_addr);pci_iounmap (board->pci_dev, board->res_addr);
@@ -210,7 +211,7 @@struct net_device **master_dev, void __iomem *conf_addr, void __iomem *res_addr, - unsigned long base_addr) + void __iomem *base_addr) { struct net_device *dev;struct sja1000_priv *priv;
@@ -252,7 +253,7 @@ board->xilinx_ver = master_board->xilinx_ver; } - dev->base_addr = base_addr + channel *KVASER_PCI_PORT_BYTES ; + priv->reg_base = base_addr + channel *KVASER_PCI_PORT_BYTES ; priv->read_reg = kvaser_pci_read_reg; priv->write_reg = kvaser_pci_write_reg;
@@ -267,8 +268,8 @@ init_step =4 ; -dev_info (&pdev->dev,"base_addr=%#lx conf_addr=%p irq=%d\n" , - dev->base_addr, board->conf_addr, dev->irq); +dev_info (&pdev->dev,"reg_base=%p conf_addr=%p irq=%d\n" , + priv->reg_base, board->conf_addr, dev->irq);SET_NETDEV_DEV (dev, &pdev->dev);
@@ -343,7 +344,7 @@for (i =0 ; i < no_channels; i++) { err =kvaser_pci_add_chan (pdev, i, &master_dev, conf_addr, res_addr, - (unsigned long )base_addr); + base_addr);if (err) goto failure_cleanup; }
drivers/net/can/sja1000/sja1000.c +55 -55
260 lines not on this page · raw message
drivers/net/can/sja1000/sja1000.h +5 -4
@@ -155,14 +155,15 @@struct sk_buff *echo_skb;/* the lower-layer is responsible for appropriate locking */ -u8 (*read_reg) (const struct net_device *dev,int reg); -void (*write_reg) (const struct net_device *dev,int reg,u8 val); -void (*pre_irq) (const struct net_device *dev); -void (*post_irq) (const struct net_device *dev); +u8 (*read_reg) (const struct sja1000_priv *priv,int reg); +void (*write_reg) (const struct sja1000_priv *priv,int reg,u8 val); +void (*pre_irq) (const struct sja1000_priv *priv); +void (*post_irq) (const struct sja1000_priv *priv);void *priv;/* for board-specific data */ struct net_device *dev; +void __iomem *reg_base;/* ioremap'ed address to registers */ unsigned long irq_flags;/* for request_irq() */ u16 flags;/* custom mode flags */
drivers/net/can/sja1000/sja1000_platform.c +10 -9
@@ -37,14 +37,14 @@MODULE_DESCRIPTION ("Socket-CAN driver for SJA1000 on the platform bus" );MODULE_LICENSE ("GPL v2" ); -static u8 sp_read_reg (const struct net_device *dev,int reg) +static u8 sp_read_reg (const struct sja1000_priv *priv,int reg) { -return ioread8 ((void __iomem *)(dev->base_addr + reg)); +return ioread8 (priv->reg_base + reg); } -static void sp_write_reg (const struct net_device *dev,int reg,u8 val) +static void sp_write_reg (const struct sja1000_priv *priv,int reg,u8 val) { -iowrite8 (val, (void __iomem *)(dev->base_addr + reg)); +iowrite8 (val, priv->reg_base + reg); }static int sp_probe (struct platform_device *pdev)
@@ -89,9 +89,9 @@ } priv =netdev_priv (dev); - dev->base_addr = (unsigned long )addr; dev->irq = res_irq->start; priv->irq_flags = res_irq->flags &IRQF_TRIGGER_MASK ; + priv->reg_base = addr; priv->read_reg = sp_read_reg; priv->write_reg = sp_write_reg; priv->can.clock.freq = pdata->clock;
@@ -108,8 +108,8 @@ goto exit_free; } -dev_info (&pdev->dev,"%s device registered (base_addr=%#lx, irq=%d)\n" , -DRV_NAME , dev->base_addr, dev->irq); +dev_info (&pdev->dev,"%s device registered (reg_base=%p, irq=%d)\n" , +DRV_NAME , priv->reg_base, dev->irq);return 0 ; exit_free:
@@ -125,13 +125,14 @@static int sp_remove (struct platform_device *pdev) {struct net_device *dev =dev_get_drvdata (&pdev->dev); +struct sja1000_priv *priv =netdev_priv (dev);struct resource *res;unregister_sja1000dev (dev);dev_set_drvdata (&pdev->dev,NULL ); -if (dev->base_addr) -iounmap ((void __iomem *)dev->base_addr); +if (priv->reg_base) +iounmap (priv->reg_base); res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );release_mem_region (res->start,resource_size (res));
PATCH 3/3 can: sja1000: generic OF platform bus driver
In reply to Wolfgang GrandeggerThis patch adds a generic driver for SJA1000 chips on the OpenFirmware platform bus found on embedded PowerPC systems. You need a SJA1000 node definition in your flattened device tree source (DTS) file similar to:
can@3,100 {
compatible = "nxp,sja1000";
reg = <3 0x100 0x80>;
interrupts = <2 0>;
interrupt-parent = <&mpic>;
nxp,external-clock-frequency = <16000000>;
};See also Documentation/powerpc/dts-bindings/can/sja1000.txt.
Signed-off-by Wolfgang Grandegger <[email protected]>
--- Documentation/powerpc/dts-bindings/can/sja1000.txt | 53 ++++ drivers/net/can/Kconfig | 9 drivers/net/can/sja1000/Makefile | 1 drivers/net/can/sja1000/sja1000_of_platform.c | 233 +++++++++++++++++++++ include/linux/can/platform/sja1000.h | 3 5 files changed, 299 insertions(+)
drivers/net/can/Kconfig +9 -0
@@ -51,6 +51,15 @@ boards from Phytec (http://www.phytec.de) like the PCM027, PCM038. +config CAN_SJA1000_OF_PLATFORM + depends on CAN_SJA1000 && PPC_OF + tristate "Generic OF Platform Bus based SJA1000 driver" + ---help--- + This driver adds support for the SJA1000 chips connected to + the OpenFirmware "platform bus" found on embedded systems with + OpenFirmware bindings, e.g. if you have a PowerPC based system + you may want to enable this option. + config CAN_EMS_PCI tristate "EMS CPC-PCI and CPC-PCIe Card" depends on PCI && CAN_SJA1000
drivers/net/can/sja1000/Makefile +1 -0
@@ -4,6 +4,7 @@ obj-$(CONFIG_CAN_SJA1000) += sja1000.o obj-$(CONFIG_CAN_SJA1000_PLATFORM) += sja1000_platform.o +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
drivers/net/can/sja1000/sja1000_of_platform.c new file +233 -0
233 lines not on this page · raw message
Documentation/powerpc/dts-bindings/can/sja1000.txt new file +53 -0
53 lines not on this page · raw message
include/linux/can/platform/sja1000.h +3 -0
@@ -13,6 +13,7 @@#define OCR_MODE_TEST 0x01#define OCR_MODE_NORMAL 0x02#define OCR_MODE_CLOCK 0x03 +#define OCR_MODE_MASK 0x07#define OCR_TX0_INVERT 0x04#define OCR_TX0_PULLDOWN 0x08#define OCR_TX0_PULLUP 0x10
@@ -21,6 +22,8 @@#define OCR_TX1_PULLDOWN 0x40#define OCR_TX1_PULLUP 0x80#define OCR_TX1_PUSHPULL 0xc0 +#define OCR_TX_MASK 0xfc +#define OCR_TX_SHIFT 2struct sja1000_platform_data {u32 clock;/* CAN bus oscillator frequency in Hz */
David Miller
maintainerFrom: Wolfgang Grandegger <[email protected]>
Date: Sat, 30 May 2009 19:55:47 +0200
Quoted 3 lines
this patch series fixes some minor issues with the initial version of the CAN device driver interface, corrects misusing
netdev->base_addrand adds the OF platform driver for the SJA1000.
All applied to net-next-2.6, thanks!