[PATCH v3 0/5] can: sja1000: cleanups and new OF property

[PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

Hello,

(could someone with a SJA1000 on SPARC perform a functional test
to see if interrupts are working? it would be great :-)

Changes since v2:
- Dropped patch 1 "can: sja1000: remove unused defines"
- Addressed Marc's comments on patch 4 and 6 (now 3 and 5)

Changes since v1:
- Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)

The first part of this series performs serveral small cleanups
(patches 1 and 2).

Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
Changes are pretty conservatives (mostly copy/paste/move). IRQ
is treated differently in the OF and non-OF versions, thus this
is where the fused version differs the most.

The final part introduces the 'reg-io-width' binding (already used
by some other drivers) to perform a similar job as what was done
with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
take into account the aliasing of the address bus.

All patches were tested using OF boot on my OMAP3 system with a
memory-mapped SJA1000. Thus, the non-OF path is not tested, as
I do not have a platform data at hand.

Regards,
Florian

v1: http://thread.gmane.org/gmane.linux.kernel/1637835
v2: http://thread.gmane.org/gmane.linux.can/4831

Florian Vaussard (5):
  can: sja1000: convert printk to use netdev API
  can: sja1000: platform: use devm_* APIs
  can: sja1000: fuse of_platform into platform
  Documentation: devicetree: sja1000: add reg-io-width binding
  can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit
    register access

 .../devicetree/bindings/net/can/sja1000.txt        |   4 +
 drivers/net/can/sja1000/Kconfig                    |  13 +-
 drivers/net/can/sja1000/Makefile                   |   1 -
 drivers/net/can/sja1000/sja1000.c                  |   3 +-
 drivers/net/can/sja1000/sja1000_of_platform.c      | 221 ---------------------
 drivers/net/can/sja1000/sja1000_platform.c         | 190 ++++++++++++------
 6 files changed, 137 insertions(+), 295 deletions(-)
 delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c

-- 
1.8.1.2

[PATCH v3 1/5] can: sja1000: convert printk to use netdev API

[Florian Vaussard]

Use netdev_* where applicable.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index f17c301..55cce47 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -106,8 +106,7 @@ static int sja1000_probe_chip(struct net_device *dev)
 	struct sja1000_priv *priv = netdev_priv(dev);
 
 	if (priv->reg_base && sja1000_is_absent(priv)) {
-		printk(KERN_INFO "%s: probing @0x%lX failed\n",
-		       DRV_NAME, dev->base_addr);
+		netdev_err(dev, "probing failed\n");
 		return 0;
 	}
 	return -1;
-- 
1.8.1.2

[PATCH v3 2/5] can: sja1000: platform: use devm_* APIs

[Florian Vaussard]

Simplify probe and remove functions by converting most of the resources
to use devm_* APIs.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000_platform.c | 46 ++++++++----------------------
 1 file changed, 12 insertions(+), 34 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 29f9b63..779679a 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -79,34 +79,26 @@ static int sp_probe(struct platform_device *pdev)
 	pdata = dev_get_platdata(&pdev->dev);
 	if (!pdata) {
 		dev_err(&pdev->dev, "No platform data provided!\n");
-		err = -ENODEV;
-		goto exit;
+		return -ENODEV;
 	}
 
 	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res_mem || !res_irq) {
-		err = -ENODEV;
-		goto exit;
-	}
+	if (!res_mem || !res_irq)
+		return -ENODEV;
 
-	if (!request_mem_region(res_mem->start, resource_size(res_mem),
-				DRV_NAME)) {
-		err = -EBUSY;
-		goto exit;
-	}
+	if (!devm_request_mem_region(&pdev->dev, res_mem->start,
+				     resource_size(res_mem), DRV_NAME))
+		return -EBUSY;
 
-	addr = ioremap_nocache(res_mem->start, resource_size(res_mem));
-	if (!addr) {
-		err = -ENOMEM;
-		goto exit_release;
-	}
+	addr = devm_ioremap_nocache(&pdev->dev, res_mem->start,
+				    resource_size(res_mem));
+	if (!addr)
+		return -ENOMEM;
 
 	dev = alloc_sja1000dev(0);
-	if (!dev) {
-		err = -ENOMEM;
-		goto exit_iounmap;
-	}
+	if (!dev)
+		return -ENOMEM;
 	priv = netdev_priv(dev);
 
 	dev->irq = res_irq->start;
