[PATCH v2 0/6] can: sja1000: cleanups and new OF property

STALE4564d

Revision v2 of 3 in this series.

21 messages, 2 authors, 2014-02-03 · open the first message on its own page

[PATCH v2 0/6] can: sja1000: cleanups and new OF property

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:35:28

Hello,

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

The first part of this series performs serveral small cleanups
(patches 1 to 3).

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

Florian Vaussard (6):
  can: sja1000: remove unused defines
  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         | 194 ++++++++++++------
 6 files changed, 141 insertions(+), 295 deletions(-)
 delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c

-- 
1.8.1.2

[PATCH v2 1/6] can: sja1000: remove unused defines

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:35:30

Remove unused defines for the OF platform.

Signed-off-by: Florian Vaussard <redacted>
---
 drivers/net/can/sja1000/sja1000_of_platform.c | 3 ---
 1 file changed, 3 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index 047accd..2f29eb9 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -55,9 +55,6 @@ 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);
-- 
1.8.1.2

[PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:35:35

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

Cc: Grant Likely <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <redacted>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <redacted>
Cc: Kumar Gala <redacted>
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Vaussard <redacted>
---
 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 v2 6/6] can: sja1000: of: add reg-io-width property for 8, 16 and 32-bit register access

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:35:36

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 <redacted>
---
 drivers/net/can/sja1000/sja1000_platform.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 96a92a1..62ebc3d 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -103,8 +103,17 @@ 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;
+	of_property_read_u32(of, "reg-io-width", &prop);
+	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

[PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:35:37

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

Signed-off-by: Florian Vaussard <redacted>
---
 drivers/net/can/sja1000/Kconfig               |  13 +-
 drivers/net/can/sja1000/Makefile              |   1 -
 drivers/net/can/sja1000/sja1000_of_platform.c | 218 --------------------------
 drivers/net/can/sja1000/sja1000_platform.c    | 141 +++++++++++++----
 4 files changed, 116 insertions(+), 257 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 2f29eb9..0000000
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ /dev/null
@@ -1,218 +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)
-
-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..96a92a1 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 SJA1000_OFP_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,98 @@ 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;
+	}
+}
+
+#if defined(CONFIG_OF)
+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 = SJA1000_OFP_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 */
+}
+#else
+static void sp_populate_of(struct sja1000_priv *priv, device_node *of)
+{
+}
+#endif
+
+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 = 0;
 	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 +174,34 @@ 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 +232,21 @@ static int sp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static struct of_device_id sja1000_ofp_table[] = {
+	{.compatible = "nxp,sja1000"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
+#endif
+
 static struct platform_driver sp_driver = {
 	.probe = sp_probe,
 	.remove = sp_remove,
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(sja1000_ofp_table),
 	},
 };
 
-- 
1.8.1.2

[PATCH v2 3/6] can: sja1000: platform: use devm_* APIs

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:36:44

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

Signed-off-by: Florian Vaussard <redacted>
---
 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 v2 2/6] can: sja1000: convert printk to use netdev API

From: Florian Vaussard <hidden>
Date: 2014-01-31 10:37:21

Use netdev_* where applicable.

Signed-off-by: Florian Vaussard <redacted>
---
 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

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

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:28:50

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted hunk
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 <redacted>
---
 drivers/net/can/sja1000/sja1000_platform.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 96a92a1..62ebc3d 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -103,8 +103,17 @@ 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;
+	of_property_read_u32(of, "reg-io-width", &prop);
The new property might not be available in the DT, please check the
return value of of_property_read_u32(). Use 8 bit wide as default.
+	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)
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   |

