[RFC: PATCH 1/5] can: sja1000_platform

[RFC: PATCH 1/5] can: sja1000_platform

[Marc Kleine-Budde]

Hello,

this series ports the device tree bindings from the existing
sja1000_of_platform driver to the sja1000_platform driver. It has been compile
time tested only. I've written the patches to get rid of the
sja1000_of_platform once this in the kernel.

Please test, especially on sparc, powerpc and arm.

You can find this series in the sj1000 branch of the linux-can-next repo on
gitorious[1].

regards, Marc

[1] https://gitorious.org/linux-can/linux-can-next

[RFC: PATCH 1/5] can: sja1000_platform: add __dev{init,exit} annotations

[Marc Kleine-Budde]

This patch add "__devinit", "__devexit" and "__devexit_p" annotations to the
sja1000_platform driver.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/sja1000_platform.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 662c5f7..80f0d67 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -66,7 +66,7 @@ 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 int __devinit sp_probe(struct platform_device *pdev)
 {
 	int err;
 	void __iomem *addr;
@@ -158,7 +158,7 @@ static int sp_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int sp_remove(struct platform_device *pdev)
+static int __devexit sp_remove(struct platform_device *pdev)
 {
 	struct net_device *dev = dev_get_drvdata(&pdev->dev);
 	struct sja1000_priv *priv = netdev_priv(dev);
@@ -180,7 +180,7 @@ static int sp_remove(struct platform_device *pdev)
 
 static struct platform_driver sp_driver = {
 	.probe = sp_probe,
-	.remove = sp_remove,
+	.remove = __devexit_p(sp_remove),
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
-- 
1.7.10

[RFC: PATCH 2/5] can: sja1000_platform: convert ioremap_nocache to devm_request_and_ioremap

[Marc Kleine-Budde]

This patch converts the "ioremap_nocache()" to "devm_request_and_ioremap()"
which does automatically cleanup.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/sja1000_platform.c |   25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 80f0d67..36300de 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -89,22 +89,16 @@ static int __devinit sp_probe(struct platform_device *pdev)
 		goto exit;
 	}
 
-	if (!request_mem_region(res_mem->start, resource_size(res_mem),
-				DRV_NAME)) {
-		err = -EBUSY;
-		goto exit;
-	}
-
-	addr = ioremap_nocache(res_mem->start, resource_size(res_mem));
+	addr = devm_request_and_ioremap(&pdev->dev, res_mem);
 	if (!addr) {
 		err = -ENOMEM;
-		goto exit_release;
+		goto exit;
 	}
 
 	dev = alloc_sja1000dev(0);
 	if (!dev) {
 		err = -ENOMEM;
-		goto exit_iounmap;
+		goto exit;
 	}
 	priv = netdev_priv(dev);
 
@@ -150,10 +144,6 @@ static int __devinit 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;
 }
@@ -161,18 +151,9 @@ static int __devinit sp_probe(struct platform_device *pdev)
 static int __devexit 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 (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.7.10

[RFC: PATCH 3/5] can: sja1000_platform: replace DRV_NAME by KBUILD_MODNAME

[Marc Kleine-Budde]

This patch replaces DRV_NAME by KBUILD_MODNAME. DRV_NAME is removed from the
"dev_LEVEL()", as it already provides every relevant information.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/sja1000_platform.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 36300de..b0ce5ef 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -30,8 +30,6 @@
 
 #include "sja1000.h"
 
-#define DRV_NAME "sja1000_platform"
-
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
 MODULE_LICENSE("GPL v2");
@@ -133,13 +131,13 @@ static int __devinit sp_probe(struct platform_device *pdev)
 
 	err = register_sja1000dev(dev);
 	if (err) {
-		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
-			DRV_NAME, err);
+		dev_err(&pdev->dev, "registering netdev failed (err=%d)\n",
+			err);
 		goto exit_free;
 	}
 
-	dev_info(&pdev->dev, "%s device registered (reg_base=%p, irq=%d)\n",
-		 DRV_NAME, priv->reg_base, dev->irq);
+	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
+		 priv->reg_base, dev->irq);
 	return 0;
 
  exit_free:
@@ -163,7 +161,7 @@ static struct platform_driver sp_driver = {
 	.probe = sp_probe,
 	.remove = __devexit_p(sp_remove),
 	.driver = {
-		.name = DRV_NAME,
+		.name = KBUILD_MODNAME,
 		.owner = THIS_MODULE,
 	},
 };
-- 
1.7.10

[RFC: PATCH 4/5] can: sja1000_platform: factor out initialization from platform data

[Marc Kleine-Budde]

This patch moves the initialization from the "sp_probe()" function into a
seperate function. This prepares the driver for the device tree bindings coming
in the next patch.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/sja1000_platform.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index b0ce5ef..6260209 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -64,6 +64,14 @@ static void sp_write_reg32(const struct sja1000_priv *priv, int reg, u8 val)
 	iowrite8(val, priv->reg_base + reg * 4);
 }
 
+static void __devinit sp_probe_pdata(struct sja1000_priv *priv, const struct sja1000_platform_data *pdata)
+{
+	/* 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;
+}
+
 static int __devinit sp_probe(struct platform_device *pdev)
 {
 	int err;
@@ -71,7 +79,7 @@ static int __devinit sp_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct sja1000_priv *priv;
 	struct resource *res_mem, *res_irq;
-	struct sja1000_platform_data *pdata;
+	const struct sja1000_platform_data *pdata;
 
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
@@ -105,10 +113,8 @@ static int __devinit sp_probe(struct platform_device *pdev)
 	if (res_irq->flags & IORESOURCE_IRQ_SHAREABLE)
 		priv->irq_flags |= IRQF_SHARED;
 	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;
+
+	sp_probe_pdata(priv, pdata);
 
 	switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
 	case IORESOURCE_MEM_32BIT:
-- 
1.7.10

[RFC: PATCH 5/5] can: sja1000_platform: add device tree bindings

[Marc Kleine-Budde]

This patch add device tree bindings, the are ported from the
sja1000_of_platform driver:

    Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>

The use of "of_get_property()" has been converted to "of_property_read_u32()"
to be endianess safe.

Cc: Wolfgang Grandegger <wg@grandegger.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/sja1000_platform.c |   91 +++++++++++++++++++++++++---
 1 file changed, 84 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
index 6260209..91f7404 100644
--- a/drivers/net/can/sja1000/sja1000_platform.c
+++ b/drivers/net/can/sja1000/sja1000_platform.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2005 Sascha Hauer, Pengutronix
- * Copyright (C) 2007 Wolfgang Grandegger <wg@grandegger.com>
+ * Copyright (C) 2007-2009 Wolfgang Grandegger <wg@grandegger.com>
+ * Copyright (C) 2012 Marc Kleine-Budde <mkl@pengutronix.de>
  *
  * 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
@@ -11,15 +12,28 @@
  * 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 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/of.h>
 #include <linux/delay.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
@@ -30,6 +44,8 @@
 
 #include "sja1000.h"
 
+#define SP_DEFAULT_CLOCK	(16000000 / 2)
+
 MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
 MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
 MODULE_LICENSE("GPL v2");
@@ -72,6 +88,61 @@ static void __devinit sp_probe_pdata(struct sja1000_priv *priv, const struct sja
 	priv->cdr = pdata->cdr;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id __devinitconst sp_match_table[] = {
+	{ .compatible = "nxp,sja1000" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sp_match_table);
+
+static void __devinit sp_probe_of(struct sja1000_priv *priv, const struct device_node *of_node)
+{
+	int err;
+	u32 prop;
+	bool bypass;
+
+	err = of_property_read_u32(of_node,
+				   "nxp,external-clock-frequency", &prop);
+	if (err)
+		priv->can.clock.freq = SP_DEFAULT_CLOCK;
+	else
+		priv->can.clock.freq = prop / 2;
+
+	err = of_property_read_u32(of_node, "nxp,tx-output-mode", &prop);
+	if (err)
+		priv->ocr |= OCR_MODE_NORMAL;	/* default */
+	else
+		priv->ocr |= prop & OCR_MODE_MASK;
+
+	err = of_property_read_u32(of_node, "nxp,tx-output-config", &prop);
+	if (err)
+		priv->ocr |= OCR_TX0_PULLDOWN;	/* default */
+	else
+		priv->ocr |= (prop << OCR_TX_SHIFT) & OCR_TX_MASK;
+
+	err = of_property_read_u32(of_node, "nxp,clock-out-frequency", &prop);
+	if (err) {
+		priv->cdr |= CDR_CLK_OFF;	/* default */
+	} else {
+		u32 divider = priv->can.clock.freq * 2 / prop;
+
+		if (divider > 1)
+			priv->cdr |= divider / 2 - 1;
+		else
+			priv->cdr |= CDR_CLKOUT_MASK;
+	}
+
+	bypass = !of_property_read_bool(of_node, "nxp,no-comparator-bypass");
+	if (bypass)
+		priv->cdr |= CDR_CBP;		/* default */
+}
+#else
+static inline void __devinit sp_probe_of(struct sja1000_priv *priv, const struct device_node *of_node)
+{
+	return;
+}
+#endif
+
 static int __devinit sp_probe(struct platform_device *pdev)
 {
 	int err;
@@ -80,10 +151,12 @@ static int __devinit sp_probe(struct platform_device *pdev)
 	struct sja1000_priv *priv;
 	struct resource *res_mem, *res_irq;
 	const struct sja1000_platform_data *pdata;
+	const struct device_node *of_node;
 
 	pdata = pdev->dev.platform_data;
-	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data provided!\n");
+	of_node = pdev->dev.of_node;
+	if (!pdata && !of_node) {
+		dev_err(&pdev->dev, "No platform data provided or no OF device!\n");
 		err = -ENODEV;
 		goto exit;
 	}
@@ -114,7 +187,10 @@ static int __devinit sp_probe(struct platform_device *pdev)
 		priv->irq_flags |= IRQF_SHARED;
 	priv->reg_base = addr;
 
-	sp_probe_pdata(priv, pdata);
+	if (pdata)
+		sp_probe_pdata(priv, pdata);
+	else
+		sp_probe_of(priv, of_node);
 
 	switch (res_mem->flags & IORESOURCE_MEM_TYPE_MASK) {
 	case IORESOURCE_MEM_32BIT:
@@ -168,6 +244,7 @@ static struct platform_driver sp_driver = {
 	.remove = __devexit_p(sp_remove),
 	.driver = {
 		.name = KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(sp_match_table),
 		.owner = THIS_MODULE,
 	},
 };
-- 
1.7.10

Re: [RFC: PATCH 5/5] can: sja1000_platform: add device tree bindings

[Wolfgang Grandegger]

Hi Marc,

On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
> This patch add device tree bindings, the are ported from the

typo?

> sja1000_of_platform driver:
> 
>     Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
> 
> The use of "of_get_property()" has been converted to "of_property_read_u32()"
> to be endianess safe.
> 
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/sja1000/sja1000_platform.c |   91 +++++++++++++++++++++++++---
>  1 file changed, 84 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
> index 6260209..91f7404 100644
> --- a/drivers/net/can/sja1000/sja1000_platform.c
> +++ b/drivers/net/can/sja1000/sja1000_platform.c
> @@ -1,6 +1,7 @@
>  /*
>   * Copyright (C) 2005 Sascha Hauer, Pengutronix
> - * Copyright (C) 2007 Wolfgang Grandegger <wg@grandegger.com>
> + * Copyright (C) 2007-2009 Wolfgang Grandegger <wg@grandegger.com>
> + * Copyright (C) 2012 Marc Kleine-Budde <mkl@pengutronix.de>
>   *
>   * 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
> @@ -11,15 +12,28 @@
>   * 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 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/of.h>
>  #include <linux/delay.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> @@ -30,6 +44,8 @@
>  
>  #include "sja1000.h"
>  
> +#define SP_DEFAULT_CLOCK	(16000000 / 2)
> +
>  MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>  MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
>  MODULE_LICENSE("GPL v2");
> @@ -72,6 +88,61 @@ static void __devinit sp_probe_pdata(struct sja1000_priv *priv, const struct sja
>  	priv->cdr = pdata->cdr;
>  }
>  
> +#ifdef CONFIG_OF

Do we still need that dependency?

Thanks for cleaning up! Unfortunately, I do not have a board with a
platform sja1000. Well, I have a phyCORE-PXA270, but I'm not sure if I
get it working with the mainline kernel quickly.

Wolfgang.

Re: [RFC: PATCH 5/5] can: sja1000_platform: add device tree bindings

[Marc Kleine-Budde]

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

On 10/05/2012 05:47 PM, Wolfgang Grandegger wrote:
> Hi Marc,
> 
> On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
>> This patch add device tree bindings, the are ported from the
> 
> typo?

yes.

> 
>> sja1000_of_platform driver:
>>
>>     Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
>>
>> The use of "of_get_property()" has been converted to "of_property_read_u32()"
>> to be endianess safe.
>>
>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>  drivers/net/can/sja1000/sja1000_platform.c |   91 +++++++++++++++++++++++++---
>>  1 file changed, 84 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/can/sja1000/sja1000_platform.c b/drivers/net/can/sja1000/sja1000_platform.c
>> index 6260209..91f7404 100644
>> --- a/drivers/net/can/sja1000/sja1000_platform.c
>> +++ b/drivers/net/can/sja1000/sja1000_platform.c
>> @@ -1,6 +1,7 @@
>>  /*
>>   * Copyright (C) 2005 Sascha Hauer, Pengutronix
>> - * Copyright (C) 2007 Wolfgang Grandegger <wg@grandegger.com>
>> + * Copyright (C) 2007-2009 Wolfgang Grandegger <wg@grandegger.com>
>> + * Copyright (C) 2012 Marc Kleine-Budde <mkl@pengutronix.de>
>>   *
>>   * 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
>> @@ -11,15 +12,28 @@
>>   * 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 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/of.h>
>>  #include <linux/delay.h>
>>  #include <linux/pci.h>
>>  #include <linux/platform_device.h>
>> @@ -30,6 +44,8 @@
>>  
>>  #include "sja1000.h"
>>  
>> +#define SP_DEFAULT_CLOCK	(16000000 / 2)
>> +
>>  MODULE_AUTHOR("Sascha Hauer <s.hauer@pengutronix.de>");
>>  MODULE_DESCRIPTION("Socket-CAN driver for SJA1000 on the platform bus");
>>  MODULE_LICENSE("GPL v2");
>> @@ -72,6 +88,61 @@ static void __devinit sp_probe_pdata(struct sja1000_priv *priv, const struct sja
>>  	priv->cdr = pdata->cdr;
>>  }
>>  
>> +#ifdef CONFIG_OF
> 
> Do we still need that dependency?

We don't need it in order to compile without OF support, but I think it
will make the module a bit bigger. However I'll check this.

> Thanks for cleaning up! Unfortunately, I do not have a board with a
> platform sja1000. Well, I have a phyCORE-PXA270, but I'm not sure if I
> get it working with the mainline kernel quickly.

I'm more interested if the powerpc board is still working. The one you
wrote the of platform driver in the first place.

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: 259 bytes --]

Re: [RFC: PATCH 5/5] can: sja1000_platform: add device tree bindings

[Wolfgang Grandegger]

On 10/05/2012 06:42 PM, Marc Kleine-Budde wrote:
> On 10/05/2012 05:47 PM, Wolfgang Grandegger wrote:
>> Hi Marc,
...
>> Thanks for cleaning up! Unfortunately, I do not have a board with a
>> platform sja1000. Well, I have a phyCORE-PXA270, but I'm not sure if I
>> get it working with the mainline kernel quickly.
> 
> I'm more interested if the powerpc board is still working. The one you
> wrote the of platform driver in the first place.

Mainline lists finds for "nxp,sja1000":

  arch/powerpc/boot/dts/digsy_mtc.dts
  arch/powerpc/boot/dts/mpc8308_p1m.dts

I'm sorry, I don't have those customer boards any more.

Wolfgang.

Re: [RFC: PATCH 1/5] can: sja1000_platform

[Andreas Larsson]

On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
> Hello,
>
> this series ports the device tree bindings from the existing
> sja1000_of_platform driver to the sja1000_platform driver. It has been compile
> time tested only. I've written the patches to get rid of the
> sja1000_of_platform once this in the kernel.
>
> Please test, especially on sparc, powerpc and arm.

On sparc, sp_probe fails for me because there is no resource of type 
IORESOURCE_IRQ.

As far as I can find out, on sparc (32 and 64 bit) IORESOURCE_IRQ 
resources are never created for the platform_device representation. The 
resources that are created are the ones created for adresses/registers 
in build_device_resources() in arch/sparc/kernel/of_device_32.c and 
arch/sparc/kernel/of_device_64.c respectively.

Cheers,
Andreas

Re: [RFC: PATCH 1/5] can: sja1000_platform

[Marc Kleine-Budde]

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

On 10/08/2012 11:16 AM, Andreas Larsson wrote:
> On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
>> Hello,
>>
>> this series ports the device tree bindings from the existing
>> sja1000_of_platform driver to the sja1000_platform driver. It has been
>> compile
>> time tested only. I've written the patches to get rid of the
>> sja1000_of_platform once this in the kernel.
>>
>> Please test, especially on sparc, powerpc and arm.
> 
> On sparc, sp_probe fails for me because there is no resource of type
> IORESOURCE_IRQ.
> 
> As far as I can find out, on sparc (32 and 64 bit) IORESOURCE_IRQ
> resources are never created for the platform_device representation. The
> resources that are created are the ones created for adresses/registers
> in build_device_resources() in arch/sparc/kernel/of_device_32.c and
> arch/sparc/kernel/of_device_64.c respectively.

Okay. Do both sparc architectures implement

    irq_of_parse_and_map()

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: 259 bytes --]

Re: [RFC: PATCH 1/5] can: sja1000_platform

[Andreas Larsson]

On 10/08/2012 02:19 PM, Marc Kleine-Budde wrote:
> On 10/08/2012 11:16 AM, Andreas Larsson wrote:
>> On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
>>> Hello,
>>>
>>> this series ports the device tree bindings from the existing
>>> sja1000_of_platform driver to the sja1000_platform driver. It has been
>>> compile
>>> time tested only. I've written the patches to get rid of the
>>> sja1000_of_platform once this in the kernel.
>>>
>>> Please test, especially on sparc, powerpc and arm.
>>
>> On sparc, sp_probe fails for me because there is no resource of type
>> IORESOURCE_IRQ.
>>
>> As far as I can find out, on sparc (32 and 64 bit) IORESOURCE_IRQ
>> resources are never created for the platform_device representation. The
>> resources that are created are the ones created for adresses/registers
>> in build_device_resources() in arch/sparc/kernel/of_device_32.c and
>> arch/sparc/kernel/of_device_64.c respectively.
>
> Okay. Do both sparc architectures implement
>
>      irq_of_parse_and_map()

Yes

Cheers,
Andreas

Re: [RFC: PATCH 1/5] can: sja1000_platform

[Andreas Larsson]

On 10/08/2012 11:16 AM, Andreas Larsson wrote:
> On 10/05/2012 04:39 PM, Marc Kleine-Budde wrote:
>> Please test, especially on sparc, powerpc and arm.
>
> On sparc, sp_probe fails for me because there is no resource of type
> IORESOURCE_IRQ.
>
> As far as I can find out, on sparc (32 and 64 bit) IORESOURCE_IRQ
> resources are never created for the platform_device representation. The
> resources that are created are the ones created for adresses/registers
> in build_device_resources() in arch/sparc/kernel/of_device_32.c and
> arch/sparc/kernel/of_device_64.c respectively.

It is also worth mentioning that on sparc, the checks against the 
different IORESOURCE_MEM_XXBIT will never be true due to the same 
reason. This information is not in the platform_device representation on 
sparc.

In my case I have 8-bit registers, so the default 8-bit saves me. With 
the irq problem worked around, the driver works fine for me on sparc32.

Cheers,
Andreas