@@ -151,28 +143,14 @@ static int sp_probe(struct platform_device *pdev)
 
  exit_free:
 	free_sja1000dev(dev);
- exit_iounmap:
-	iounmap(addr);
- exit_release:
-	release_mem_region(res_mem->start, resource_size(res_mem));
- exit:
 	return err;
 }
 
 static int sp_remove(struct platform_device *pdev)
 {
 	struct net_device *dev = platform_get_drvdata(pdev);
-	struct sja1000_priv *priv = netdev_priv(dev);
-	struct resource *res;
 
 	unregister_sja1000dev(dev);
-
-	if (priv->reg_base)
-		iounmap(priv->reg_base);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
 	free_sja1000dev(dev);
 
 	return 0;
-- 
1.8.1.2

[PATCH v3 3/5] can: sja1000: fuse of_platform into platform

[Florian Vaussard]

The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/Kconfig               |  13 +-
 drivers/net/can/sja1000/Makefile              |   1 -
 drivers/net/can/sja1000/sja1000_of_platform.c | 221 --------------------------
 drivers/net/can/sja1000/sja1000_platform.c    | 134 ++++++++++++----
 4 files changed, 109 insertions(+), 260 deletions(-)
 delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c

diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
index ff2ba86..4b18b87 100644
--- a/drivers/net/can/sja1000/Kconfig
+++ b/drivers/net/can/sja1000/Kconfig
@@ -17,16 +17,9 @@ config CAN_SJA1000_PLATFORM
 	  the "platform bus" (Linux abstraction for directly to the
 	  processor attached devices).  Which can be found on various
 	  boards from Phytec (http://www.phytec.de) like the PCM027,
-	  PCM038.
-
-config CAN_SJA1000_OF_PLATFORM
-	tristate "Generic OF Platform Bus based SJA1000 driver"
-	depends on OF
-	---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.
+	  PCM038. It also provides 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_PCMCIA
 	tristate "EMS CPC-CARD Card"
diff --git a/drivers/net/can/sja1000/Makefile b/drivers/net/can/sja1000/Makefile
index b3d05cb..531d5fc 100644
--- a/drivers/net/can/sja1000/Makefile
+++ b/drivers/net/can/sja1000/Makefile
@@ -5,7 +5,6 @@
 obj-$(CONFIG_CAN_SJA1000) += sja1000.o
 obj-$(CONFIG_CAN_SJA1000_ISA) += sja1000_isa.o
 obj-$(CONFIG_CAN_SJA1000_PLATFORM) += sja1000_platform.o
-obj-$(CONFIG_CAN_SJA1000_OF_PLATFORM) += sja1000_of_platform.o
 obj-$(CONFIG_CAN_EMS_PCMCIA) += ems_pcmcia.o
 obj-$(CONFIG_CAN_EMS_PCI) += ems_pci.o
 obj-$(CONFIG_CAN_KVASER_PCI) += kvaser_pci.o
diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
deleted file mode 100644
index 047accd..0000000
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ /dev/null
@@ -1,221 +0,0 @@
-/*
- * Driver for SJA1000 CAN controllers on the OpenFirmware platform bus
- *
- * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the version 2 of the GNU General Public License
- * as published by the Free Software Foundation
- *
- * 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, write to the Free Software Foundation,
- * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- */
-
-/* This is a generic driver for SJA1000 chips on the OpenFirmware platform
- * bus found on embedded PowerPC systems. You need a SJA1000 CAN 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 "Documentation/devicetree/bindings/net/can/sja1000.txt" for further
- * information.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/netdevice.h>
-#include <linux/delay.h>
-#include <linux/io.h>
-#include <linux/can/dev.h>
-
-#include <linux/of_platform.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
-
-#include "sja1000.h"
-
-#define DRV_NAME "sja1000_of_platform"
-
-MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
-MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the OF platform bus");
-MODULE_LICENSE("GPL v2");
-
-#define SJA1000_OFP_CAN_CLOCK  (16000000 / 2)
-
-#define SJA1000_OFP_OCR        OCR_TX0_PULLDOWN
-#define SJA1000_OFP_CDR        (CDR_CBP | CDR_CLK_OFF)
-
-static u8 sja1000_ofp_read_reg(const struct sja1000_priv *priv, int reg)
-{
-	return ioread8(priv->reg_base + reg);
-}
-
-static void sja1000_ofp_write_reg(const struct sja1000_priv *priv,
-				  int reg, u8 val)
-{
-	iowrite8(val, priv->reg_base + reg);
-}
-
-static int sja1000_ofp_remove(struct platform_device *ofdev)
-{
-	struct net_device *dev = platform_get_drvdata(ofdev);
-	struct sja1000_priv *priv = netdev_priv(dev);
-	struct device_node *np = ofdev->dev.of_node;
-	struct resource res;
-
-	unregister_sja1000dev(dev);
-	free_sja1000dev(dev);
-	iounmap(priv->reg_base);
-	irq_dispose_mapping(dev->irq);
-
-	of_address_to_resource(np, 0, &res);
-	release_mem_region(res.start, resource_size(&res));
-
-	return 0;
-}
-
-static int sja1000_ofp_probe(struct platform_device *ofdev)
-{
-	struct device_node *np = ofdev->dev.of_node;
-	struct net_device *dev;
-	struct sja1000_priv *priv;
-	struct resource res;
-	u32 prop;
-	int err, irq, res_size;
-	void __iomem *base;
-
-	err = of_address_to_resource(np, 0, &res);
-	if (err) {
-		dev_err(&ofdev->dev, "invalid address\n");
-		return err;
-	}
-
-	res_size = resource_size(&res);
-
-	if (!request_mem_region(res.start, res_size, DRV_NAME)) {
-		dev_err(&ofdev->dev, "couldn't request %pR\n", &res);
-		return -EBUSY;
-	}
-
-	base = ioremap_nocache(res.start, res_size);
-	if (!base) {
-		dev_err(&ofdev->dev, "couldn't ioremap %pR\n", &res);
-		err = -ENOMEM;
-		goto exit_release_mem;
-	}
-
-	irq = irq_of_parse_and_map(np, 0);
-	if (irq == 0) {
-		dev_err(&ofdev->dev, "no irq found\n");
-		err = -ENODEV;
-		goto exit_unmap_mem;
-	}
-
-	dev = alloc_sja1000dev(0);
-	if (!dev) {
-		err = -ENOMEM;
-		goto exit_dispose_irq;
-	}
-
-	priv = netdev_priv(dev);
-
-	priv->read_reg = sja1000_ofp_read_reg;
-	priv->write_reg = sja1000_ofp_write_reg;
-
-	err = of_property_read_u32(np, "nxp,external-clock-frequency", &prop);
-	if (!err)
-		priv->can.clock.freq = prop / 2;
-	else
-		priv->can.clock.freq = SJA1000_OFP_CAN_CLOCK; /* default */
-
-	err = of_property_read_u32(np, "nxp,tx-output-mode", &prop);
-	if (!err)
-		priv->ocr |= prop & OCR_MODE_MASK;
-	else
-		priv->ocr |= OCR_MODE_NORMAL; /* default */
-
-	err = of_property_read_u32(np, "nxp,tx-output-config", &prop);
-	if (!err)
-		priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
-	else
-		priv->ocr |= OCR_TX0_PULLDOWN; /* default */
-
-	err = of_property_read_u32(np, "nxp,clock-out-frequency", &prop);
-	if (!err && prop) {
-		u32 divider = priv->can.clock.freq * 2 / prop;
-
-		if (divider > 1)
-			priv->cdr |= divider / 2 - 1;
-		else
-			priv->cdr |= CDR_CLKOUT_MASK;
-	} else {
-		priv->cdr |= CDR_CLK_OFF; /* default */
-	}
-
-	if (!of_property_read_bool(np, "nxp,no-comparator-bypass"))
-		priv->cdr |= CDR_CBP; /* default */
-
-	priv->irq_flags = IRQF_SHARED;
-	priv->reg_base = base;
-
-	dev->irq = irq;
-
-	dev_info(&ofdev->dev,
-		 "reg_base=0x%p irq=%d clock=%d ocr=0x%02x cdr=0x%02x\n",
-		 priv->reg_base, dev->irq, priv->can.clock.freq,
-		 priv->ocr, priv->cdr);
-
-	platform_set_drvdata(ofdev, dev);
-	SET_NETDEV_DEV(dev, &ofdev->dev);
-
-	err = register_sja1000dev(dev);
-	if (err) {
-		dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
-			DRV_NAME, err);
-		goto exit_free_sja1000;
-	}
-
-	return 0;
-
-exit_free_sja1000:
-	free_sja1000dev(dev);
-exit_dispose_irq:
-	irq_dispose_mapping(irq);
-exit_unmap_mem:
-	iounmap(base);
-exit_release_mem:
-	release_mem_region(res.start, res_size);
-
-	return err;
-}
-
-static struct of_device_id sja1000_ofp_table[] = {
-	{.compatible = "nxp,sja1000"},
-	{},
-};
-MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
-
-static struct platform_driver sja1000_ofp_driver = {
-	.driver = {
-		.owner = THIS_MODULE,
-		.name = DRV_NAME,
-		.of_match_table = sja1000_ofp_table,
-	},
-	.probe = sja1000_ofp_probe,
-	.remove = sja1000_ofp_remove,
-};
-
-module_platform_driver(sja1000_ofp_driver);
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 779679a..50dece8 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -27,12 +27,16 @@
 #include <linux/can/dev.h>
 #include <linux/can/platform/sja1000.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
 
 #include "sja1000.h"
 
 #define DRV_NAME "sja1000_platform"