Re: [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:29:44

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
Add the reg-io-width property to describe the width of the memory
accesses.

Cc: Grant Likely <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <redacted>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <redacted>
Cc: Kumar Gala <redacted>
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Vaussard <redacted>
I think it makes sense to squash into patch 6.

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   |

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:33:45

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Signed-off-by: Florian Vaussard <redacted>
---
 drivers/net/can/sja1000/Kconfig               |  13 +-
 drivers/net/can/sja1000/Makefile              |   1 -
 drivers/net/can/sja1000/sja1000_of_platform.c | 218 --------------------------
 drivers/net/can/sja1000/sja1000_platform.c    | 141 +++++++++++++----
 4 files changed, 116 insertions(+), 257 deletions(-)
 delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
[...]
quoted hunk
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 779679a..96a92a1 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 SJA1000_OFP_CAN_CLOCK  (16000000 / 2)
This driver uses "SP" short for sja100_platform as namespace, please
convert.
quoted hunk
 
 MODULE_AUTHOR("Sascha Hauer [off-list ref]");
+MODULE_AUTHOR("Wolfgang Grandegger [off-list ref]");
 MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
 MODULE_ALIAS("platform:" DRV_NAME);
 MODULE_LICENSE("GPL v2");
@@ -67,24 +71,98 @@ 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;
+	}
+}
+
+#if defined(CONFIG_OF)
The driver also compiles on systems without CONFIG_OF without these
defines. Please remove.
+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 = SJA1000_OFP_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 */
+}
+#else
+static void sp_populate_of(struct sja1000_priv *priv, device_node *of)
+{
+}
+#endif
+
+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 = 0;
Please use NULL to initialize pointers.
quoted hunk
 	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 +174,34 @@ 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;
Please add { } for else, too.
quoted hunk
+
+	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 +232,21 @@ static int sp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
Please remove the ifdef.
+static struct of_device_id sja1000_ofp_table[] = {
Please rename into sp_of_table or something similar.
+	{.compatible = "nxp,sja1000"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
+#endif
+
 static struct platform_driver sp_driver = {
 	.probe = sp_probe,
 	.remove = sp_remove,
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(sja1000_ofp_table),
The of_match_ptr is not needed anymore, please remove.
 	},
 };
 
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   |

Re: [PATCH v2 3/6] can: sja1000: platform: use devm_* APIs

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:34:07

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
Simplify probe and remove functions by converting most of the resources
to use devm_* APIs.

Signed-off-by: Florian Vaussard <redacted>
Looks good.

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   |

Re: [PATCH v2 2/6] can: sja1000: convert printk to use netdev API

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:34:34

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
Use netdev_* where applicable.

Signed-off-by: Florian Vaussard <redacted>
Loks good.

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

Re: [PATCH v2 1/6] can: sja1000: remove unused defines

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:35:38

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
Remove unused defines for the OF platform.

Signed-off-by: Florian Vaussard <redacted>
I've overlooked in my first review that this patch is touching the
of_platform driver. Please remove the patch, as we want to remove the
driver sooner or later.

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   |

Re: [PATCH v2 0/6] can: sja1000: cleanups and new OF property

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:37:33

On 01/31/2014 11:35 AM, Florian Vaussard wrote:
Hello,

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

The first part of this series performs serveral small cleanups
(patches 1 to 3).

Patch 4 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.
Good work, please keep compatibility for DTs without the 'reg-io-width'
binding. The rest of my comments are only nitpicks. :)

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   |

Re: [PATCH v2 0/6] can: sja1000: cleanups and new OF property

From: Florian Vaussard <hidden>
Date: 2014-01-31 12:41:42

Hello Marc,

On 01/31/2014 01:37 PM, Marc Kleine-Budde wrote:
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
Hello,

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

The first part of this series performs serveral small cleanups
(patches 1 to 3).

Patch 4 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.
Good work, please keep compatibility for DTs without the 'reg-io-width'
binding. The rest of my comments are only nitpicks. :)
Good points. I will send a v3.

Regards,

Florian

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Florian Vaussard <hidden>
Date: 2014-01-31 12:45:37

Hello Marc,

On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.

Regards,
Florian
quoted
Signed-off-by: Florian Vaussard <redacted>
---
 drivers/net/can/sja1000/Kconfig               |  13 +-
 drivers/net/can/sja1000/Makefile              |   1 -
 drivers/net/can/sja1000/sja1000_of_platform.c | 218 --------------------------
 drivers/net/can/sja1000/sja1000_platform.c    | 141 +++++++++++++----
 4 files changed, 116 insertions(+), 257 deletions(-)
 delete mode 100644 drivers/net/can/sja1000/sja1000_of_platform.c