+#define SP_CAN_CLOCK  (16000000 / 2)
 
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
+MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
 MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
 MODULE_ALIAS("platform:" DRV_NAME);
 MODULE_LICENSE("GPL v2");
@@ -67,24 +71,92 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
 	iowrite8(val, priv->reg_base + reg * 4);
 }
 
-static int sp_probe(struct platform_device *pdev)
+static void sp_populate(struct sja1000_priv *priv,
+			struct sja1000_platform_data *pdata,
+			unsigned long resource_mem_flags)
+{
+	/* The CAN clock frequency is half the oscillator clock frequency */
+	priv->can.clock.freq = pdata->osc_freq / 2;
+	priv->ocr = pdata->ocr;
+	priv->cdr = pdata->cdr;
+
+	switch (resource_mem_flags & IORESOURCE_MEM_TYPE_MASK) {
+	case IORESOURCE_MEM_32BIT:
+		priv->read_reg = sp_read_reg32;
+		priv->write_reg = sp_write_reg32;
+		break;
+	case IORESOURCE_MEM_16BIT:
+		priv->read_reg = sp_read_reg16;
+		priv->write_reg = sp_write_reg16;
+		break;
+	case IORESOURCE_MEM_8BIT:
+	default:
+		priv->read_reg = sp_read_reg8;
+		priv->write_reg = sp_write_reg8;
+		break;
+	}
+}
+
+static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
 {
 	int err;
+	u32 prop;
+
+	priv->read_reg = sp_read_reg8;
+	priv->write_reg = sp_write_reg8;
+
+	err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
+	if (!err)
+		priv->can.clock.freq = prop / 2;
+	else
+		priv->can.clock.freq = SP_CAN_CLOCK; /* default */
+
+	err = of_property_read_u32(of, "nxp,tx-output-mode", &prop);
+	if (!err)
+		priv->ocr |= prop & OCR_MODE_MASK;
+	else
+		priv->ocr |= OCR_MODE_NORMAL; /* default */
+
+	err = of_property_read_u32(of, "nxp,tx-output-config", &prop);
+	if (!err)
+		priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
+	else
+		priv->ocr |= OCR_TX0_PULLDOWN; /* default */
+
+	err = of_property_read_u32(of, "nxp,clock-out-frequency", &prop);
+	if (!err && prop) {
+		u32 divider = priv->can.clock.freq * 2 / prop;
+
+		if (divider > 1)
+			priv->cdr |= divider / 2 - 1;
+		else
+			priv->cdr |= CDR_CLKOUT_MASK;
+	} else {
+		priv->cdr |= CDR_CLK_OFF; /* default */
+	}
+
+	if (!of_property_read_bool(of, "nxp,no-comparator-bypass"))
+		priv->cdr |= CDR_CBP; /* default */
+}
+
+static int sp_probe(struct platform_device *pdev)
+{
+	int err, irq = 0;
 	void __iomem *addr;
 	struct net_device *dev;
 	struct sja1000_priv *priv;
-	struct resource *res_mem, *res_irq;
+	struct resource *res_mem, *res_irq = NULL;
 	struct sja1000_platform_data *pdata;
+	struct device_node *of = pdev->dev.of_node;
 
 	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
+	if (!pdata && !of) {
 		dev_err(&pdev->dev, "No platform data provided!\n");
 		return -ENODEV;
 	}
 
 	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res_mem || !res_irq)
+	if (!res_mem)
 		return -ENODEV;
 
 	if (!devm_request_mem_region(&pdev->dev, res_mem->start,
@@ -96,36 +168,35 @@ static int sp_probe(struct platform_device *pdev)
 	if (!addr)
 		return -ENOMEM;
 
+	if (of)
+		irq = irq_of_parse_and_map(of, 0);
+	else
+		res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+	if (!irq && !res_irq)
+		return -ENODEV;
+
 	dev = alloc_sja1000dev(0);
 	if (!dev)
 		return -ENOMEM;
 	priv = netdev_priv(dev);
 
-	dev->irq = res_irq->start;
-	priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
-	if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
-		priv->irq_flags |= IRQF_SHARED;
+	if (res_irq) {
+		irq = res_irq->start;
+		priv->irq_flags = res_irq->flags & IRQF_TRIGGER_MASK;
+		if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
+			priv->irq_flags |= IRQF_SHARED;
+	} else {
+		priv->irq_flags = IRQF_SHARED;
+	}
+
+	dev->irq = irq;
 	priv->reg_base = addr;
-	/* The CAN clock frequency is half the oscillator clock frequency */
-	priv->can.clock.freq = pdata->osc_freq / 2;
-	priv->ocr = pdata->ocr;
-	priv->cdr = pdata->cdr;
 
-	switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
-	case IORESOURCE_MEM_32BIT:
-		priv->read_reg = sp_read_reg32;
-		priv->write_reg = sp_write_reg32;
-		break;
-	case IORESOURCE_MEM_16BIT:
-		priv->read_reg = sp_read_reg16;
-		priv->write_reg = sp_write_reg16;
-		break;
-	case IORESOURCE_MEM_8BIT:
-	default:
-		priv->read_reg = sp_read_reg8;
-		priv->write_reg = sp_write_reg8;
-		break;
-	}
+	if (of)
+		sp_populate_of(priv, of);
+	else
+		sp_populate(priv, pdata, res_mem->flags);
 
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
@@ -156,12 +227,19 @@ static int sp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id sp_of_table[] = {
+	{.compatible = "nxp,sja1000"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sp_of_table);
+
 static struct platform_driver sp_driver = {
 	.probe = sp_probe,
 	.remove = sp_remove,
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
+		.of_match_table = sp_of_table,
 	},
 };
 
-- 
1.8.1.2

[PATCH v3 4/5] Documentation: devicetree: sja1000: add reg-io-width binding

[Florian Vaussard]

Add the reg-io-width property to describe the width of the memory
accesses.

Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 Documentation/devicetree/bindings/net/can/sja1000.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt
index f2105a4..b4a6d53 100644
--- a/Documentation/devicetree/bindings/net/can/sja1000.txt
+++ b/Documentation/devicetree/bindings/net/can/sja1000.txt
@@ -12,6 +12,10 @@ Required properties:
 
 Optional properties:
 
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+	should be performed on the device.  Valid value is 1, 2 or 4.
+	Default to 1 (8 bits).
+
 - nxp,external-clock-frequency : Frequency of the external oscillator
 	clock in Hz. Note that the internal clock frequency used by the
 	SJA1000 is half of that value. If not specified, a default value
-- 
1.8.1.2

[PATCH v3 5/5] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access

[Florian Vaussard]

Add the 'reg-io-width' property for 8, 16 and 32-bit access, like
what is currently done with IORESOURCE_MEM_{8,16,32}BIT for non-OF
boot.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/net/can/sja1000/sja1000_platform.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 50dece8..25122bf 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -102,8 +102,20 @@ static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
 	int err;
 	u32 prop;
 
-	priv->read_reg = sp_read_reg8;
-	priv->write_reg = sp_write_reg8;
+	err = of_property_read_u32(of, "reg-io-width", &prop);
+	if (err)
+		prop = 1;
+
+	if (prop == 4) {
+		priv->read_reg = sp_read_reg32;
+		priv->write_reg = sp_write_reg32;
+	} else if (prop == 2) {
+		priv->read_reg = sp_read_reg16;
+		priv->write_reg = sp_write_reg16;
+	} else {
+		priv->read_reg = sp_read_reg8;
+		priv->write_reg = sp_write_reg8;
+	}
 
 	err = of_property_read_u32(of, "nxp,external-clock-frequency", &prop);
 	if (!err)
-- 
1.8.1.2

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

On 01/31/2014 02:34 PM, Florian Vaussard wrote:
> Hello,
> 
> (could someone with a SJA1000 on SPARC perform a functional test
> to see if interrupts are working? it would be great :-)
> 
> Changes since v2:
> - Dropped patch 1 "can: sja1000: remove unused defines"
> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
> 
> Changes since v1:
> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
> 
> The first part of this series performs serveral small cleanups
> (patches 1 and 2).
> 
> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
> Changes are pretty conservatives (mostly copy/paste/move). IRQ
> is treated differently in the OF and non-OF versions, thus this
> is where the fused version differs the most.
> 
> The final part introduces the 'reg-io-width' binding (already used
> by some other drivers) to perform a similar job as what was done
> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
> take into account the aliasing of the address bus.
> 
> All patches were tested using OF boot on my OMAP3 system with a
> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
> I do not have a platform data at hand.

Nice, looks good now. I'll give Andreas some time to test on sparc and
then apply the patches.

regards,
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Andreas Larsson]