[...]
quoted
diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 779679a..96a92a1 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 SJA1000_OFP_CAN_CLOCK  (16000000 / 2)
This driver uses "SP" short for sja100_platform as namespace, please
convert.
quoted
 
 MODULE_AUTHOR("Sascha Hauer [off-list ref]");
+MODULE_AUTHOR("Wolfgang Grandegger [off-list ref]");
 MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
 MODULE_ALIAS("platform:" DRV_NAME);
 MODULE_LICENSE("GPL v2");
@@ -67,24 +71,98 @@ 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;
+	}
+}
+
+#if defined(CONFIG_OF)
The driver also compiles on systems without CONFIG_OF without these
defines. Please remove.
quoted
+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 = SJA1000_OFP_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 */
+}
+#else
+static void sp_populate_of(struct sja1000_priv *priv, device_node *of)
+{
+}
+#endif
+
+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 = 0;
Please use NULL to initialize pointers.
quoted
 	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 +174,34 @@ 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;
Please add { } for else, too.
quoted
+
+	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 +232,21 @@ static int sp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
Please remove the ifdef.
quoted
+static struct of_device_id sja1000_ofp_table[] = {
Please rename into sp_of_table or something similar.
quoted
+	{.compatible = "nxp,sja1000"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, sja1000_ofp_table);
+#endif
+
 static struct platform_driver sp_driver = {
 	.probe = sp_probe,
 	.remove = sp_remove,
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(sja1000_ofp_table),
The of_match_ptr is not needed anymore, please remove.
quoted
 	},
 };
 
Marc

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:49:49

On 01/31/2014 01:45 PM, Florian Vaussard wrote:
Hello Marc,

On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.
Okay.
IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
get a Tested-by from Andreas (Cc'ed).

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   |

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 12:51:25

On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
On 01/31/2014 01:45 PM, Florian Vaussard wrote:
quoted
Hello Marc,

On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.
Okay.
IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
get a Tested-by from Andreas (Cc'ed).
You have to use irq_of_parse_and_map() on sparc, but your patches does this.

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   |

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Florian Vaussard <hidden>
Date: 2014-01-31 13:00:11


On 01/31/2014 01:51 PM, Marc Kleine-Budde wrote:
On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 01:45 PM, Florian Vaussard wrote:
quoted
Hello Marc,

On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.
Okay.
IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
get a Tested-by from Andreas (Cc'ed).
So it is ok for you if I remove of_platform, right?
You have to use irq_of_parse_and_map() on sparc, but your patches does this.
My first implementation was not using irq_of_parse_and_map(), as I do
not need it on ARM, but I suspected that other people might need it.
This is why the final code is slightly more complicated to handle both
cases.

Regards,
Florian

Re: [PATCH v2 4/6] can: sja1000: fuse of_platform into platform

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: 2014-01-31 13:14:22

On 01/31/2014 02:00 PM, Florian Vaussard wrote:

On 01/31/2014 01:51 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 01:49 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 01:45 PM, Florian Vaussard wrote:
quoted
Hello Marc,

On 01/31/2014 01:33 PM, Marc Kleine-Budde wrote:
quoted
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
The OpenFirmware probe can be merged into the standard platform
probe to leverage common code.
Good work, as we want to replace the existing driver, I'm quite picky on
this patch, see more comments inline.

Please don't delete of of_platform driver, yet.
Do you have any reason for not deleting of_platform? After this patch,
we will have duplicated functionalities, this may be misleading for
other people.
Okay.
IIRC there _was_ a problem on sparc, it _should_ be fixed now, better
get a Tested-by from Andreas (Cc'ed).
So it is ok for you if I remove of_platform, right?
If you get a Tested-by for this patch by a sparc user.

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   |

Re: [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding

From: Florian Vaussard <hidden>
Date: 2014-02-03 08:11:45


On 01/31/2014 01:29 PM, Marc Kleine-Budde wrote:
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
quoted
Add the reg-io-width property to describe the width of the memory
accesses.

Cc: Grant Likely <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <redacted>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <redacted>
Cc: Kumar Gala <redacted>
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Vaussard <redacted>
I think it makes sense to squash into patch 6.
According to recent guidelines from the DT maintainers [1], the
documentation part should be a separate patch. I guess it makes sense to
ease the review process for new bindings.

Regards
Florian

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/submitting-patches.txt
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help