On 2014-01-31 14:40, Marc Kleine-Budde wrote:
> On 01/31/2014 02:34 PM, Florian Vaussard wrote:
>> Hello,
>>
>> (could someone with a SJA1000 on SPARC perform a functional test
>> to see if interrupts are working? it would be great :-)
>>
>> Changes since v2:
>> - Dropped patch 1 "can: sja1000: remove unused defines"
>> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
>>
>> Changes since v1:
>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>
>> The first part of this series performs serveral small cleanups
>> (patches 1 and 2).
>>
>> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>> is treated differently in the OF and non-OF versions, thus this
>> is where the fused version differs the most.
>>
>> The final part introduces the 'reg-io-width' binding (already used
>> by some other drivers) to perform a similar job as what was done
>> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
>> take into account the aliasing of the address bus.
>>
>> All patches were tested using OF boot on my OMAP3 system with a
>> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
>> I do not have a platform data at hand.
>
> Nice, looks good now. I'll give Andreas some time to test on sparc and
> then apply the patches.


I am on it. I will dig up some hardware for this on Monday to test it.

Best regards,
Andreas

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On 01/31/2014 02:50 PM, Andreas Larsson wrote:
> On 2014-01-31 14:40, Marc Kleine-Budde wrote:
>> On 01/31/2014 02:34 PM, Florian Vaussard wrote:
>>> Hello,
>>>
>>> (could someone with a SJA1000 on SPARC perform a functional test
>>> to see if interrupts are working? it would be great :-)
>>>
>>> Changes since v2:
>>> - Dropped patch 1 "can: sja1000: remove unused defines"
>>> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
>>>
>>> Changes since v1:
>>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>>
>>> The first part of this series performs serveral small cleanups
>>> (patches 1 and 2).
>>>
>>> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
>>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>>> is treated differently in the OF and non-OF versions, thus this
>>> is where the fused version differs the most.
>>>
>>> The final part introduces the 'reg-io-width' binding (already used
>>> by some other drivers) to perform a similar job as what was done
>>> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
>>> take into account the aliasing of the address bus.
>>>
>>> All patches were tested using OF boot on my OMAP3 system with a
>>> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
>>> I do not have a platform data at hand.
>>
>> Nice, looks good now. I'll give Andreas some time to test on sparc and
>> then apply the patches.
> 
> 
> I am on it. I will dig up some hardware for this on Monday to test it.

\o/

-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH v3 5/5] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access

[Sergei Shtylyov]

Hello.

On 31-01-2014 17:34, Florian Vaussard wrote:

> Add the 'reg-io-width' property for 8, 16 and 32-bit access, like
> what is currently done with IORESOURCE_MEM_{8,16,32}BIT for non-OF
> boot.

> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>   drivers/net/can/sja1000/sja1000_platform.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)

> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
> index 50dece8..25122bf 100644
> --- a/drivers/net/can/sja1000/sja1000_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_platform.c
> @@ -102,8 +102,20 @@ static void sp_populate_of(struct sja1000_priv *priv, struct device_node *of)
>   	int err;
>   	u32 prop;
>
> -	priv->read_reg = sp_read_reg8;
> -	priv->write_reg = sp_write_reg8;
> +	err = of_property_read_u32(of, "reg-io-width", &prop);
> +	if (err)
> +		prop = 1;
> +
> +	if (prop == 4) {

    This is asking to be a *switch* statement instead.

> +		priv->read_reg = sp_read_reg32;
> +		priv->write_reg = sp_write_reg32;
> +	} else if (prop == 2) {
> +		priv->read_reg = sp_read_reg16;
> +		priv->write_reg = sp_write_reg16;
> +	} else {
> +		priv->read_reg = sp_read_reg8;
> +		priv->write_reg = sp_write_reg8;
> +	}

WBR, Sergei

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

On 01/31/2014 02:50 PM, Andreas Larsson wrote:
> On 2014-01-31 14:40, Marc Kleine-Budde wrote:
>> On 01/31/2014 02:34 PM, Florian Vaussard wrote:
>>> Hello,
>>>
>>> (could someone with a SJA1000 on SPARC perform a functional test
>>> to see if interrupts are working? it would be great :-)
>>>
>>> Changes since v2:
>>> - Dropped patch 1 "can: sja1000: remove unused defines"
>>> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
>>>
>>> Changes since v1:
>>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>>
>>> The first part of this series performs serveral small cleanups
>>> (patches 1 and 2).
>>>
>>> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
>>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>>> is treated differently in the OF and non-OF versions, thus this
>>> is where the fused version differs the most.
>>>
>>> The final part introduces the 'reg-io-width' binding (already used
>>> by some other drivers) to perform a similar job as what was done
>>> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
>>> take into account the aliasing of the address bus.
>>>
>>> All patches were tested using OF boot on my OMAP3 system with a
>>> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
>>> I do not have a platform data at hand.
>>
>> Nice, looks good now. I'll give Andreas some time to test on sparc and
>> then apply the patches.
> 
> 
> I am on it. I will dig up some hardware for this on Monday to test it.
> 

Many thanks :o)

(be careful to enable CAN_SJA1000_PLATFORM, as I removed
CAN_SJA1000_PLATFORM_OF)

Regards,
Florian

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Andreas Larsson]

On 2014-01-31 14:50, Andreas Larsson wrote:
> On 2014-01-31 14:40, Marc Kleine-Budde wrote:
>> On 01/31/2014 02:34 PM, Florian Vaussard wrote:
>>> Hello,
>>>
>>> (could someone with a SJA1000 on SPARC perform a functional test
>>> to see if interrupts are working? it would be great :-)
>>>
>>> Changes since v2:
>>> - Dropped patch 1 "can: sja1000: remove unused defines"
>>> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
>>>
>>> Changes since v1:
>>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>>
>>> The first part of this series performs serveral small cleanups
>>> (patches 1 and 2).
>>>
>>> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
>>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>>> is treated differently in the OF and non-OF versions, thus this
>>> is where the fused version differs the most.
>>>
>>> The final part introduces the 'reg-io-width' binding (already used
>>> by some other drivers) to perform a similar job as what was done
>>> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
>>> take into account the aliasing of the address bus.
>>>
>>> All patches were tested using OF boot on my OMAP3 system with a
>>> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
>>> I do not have a platform data at hand.
>>
>> Nice, looks good now. I'll give Andreas some time to test on sparc and
>> then apply the patches.
>
>
> I am on it. I will dig up some hardware for this on Monday to test it.

Works splendidly. You can add:

Tested-by: Andreas Larsson <andreas@gaisler.com>

Cheers,
Andreas Larsson

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On 02/04/2014 09:19 AM, Andreas Larsson wrote:

> Works splendidly. You can add:
> 
> Tested-by: Andreas Larsson <andreas@gaisler.com>

Thanks,

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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH v3 0/5] can: sja1000: cleanups and new OF property

[Florian Vaussard]

On 02/04/2014 09:19 AM, Andreas Larsson wrote:
> On 2014-01-31 14:50, Andreas Larsson wrote:
>> On 2014-01-31 14:40, Marc Kleine-Budde wrote:
>>> On 01/31/2014 02:34 PM, Florian Vaussard wrote:
>>>> Hello,
>>>>
>>>> (could someone with a SJA1000 on SPARC perform a functional test
>>>> to see if interrupts are working? it would be great :-)
>>>>
>>>> Changes since v2:
>>>> - Dropped patch 1 "can: sja1000: remove unused defines"
>>>> - Addressed Marc's comments on patch 4 and 6 (now 3 and 5)
>>>>
>>>> Changes since v1:
>>>> - Merge sja1000_of_platform.c into sja1000_platform.c (patch 4)
>>>>
>>>> The first part of this series performs serveral small cleanups
>>>> (patches 1 and 2).
>>>>
>>>> Patch 3 merges sja1000_of_platform.c into sja1000_platform.c.
>>>> Changes are pretty conservatives (mostly copy/paste/move). IRQ
>>>> is treated differently in the OF and non-OF versions, thus this
>>>> is where the fused version differs the most.
>>>>
>>>> The final part introduces the 'reg-io-width' binding (already used
>>>> by some other drivers) to perform a similar job as what was done
>>>> with IORESOURCE_MEM_XXBIT. This is needed on my system to correctly
>>>> take into account the aliasing of the address bus.
>>>>
>>>> All patches were tested using OF boot on my OMAP3 system with a
>>>> memory-mapped SJA1000. Thus, the non-OF path is not tested, as
>>>> I do not have a platform data at hand.
>>>
>>> Nice, looks good now. I'll give Andreas some time to test on sparc and
>>> then apply the patches.
>>
>>
>> I am on it. I will dig up some hardware for this on Monday to test it.
> 
> Works splendidly. You can add:
> 
> Tested-by: Andreas Larsson <andreas@gaisler.com>
> 

Waohhh, great :D Thank you!

Regards,
Florian

Re: [PATCH v3 5/5] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On 01/31/2014 03:08 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 31-01-2014 17:34, Florian Vaussard wrote:
> 
>> Add the 'reg-io-width' property for 8, 16 and 32-bit access, like
>> what is currently done with IORESOURCE_MEM_{8,16,32}BIT for non-OF
>> boot.
> 
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>   drivers/net/can/sja1000/sja1000_platform.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
>> diff --git a/drivers/net/can/sja1000/sja1000_platform.c
>> b/drivers/net/can/sja1000/sja1000_platform.c
>> index 50dece8..25122bf 100644
>> --- a/drivers/net/can/sja1000/sja1000_platform.c
>> +++ b/drivers/net/can/sja1000/sja1000_platform.c
>> @@ -102,8 +102,20 @@ static void sp_populate_of(struct sja1000_priv
>> *priv, struct device_node *of)
>>       int err;
>>       u32 prop;
>>
>> -    priv->read_reg = sp_read_reg8;
>> -    priv->write_reg = sp_write_reg8;
>> +    err = of_property_read_u32(of, "reg-io-width", &prop);
>> +    if (err)
>> +        prop = 1;
>> +
>> +    if (prop == 4) {
> 
>    This is asking to be a *switch* statement instead.

Good point, I'll send a v4.

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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]

Re: [PATCH v3 5/5] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access

[Florian Vaussard]

Hi,

On 02/06/2014 11:57 AM, Marc Kleine-Budde wrote:
> On 01/31/2014 03:08 PM, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 31-01-2014 17:34, Florian Vaussard wrote:
>>
>>> Add the 'reg-io-width' property for 8, 16 and 32-bit access, like
>>> what is currently done with IORESOURCE_MEM_{8,16,32}BIT for non-OF
>>> boot.
>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>>> ---
>>>   drivers/net/can/sja1000/sja1000_platform.c | 16 ++++++++++++++--
>>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>>> diff --git a/drivers/net/can/sja1000/sja1000_platform.c
>>> b/drivers/net/can/sja1000/sja1000_platform.c
>>> index 50dece8..25122bf 100644
>>> --- a/drivers/net/can/sja1000/sja1000_platform.c
>>> +++ b/drivers/net/can/sja1000/sja1000_platform.c
>>> @@ -102,8 +102,20 @@ static void sp_populate_of(struct sja1000_priv
>>> *priv, struct device_node *of)
>>>       int err;
>>>       u32 prop;
>>>
>>> -    priv->read_reg = sp_read_reg8;
>>> -    priv->write_reg = sp_write_reg8;
>>> +    err = of_property_read_u32(of, "reg-io-width", &prop);
>>> +    if (err)
>>> +        prop = 1;
>>> +
>>> +    if (prop == 4) {
>>
>>    This is asking to be a *switch* statement instead.
> 
> Good point, I'll send a v4.
> 

Thank you!

Florian