[PATCH 0/3] dt and pinctrl support for at91_can

[PATCH 0/3] dt and pinctrl support for at91_can

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

Hi,

This set of patches add device tree and pinctrl support to the at91_can driver.

Regards


Ludovic Desroches (3):
  can: at91_can: add dt support
  can: at91_can: add pinctrl support
  can: Kconfig: CAN_AT91 depends on ARCH_AT91

 .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
 drivers/net/can/Kconfig                            |  2 +-
 drivers/net/can/at91_can.c                         | 84 +++++++++++++++++-----
 3 files changed, 80 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt

-- 
1.7.11.3

[PATCH 1/3] can: at91_can: add dt support

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

Add device tree support.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
 drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
 2 files changed, 70 insertions(+), 19 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt

diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
new file mode 100644
index 0000000..69381db
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
@@ -0,0 +1,14 @@
+* AT91 CAN *
+
+Required properties:
+  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
+  - reg: Should contain RTC registers location and length
+  - interrupts: Should contain IRQ line for the CAN controller
+
+Example:
+
+	can0: can@f000c000 {
+		compatbile = "atmel,at91sam9x5-can";
+		reg = <0xf000c000 0x300>;
+		interrupts = <40 4 5>
+	};
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 44f3637..c7f70d4 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -27,6 +27,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/rtnetlink.h>
 #include <linux/skbuff.h>
@@ -155,19 +156,20 @@ struct at91_priv {
 	canid_t mb0_id;
 };
 
-static const struct at91_devtype_data at91_devtype_data[] = {
-	[AT91_DEVTYPE_SAM9263] = {
-		.rx_first = 1,
-		.rx_split = 8,
-		.rx_last = 11,
-		.tx_shift = 2,
-	},
-	[AT91_DEVTYPE_SAM9X5] = {
-		.rx_first = 0,
-		.rx_split = 4,
-		.rx_last = 5,
-		.tx_shift = 1,
-	},
+static struct at91_devtype_data at91_at91sam9263_data = {
+	.rx_first = 1,
+	.rx_split = 8,
+	.rx_last = 11,
+	.tx_shift = 2,
+	.type = AT91_DEVTYPE_SAM9263
+};
+
+static struct at91_devtype_data at91_at91sam9x5_data = {
+	.rx_first = 0,
+	.rx_split = 4,
+	.rx_last = 5,
+	.tx_shift = 1,
+	.type = AT91_DEVTYPE_SAM9X5
 };
 
 static const struct can_bittiming_const at91_bittiming_const = {
@@ -1249,10 +1251,41 @@ static struct attribute_group at91_sysfs_attr_group = {
 	.attrs = at91_sysfs_attrs,
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id at91_can_dt_ids[] = {
+	{
+		.compatible = "atmel,at91sam9x5-can",
+		.data = &at91_at91sam9x5_data,
+	}, {
+		.compatible = "atmel,at91sam9263-can",
+		.data = &at91_at91sam9263_data,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, at91_can_dt_ids);
+#else
+#define at91_can_dt_ids NULL
+#endif
+
+static struct at91_devtype_data* at91_can_get_driver_data(
+						struct platform_device *pdev)
+{
+	if (pdev->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(at91_can_dt_ids, pdev->dev.of_node);
+		if (!match) {
+			dev_err(&pdev->dev, "no matching node found in dtb\n");
+			return NULL;
+		}
+		return (struct at91_devtype_data *)match->data;
+	}
+	return (struct at91_devtype_data *)platform_get_device_id(pdev)->driver_data;
+}
+
 static int at91_can_probe(struct platform_device *pdev)
 {
 	const struct at91_devtype_data *devtype_data;
-	enum at91_devtype devtype;
 	struct net_device *dev;
 	struct at91_priv *priv;
 	struct resource *res;
@@ -1260,8 +1293,12 @@ static int at91_can_probe(struct platform_device *pdev)
 	void __iomem *addr;
 	int err, irq;
 
-	devtype = pdev->id_entry->driver_data;
-	devtype_data = &at91_devtype_data[devtype];
+	devtype_data = at91_can_get_driver_data(pdev);
+	if (!devtype_data) {
+		dev_err(&pdev->dev, "no driver data\n");
+		err = -ENODEV;
+		goto exit;
+	}
 
 	clk = clk_get(&pdev->dev, "can_clk");
 	if (IS_ERR(clk)) {
@@ -1310,7 +1347,6 @@ static int at91_can_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->reg_base = addr;
 	priv->devtype_data = *devtype_data;
-	priv->devtype_data.type = devtype;
 	priv->clk = clk;
 	priv->pdata = pdev->dev.platform_data;
 	priv->mb0_id = 0x7ff;
@@ -1373,10 +1409,10 @@ static int at91_can_remove(struct platform_device *pdev)
 static const struct platform_device_id at91_can_id_table[] = {
 	{
 		.name = "at91_can",
-		.driver_data = AT91_DEVTYPE_SAM9263,
+		.driver_data = (unsigned long)&at91_at91sam9x5_data,
 	}, {
 		.name = "at91sam9x5_can",
-		.driver_data = AT91_DEVTYPE_SAM9X5,
+		.driver_data = (unsigned long)&at91_at91sam9263_data,
 	}, {
 		/* sentinel */
 	}
@@ -1389,6 +1425,7 @@ static struct platform_driver at91_can_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.owner = THIS_MODULE,
+		.of_match_table = at91_can_dt_ids,
 	},
 	.id_table = at91_can_id_table,
 };
-- 
1.7.11.3

[PATCH 2/3] can: at91_can: add pinctrl support

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/net/can/at91_can.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index c7f70d4..56fb2aa 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -28,6 +28,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/rtnetlink.h>
 #include <linux/skbuff.h>
@@ -1292,6 +1293,7 @@ static int at91_can_probe(struct platform_device *pdev)
 	struct clk *clk;
 	void __iomem *addr;
 	int err, irq;
+	struct pinctrl *pinctrl;
 
 	devtype_data = at91_can_get_driver_data(pdev);
 	if (!devtype_data) {
@@ -1314,6 +1316,13 @@ static int at91_can_probe(struct platform_device *pdev)
 		goto exit_put;
 	}
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl)) {
+		dev_err(&pdev->dev, "Failed to request pinctrl\n");
+		err = PTR_ERR(pinctrl);
+		goto exit_put;
+	}
+
 	if (!request_mem_region(res->start,
 				resource_size(res),
 				pdev->name)) {
-- 
1.7.11.3

[PATCH 3/3] can: Kconfig: CAN_AT91 depends on ARCH_AT91

[ludovic.desroches]

From: Ludovic Desroches <ludovic.desroches@atmel.com>

SAMA5D3 devices also embed CAN feature. Moreover if we want to produce a single
kernel image (at least for Atmel devices) it is not useful to be too
restrictive.

Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
---
 drivers/net/can/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 9862b2e..27311c3 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -65,7 +65,7 @@ config CAN_LEDS
 
 config CAN_AT91
 	tristate "Atmel AT91 onchip CAN controller"
-	depends on ARCH_AT91SAM9263 || ARCH_AT91SAM9X5
+	depends on ARCH_AT91
 	---help---
 	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263
 	  and AT91SAM9X5 processors.
-- 
1.7.11.3

Re: [PATCH 2/3] can: at91_can: add pinctrl support

[Marc Kleine-Budde]

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

On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
>  drivers/net/can/at91_can.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index c7f70d4..56fb2aa 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -28,6 +28,7 @@
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
>  #include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtnetlink.h>
>  #include <linux/skbuff.h>
> @@ -1292,6 +1293,7 @@ static int at91_can_probe(struct platform_device *pdev)
>  	struct clk *clk;
>  	void __iomem *addr;
>  	int err, irq;
> +	struct pinctrl *pinctrl;
>  
>  	devtype_data = at91_can_get_driver_data(pdev);
>  	if (!devtype_data) {
> @@ -1314,6 +1316,13 @@ static int at91_can_probe(struct platform_device *pdev)
>  		goto exit_put;
>  	}
>  
> +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);

Is this still needed? I think the pinctrl framework automatically
enables the default pinctrl for a device if available.

Marc

> +	if (IS_ERR(pinctrl)) {
> +		dev_err(&pdev->dev, "Failed to request pinctrl\n");
> +		err = PTR_ERR(pinctrl);
> +		goto exit_put;
> +	}
> +
>  	if (!request_mem_region(res->start,
>  				resource_size(res),
>  				pdev->name)) {
> 


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

Re: [PATCH 1/3] can: at91_can: add dt support

[Marc Kleine-Budde]

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

On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> Add device tree support.
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
>  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
>  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
>  2 files changed, 70 insertions(+), 19 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> new file mode 100644
> index 0000000..69381db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> @@ -0,0 +1,14 @@
> +* AT91 CAN *
> +
> +Required properties:
> +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"

On imx we use the oldest SoC with that IP available. Which strategy are
you following on at91?

> +  - reg: Should contain RTC registers location and length

RTC?

> +  - interrupts: Should contain IRQ line for the CAN controller
> +
> +Example:
> +
> +	can0: can@f000c000 {
> +		compatbile = "atmel,at91sam9x5-can";
> +		reg = <0xf000c000 0x300>;
> +		interrupts = <40 4 5>
> +	};
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index 44f3637..c7f70d4 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -27,6 +27,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtnetlink.h>
>  #include <linux/skbuff.h>
> @@ -155,19 +156,20 @@ struct at91_priv {
>  	canid_t mb0_id;
>  };
>  
> -static const struct at91_devtype_data at91_devtype_data[] = {
> -	[AT91_DEVTYPE_SAM9263] = {
> -		.rx_first = 1,
> -		.rx_split = 8,
> -		.rx_last = 11,
> -		.tx_shift = 2,
> -	},
> -	[AT91_DEVTYPE_SAM9X5] = {
> -		.rx_first = 0,
> -		.rx_split = 4,
> -		.rx_last = 5,
> -		.tx_shift = 1,
> -	},
> +static struct at91_devtype_data at91_at91sam9263_data = {
> +	.rx_first = 1,
> +	.rx_split = 8,
> +	.rx_last = 11,
> +	.tx_shift = 2,
> +	.type = AT91_DEVTYPE_SAM9263

nitpick:
can you add a trailing ","

> +};
> +
> +static struct at91_devtype_data at91_at91sam9x5_data = {
> +	.rx_first = 0,
> +	.rx_split = 4,
> +	.rx_last = 5,
> +	.tx_shift = 1,
> +	.type = AT91_DEVTYPE_SAM9X5

dito

>  };
>  
>  static const struct can_bittiming_const at91_bittiming_const = {
> @@ -1249,10 +1251,41 @@ static struct attribute_group at91_sysfs_attr_group = {
>  	.attrs = at91_sysfs_attrs,
>  };
>  
> +#if defined(CONFIG_OF)
> +static const struct of_device_id at91_can_dt_ids[] = {
> +	{
> +		.compatible = "atmel,at91sam9x5-can",
> +		.data = &at91_at91sam9x5_data,
> +	}, {
> +		.compatible = "atmel,at91sam9263-can",
> +		.data = &at91_at91sam9263_data,
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, at91_can_dt_ids);
> +#else
> +#define at91_can_dt_ids NULL
> +#endif
> +
> +static struct at91_devtype_data* at91_can_get_driver_data(
> +						struct platform_device *pdev)

I think it's okay to put this into one line.

> +{
> +	if (pdev->dev.of_node) {
> +		const struct of_device_id *match;
> +		match = of_match_node(at91_can_dt_ids, pdev->dev.of_node);
> +		if (!match) {
> +			dev_err(&pdev->dev, "no matching node found in dtb\n");
> +			return NULL;
> +		}
> +		return (struct at91_devtype_data *)match->data;
> +	}
> +	return (struct at91_devtype_data *)platform_get_device_id(pdev)->driver_data;
> +}
> +
>  static int at91_can_probe(struct platform_device *pdev)
>  {
>  	const struct at91_devtype_data *devtype_data;
> -	enum at91_devtype devtype;
>  	struct net_device *dev;
>  	struct at91_priv *priv;
>  	struct resource *res;
> @@ -1260,8 +1293,12 @@ static int at91_can_probe(struct platform_device *pdev)
>  	void __iomem *addr;
>  	int err, irq;
>  
> -	devtype = pdev->id_entry->driver_data;
> -	devtype_data = &at91_devtype_data[devtype];
> +	devtype_data = at91_can_get_driver_data(pdev);
> +	if (!devtype_data) {
> +		dev_err(&pdev->dev, "no driver data\n");
> +		err = -ENODEV;
> +		goto exit;
> +	}
>  
>  	clk = clk_get(&pdev->dev, "can_clk");
>  	if (IS_ERR(clk)) {
> @@ -1310,7 +1347,6 @@ static int at91_can_probe(struct platform_device *pdev)
>  	priv->dev = dev;
>  	priv->reg_base = addr;
>  	priv->devtype_data = *devtype_data;
> -	priv->devtype_data.type = devtype;
>  	priv->clk = clk;
>  	priv->pdata = pdev->dev.platform_data;
>  	priv->mb0_id = 0x7ff;
> @@ -1373,10 +1409,10 @@ static int at91_can_remove(struct platform_device *pdev)
>  static const struct platform_device_id at91_can_id_table[] = {
>  	{
>  		.name = "at91_can",
> -		.driver_data = AT91_DEVTYPE_SAM9263,
> +		.driver_data = (unsigned long)&at91_at91sam9x5_data,
>  	}, {
>  		.name = "at91sam9x5_can",
> -		.driver_data = AT91_DEVTYPE_SAM9X5,
> +		.driver_data = (unsigned long)&at91_at91sam9263_data,
>  	}, {
>  		/* sentinel */
>  	}
> @@ -1389,6 +1425,7 @@ static struct platform_driver at91_can_driver = {
>  	.driver = {
>  		.name = KBUILD_MODNAME,
>  		.owner = THIS_MODULE,
> +		.of_match_table = at91_can_dt_ids,
>  	},
>  	.id_table = at91_can_id_table,
>  };

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

Re: [PATCH 3/3] can: Kconfig: CAN_AT91 depends on ARCH_AT91

[Marc Kleine-Budde]

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

On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> 
> SAMA5D3 devices also embed CAN feature. Moreover if we want to produce a single
> kernel image (at least for Atmel devices) it is not useful to be too
> restrictive.

If it compiles on other ARMs aswell we can make it depend on ARCH_ARM.

Marc
> 
> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> ---
>  drivers/net/can/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 9862b2e..27311c3 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -65,7 +65,7 @@ config CAN_LEDS
>  
>  config CAN_AT91
>  	tristate "Atmel AT91 onchip CAN controller"
> -	depends on ARCH_AT91SAM9263 || ARCH_AT91SAM9X5
> +	depends on ARCH_AT91
>  	---help---
>  	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263
>  	  and AT91SAM9X5 processors.
> 


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

Re: [PATCH 2/3] can: at91_can: add pinctrl support

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:37 Fri 08 Mar     , Marc Kleine-Budde wrote:
> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> >  drivers/net/can/at91_can.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> > index c7f70d4..56fb2aa 100644
> > --- a/drivers/net/can/at91_can.c
> > +++ b/drivers/net/can/at91_can.c
> > @@ -28,6 +28,7 @@
> >  #include <linux/module.h>
> >  #include <linux/netdevice.h>
> >  #include <linux/of.h>
> > +#include <linux/pinctrl/consumer.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/rtnetlink.h>
> >  #include <linux/skbuff.h>
> > @@ -1292,6 +1293,7 @@ static int at91_can_probe(struct platform_device *pdev)
> >  	struct clk *clk;
> >  	void __iomem *addr;
> >  	int err, irq;
> > +	struct pinctrl *pinctrl;
> >  
> >  	devtype_data = at91_can_get_driver_data(pdev);
> >  	if (!devtype_data) {
> > @@ -1314,6 +1316,13 @@ static int at91_can_probe(struct platform_device *pdev)
> >  		goto exit_put;
> >  	}
> >  
> > +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> 
> Is this still needed? I think the pinctrl framework automatically
> enables the default pinctrl for a device if available.
yes this is useless

Best Regards,
J.
> 
> Marc
> 
> > +	if (IS_ERR(pinctrl)) {
> > +		dev_err(&pdev->dev, "Failed to request pinctrl\n");
> > +		err = PTR_ERR(pinctrl);
> > +		goto exit_put;
> > +	}
> > +
> >  	if (!request_mem_region(res->start,
> >  				resource_size(res),
> >  				pdev->name)) {
> > 
> 
> 
> -- 
> 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 1/3] can: at91_can: add dt support

[Ludovic Desroches]

On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
> On 03/08/2013 06:30 PM, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> > From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > 
> > Add device tree support.
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
> >  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
> >  2 files changed, 70 insertions(+), 19 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> > new file mode 100644
> > index 0000000..69381db
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> > @@ -0,0 +1,14 @@
> > +* AT91 CAN *
> > +
> > +Required properties:
> > +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
> 
> On imx we use the oldest SoC with that IP available. Which strategy are
> you following on at91?
> 

We are using the same strategy.

> > +  - reg: Should contain RTC registers location and length
> 
> RTC?
> 

My mistake, I'll correct that, bad copy-paste.

> > +  - interrupts: Should contain IRQ line for the CAN controller
> > +
> > +Example:
> > +
> > +	can0: can@f000c000 {
> > +		compatbile = "atmel,at91sam9x5-can";
> > +		reg = <0xf000c000 0x300>;
> > +		interrupts = <40 4 5>
> > +	};
> > diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> > index 44f3637..c7f70d4 100644
> > --- a/drivers/net/can/at91_can.c
> > +++ b/drivers/net/can/at91_can.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/netdevice.h>
> > +#include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/rtnetlink.h>
> >  #include <linux/skbuff.h>
> > @@ -155,19 +156,20 @@ struct at91_priv {
> >  	canid_t mb0_id;
> >  };
> >  
> > -static const struct at91_devtype_data at91_devtype_data[] = {
> > -	[AT91_DEVTYPE_SAM9263] = {
> > -		.rx_first = 1,
> > -		.rx_split = 8,
> > -		.rx_last = 11,
> > -		.tx_shift = 2,
> > -	},
> > -	[AT91_DEVTYPE_SAM9X5] = {
> > -		.rx_first = 0,
> > -		.rx_split = 4,
> > -		.rx_last = 5,
> > -		.tx_shift = 1,
> > -	},
> > +static struct at91_devtype_data at91_at91sam9263_data = {
> > +	.rx_first = 1,
> > +	.rx_split = 8,
> > +	.rx_last = 11,
> > +	.tx_shift = 2,
> > +	.type = AT91_DEVTYPE_SAM9263
> 
> nitpick:
> can you add a trailing ","
> 
> > +};
> > +
> > +static struct at91_devtype_data at91_at91sam9x5_data = {
> > +	.rx_first = 0,
> > +	.rx_split = 4,
> > +	.rx_last = 5,
> > +	.tx_shift = 1,
> > +	.type = AT91_DEVTYPE_SAM9X5
> 
> dito
> 

ok

> >  };
> >  
> >  static const struct can_bittiming_const at91_bittiming_const = {
> > @@ -1249,10 +1251,41 @@ static struct attribute_group at91_sysfs_attr_group = {
> >  	.attrs = at91_sysfs_attrs,
> >  };
> >  
> > +#if defined(CONFIG_OF)
> > +static const struct of_device_id at91_can_dt_ids[] = {
> > +	{
> > +		.compatible = "atmel,at91sam9x5-can",
> > +		.data = &at91_at91sam9x5_data,
> > +	}, {
> > +		.compatible = "atmel,at91sam9263-can",
> > +		.data = &at91_at91sam9263_data,
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, at91_can_dt_ids);
> > +#else
> > +#define at91_can_dt_ids NULL
> > +#endif
> > +
> > +static struct at91_devtype_data* at91_can_get_driver_data(
> > +						struct platform_device *pdev)
> 
> I think it's okay to put this into one line.
> 

ok

> > +{
> > +	if (pdev->dev.of_node) {
> > +		const struct of_device_id *match;
> > +		match = of_match_node(at91_can_dt_ids, pdev->dev.of_node);
> > +		if (!match) {
> > +			dev_err(&pdev->dev, "no matching node found in dtb\n");
> > +			return NULL;
> > +		}
> > +		return (struct at91_devtype_data *)match->data;
> > +	}
> > +	return (struct at91_devtype_data *)platform_get_device_id(pdev)->driver_data;
> > +}
> > +
> >  static int at91_can_probe(struct platform_device *pdev)
> >  {
> >  	const struct at91_devtype_data *devtype_data;
> > -	enum at91_devtype devtype;
> >  	struct net_device *dev;
> >  	struct at91_priv *priv;
> >  	struct resource *res;
> > @@ -1260,8 +1293,12 @@ static int at91_can_probe(struct platform_device *pdev)
> >  	void __iomem *addr;
> >  	int err, irq;
> >  
> > -	devtype = pdev->id_entry->driver_data;
> > -	devtype_data = &at91_devtype_data[devtype];
> > +	devtype_data = at91_can_get_driver_data(pdev);
> > +	if (!devtype_data) {
> > +		dev_err(&pdev->dev, "no driver data\n");
> > +		err = -ENODEV;
> > +		goto exit;
> > +	}
> >  
> >  	clk = clk_get(&pdev->dev, "can_clk");
> >  	if (IS_ERR(clk)) {
> > @@ -1310,7 +1347,6 @@ static int at91_can_probe(struct platform_device *pdev)
> >  	priv->dev = dev;
> >  	priv->reg_base = addr;
> >  	priv->devtype_data = *devtype_data;
> > -	priv->devtype_data.type = devtype;
> >  	priv->clk = clk;
> >  	priv->pdata = pdev->dev.platform_data;
> >  	priv->mb0_id = 0x7ff;
> > @@ -1373,10 +1409,10 @@ static int at91_can_remove(struct platform_device *pdev)
> >  static const struct platform_device_id at91_can_id_table[] = {
> >  	{
> >  		.name = "at91_can",
> > -		.driver_data = AT91_DEVTYPE_SAM9263,
> > +		.driver_data = (unsigned long)&at91_at91sam9x5_data,
> >  	}, {
> >  		.name = "at91sam9x5_can",
> > -		.driver_data = AT91_DEVTYPE_SAM9X5,
> > +		.driver_data = (unsigned long)&at91_at91sam9263_data,
> >  	}, {
> >  		/* sentinel */
> >  	}
> > @@ -1389,6 +1425,7 @@ static struct platform_driver at91_can_driver = {
> >  	.driver = {
> >  		.name = KBUILD_MODNAME,
> >  		.owner = THIS_MODULE,
> > +		.of_match_table = at91_can_dt_ids,
> >  	},
> >  	.id_table = at91_can_id_table,
> >  };
> 
> Marc

Thanks for your feedback

Regards

Ludovic

Re: [PATCH 1/3] can: at91_can: add dt support

[Ludovic Desroches]

On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > 
> > Add device tree support.
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> >  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
> >  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
> >  2 files changed, 70 insertions(+), 19 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> > new file mode 100644
> > index 0000000..69381db
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> > @@ -0,0 +1,14 @@
> > +* AT91 CAN *
> > +
> > +Required properties:
> > +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
> 
> On imx we use the oldest SoC with that IP available. Which strategy are
> you following on at91?
> 

We are using the same strategy.

> > +  - reg: Should contain RTC registers location and length
> 
> RTC?
> 

My mistake, I'll correct that, bad copy-paste.

> > +  - interrupts: Should contain IRQ line for the CAN controller
> > +
> > +Example:
> > +
> > +	can0: can@f000c000 {
> > +		compatbile = "atmel,at91sam9x5-can";
> > +		reg = <0xf000c000 0x300>;
> > +		interrupts = <40 4 5>
> > +	};
> > diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> > index 44f3637..c7f70d4 100644
> > --- a/drivers/net/can/at91_can.c
> > +++ b/drivers/net/can/at91_can.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/netdevice.h>
> > +#include <linux/of.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/rtnetlink.h>
> >  #include <linux/skbuff.h>
> > @@ -155,19 +156,20 @@ struct at91_priv {
> >  	canid_t mb0_id;
> >  };
> >  
> > -static const struct at91_devtype_data at91_devtype_data[] = {
> > -	[AT91_DEVTYPE_SAM9263] = {
> > -		.rx_first = 1,
> > -		.rx_split = 8,
> > -		.rx_last = 11,
> > -		.tx_shift = 2,
> > -	},
> > -	[AT91_DEVTYPE_SAM9X5] = {
> > -		.rx_first = 0,
> > -		.rx_split = 4,
> > -		.rx_last = 5,
> > -		.tx_shift = 1,
> > -	},
> > +static struct at91_devtype_data at91_at91sam9263_data = {
> > +	.rx_first = 1,
> > +	.rx_split = 8,
> > +	.rx_last = 11,
> > +	.tx_shift = 2,
> > +	.type = AT91_DEVTYPE_SAM9263
> 
> nitpick:
> can you add a trailing ","
> 
> > +};
> > +
> > +static struct at91_devtype_data at91_at91sam9x5_data = {
> > +	.rx_first = 0,
> > +	.rx_split = 4,
> > +	.rx_last = 5,
> > +	.tx_shift = 1,
> > +	.type = AT91_DEVTYPE_SAM9X5
> 
> dito
> 

ok

> >  };
> >  
> >  static const struct can_bittiming_const at91_bittiming_const = {
> > @@ -1249,10 +1251,41 @@ static struct attribute_group at91_sysfs_attr_group = {
> >  	.attrs = at91_sysfs_attrs,
> >  };
> >  
> > +#if defined(CONFIG_OF)
> > +static const struct of_device_id at91_can_dt_ids[] = {
> > +	{
> > +		.compatible = "atmel,at91sam9x5-can",
> > +		.data = &at91_at91sam9x5_data,
> > +	}, {
> > +		.compatible = "atmel,at91sam9263-can",
> > +		.data = &at91_at91sam9263_data,
> > +	}, {
> > +		/* sentinel */
> > +	}
> > +};
> > +MODULE_DEVICE_TABLE(of, at91_can_dt_ids);
> > +#else
> > +#define at91_can_dt_ids NULL
> > +#endif
> > +
> > +static struct at91_devtype_data* at91_can_get_driver_data(
> > +						struct platform_device *pdev)
> 
> I think it's okay to put this into one line.
> 

ok

> > +{
> > +	if (pdev->dev.of_node) {
> > +		const struct of_device_id *match;
> > +		match = of_match_node(at91_can_dt_ids, pdev->dev.of_node);
> > +		if (!match) {
> > +			dev_err(&pdev->dev, "no matching node found in dtb\n");
> > +			return NULL;
> > +		}
> > +		return (struct at91_devtype_data *)match->data;
> > +	}
> > +	return (struct at91_devtype_data *)platform_get_device_id(pdev)->driver_data;
> > +}
> > +
> >  static int at91_can_probe(struct platform_device *pdev)
> >  {
> >  	const struct at91_devtype_data *devtype_data;
> > -	enum at91_devtype devtype;
> >  	struct net_device *dev;
> >  	struct at91_priv *priv;
> >  	struct resource *res;
> > @@ -1260,8 +1293,12 @@ static int at91_can_probe(struct platform_device *pdev)
> >  	void __iomem *addr;
> >  	int err, irq;
> >  
> > -	devtype = pdev->id_entry->driver_data;
> > -	devtype_data = &at91_devtype_data[devtype];
> > +	devtype_data = at91_can_get_driver_data(pdev);
> > +	if (!devtype_data) {
> > +		dev_err(&pdev->dev, "no driver data\n");
> > +		err = -ENODEV;
> > +		goto exit;
> > +	}
> >  
> >  	clk = clk_get(&pdev->dev, "can_clk");
> >  	if (IS_ERR(clk)) {
> > @@ -1310,7 +1347,6 @@ static int at91_can_probe(struct platform_device *pdev)
> >  	priv->dev = dev;
> >  	priv->reg_base = addr;
> >  	priv->devtype_data = *devtype_data;
> > -	priv->devtype_data.type = devtype;
> >  	priv->clk = clk;
> >  	priv->pdata = pdev->dev.platform_data;
> >  	priv->mb0_id = 0x7ff;
> > @@ -1373,10 +1409,10 @@ static int at91_can_remove(struct platform_device *pdev)
> >  static const struct platform_device_id at91_can_id_table[] = {
> >  	{
> >  		.name = "at91_can",
> > -		.driver_data = AT91_DEVTYPE_SAM9263,
> > +		.driver_data = (unsigned long)&at91_at91sam9x5_data,
> >  	}, {
> >  		.name = "at91sam9x5_can",
> > -		.driver_data = AT91_DEVTYPE_SAM9X5,
> > +		.driver_data = (unsigned long)&at91_at91sam9263_data,
> >  	}, {
> >  		/* sentinel */
> >  	}
> > @@ -1389,6 +1425,7 @@ static struct platform_driver at91_can_driver = {
> >  	.driver = {
> >  		.name = KBUILD_MODNAME,
> >  		.owner = THIS_MODULE,
> > +		.of_match_table = at91_can_dt_ids,
> >  	},
> >  	.id_table = at91_can_id_table,
> >  };
> 
> Marc

Thanks for your feedback

Regards

Ludovic

Re: [PATCH 3/3] can: Kconfig: CAN_AT91 depends on ARCH_AT91

[Ludovic Desroches]

On Fri, Mar 08, 2013 at 06:44:54PM +0100, Marc Kleine-Budde wrote:
> On 03/08/2013 06:30 PM, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> > From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > 
> > SAMA5D3 devices also embed CAN feature. Moreover if we want to produce a single
> > kernel image (at least for Atmel devices) it is not useful to be too
> > restrictive.
> 
> If it compiles on other ARMs aswell we can make it depend on ARCH_ARM.
> 

I was thinking about it but I am wondering if it makes sense. Should we have a
non atmel device with atmel can?

I have no position about this point, if you think it's better to make it depending onto ARCH_ARM, I'll change it if there is no compilation issue.

Regards

Ludovic

> Marc
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/net/can/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> > index 9862b2e..27311c3 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -65,7 +65,7 @@ config CAN_LEDS
> >  
> >  config CAN_AT91
> >  	tristate "Atmel AT91 onchip CAN controller"
> > -	depends on ARCH_AT91SAM9263 || ARCH_AT91SAM9X5
> > +	depends on ARCH_AT91
> >  	---help---
> >  	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263
> >  	  and AT91SAM9X5 processors.
> > 
> 
> 
> -- 
> 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 3/3] can: Kconfig: CAN_AT91 depends on ARCH_AT91

[Ludovic Desroches]

On Fri, Mar 08, 2013 at 06:44:54PM +0100, Marc Kleine-Budde wrote:
> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> > From: Ludovic Desroches <ludovic.desroches@atmel.com>
> > 
> > SAMA5D3 devices also embed CAN feature. Moreover if we want to produce a single
> > kernel image (at least for Atmel devices) it is not useful to be too
> > restrictive.
> 
> If it compiles on other ARMs aswell we can make it depend on ARCH_ARM.
> 

I was thinking about it but I am wondering if it makes sense. Should we have a
non atmel device with atmel can?

I have no position about this point, if you think it's better to make it depending onto ARCH_ARM, I'll change it if there is no compilation issue.

Regards

Ludovic

> Marc
> > 
> > Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > ---
> >  drivers/net/can/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> > index 9862b2e..27311c3 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -65,7 +65,7 @@ config CAN_LEDS
> >  
> >  config CAN_AT91
> >  	tristate "Atmel AT91 onchip CAN controller"
> > -	depends on ARCH_AT91SAM9263 || ARCH_AT91SAM9X5
> > +	depends on ARCH_AT91
> >  	---help---
> >  	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263
> >  	  and AT91SAM9X5 processors.
> > 
> 
> 
> -- 
> 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 3/3] can: Kconfig: CAN_AT91 depends on ARCH_AT91

[Marc Kleine-Budde]

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

On 03/11/2013 10:24 AM, Ludovic Desroches wrote:
> On Fri, Mar 08, 2013 at 06:44:54PM +0100, Marc Kleine-Budde wrote:
>> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
>>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>
>>> SAMA5D3 devices also embed CAN feature. Moreover if we want to produce a single
>>> kernel image (at least for Atmel devices) it is not useful to be too
>>> restrictive.
>>
>> If it compiles on other ARMs aswell we can make it depend on ARCH_ARM.
>> 
> I was thinking about it but I am wondering if it makes sense. Should we have a
> non atmel device with atmel can?

I don't know if atmels sells the IP core separately, but ARM Linux is
going towards a multi ARCH kernel anyways. So the can core should
compile on non atmel and/or multi arch kernels.

> I have no position about this point, if you think it's better to make
> it depending onto ARCH_ARM, I'll change it if there is no compilation
> issue.

Please make it so.

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

Re: [PATCH 1/3] can: at91_can: add dt support

[Marc Kleine-Budde]

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

On 03/11/2013 10:17 AM, Ludovic Desroches wrote:
> On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
>> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
>>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>
>>> Add device tree support.
>>>
>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>> ---
>>>  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
>>>  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
>>>  2 files changed, 70 insertions(+), 19 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
>>> new file mode 100644
>>> index 0000000..69381db
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
>>> @@ -0,0 +1,14 @@
>>> +* AT91 CAN *
>>> +
>>> +Required properties:
>>> +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
>>
>> On imx we use the oldest SoC with that IP available. Which strategy are
>> you following on at91?
>>
> 
> We are using the same strategy.

But "atmel,at91sam9x5-can" isn't a specific SoC, probably a SoC family.
Or are you using "atmel,at91sam9x5-can" for the other devices in the DT,
too.

I picked at91sam9x5-can for the non devicetree driver out of the blue,
as there was no scheme established, yet. I'm not hanging on that name,
I'd rather appreciate to follow the common at91 DT rules.

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

Re: [PATCH 1/3] can: at91_can: add dt support

[Ludovic Desroches]

On Mon, Mar 11, 2013 at 04:12:46PM +0100, Marc Kleine-Budde wrote:
> On 03/11/2013 10:17 AM, Ludovic Desroches wrote:
> > On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
> >> On 03/08/2013 06:30 PM, ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org wrote:
> >>> From: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>>
> >>> Add device tree support.
> >>>
> >>> Signed-off-by: Ludovic Desroches <ludovic.desroches-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> >>> ---
> >>>  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
> >>>  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
> >>>  2 files changed, 70 insertions(+), 19 deletions(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>> new file mode 100644
> >>> index 0000000..69381db
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>> @@ -0,0 +1,14 @@
> >>> +* AT91 CAN *
> >>> +
> >>> +Required properties:
> >>> +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
> >>
> >> On imx we use the oldest SoC with that IP available. Which strategy are
> >> you following on at91?
> >>
> > 
> > We are using the same strategy.
> 
> But "atmel,at91sam9x5-can" isn't a specific SoC, probably a SoC family.
> Or are you using "atmel,at91sam9x5-can" for the other devices in the DT,
> too.
> 

You are right at91sam9x5 is not a specific SoC but a family of SoCs:
at91sam9g25, at91sam9g35, at91sam9x25 and at91sam9x35. This compatible string
is used for all the devices from and after the at91sam9x5 family: at91sam9n12,
samad3x, etc.

> I picked at91sam9x5-can for the non devicetree driver out of the blue,
> as there was no scheme established, yet. I'm not hanging on that name,
> I'd rather appreciate to follow the common at91 DT rules.

It follows at91 Dt rules, we have atmel,at91sam9x5-i2c, atmel,at91sam9x5-gpio
and others.

Regards

Ludovic

Re: [PATCH 1/3] can: at91_can: add dt support

[Ludovic Desroches]

On Mon, Mar 11, 2013 at 04:12:46PM +0100, Marc Kleine-Budde wrote:
> On 03/11/2013 10:17 AM, Ludovic Desroches wrote:
> > On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
> >> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
> >>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>>
> >>> Add device tree support.
> >>>
> >>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>> ---
> >>>  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
> >>>  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
> >>>  2 files changed, 70 insertions(+), 19 deletions(-)
> >>>  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>> new file mode 100644
> >>> index 0000000..69381db
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
> >>> @@ -0,0 +1,14 @@
> >>> +* AT91 CAN *
> >>> +
> >>> +Required properties:
> >>> +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
> >>
> >> On imx we use the oldest SoC with that IP available. Which strategy are
> >> you following on at91?
> >>
> > 
> > We are using the same strategy.
> 
> But "atmel,at91sam9x5-can" isn't a specific SoC, probably a SoC family.
> Or are you using "atmel,at91sam9x5-can" for the other devices in the DT,
> too.
> 

You are right at91sam9x5 is not a specific SoC but a family of SoCs:
at91sam9g25, at91sam9g35, at91sam9x25 and at91sam9x35. This compatible string
is used for all the devices from and after the at91sam9x5 family: at91sam9n12,
samad3x, etc.

> I picked at91sam9x5-can for the non devicetree driver out of the blue,
> as there was no scheme established, yet. I'm not hanging on that name,
> I'd rather appreciate to follow the common at91 DT rules.

It follows at91 Dt rules, we have atmel,at91sam9x5-i2c, atmel,at91sam9x5-gpio
and others.

Regards

Ludovic

Re: [PATCH 1/3] can: at91_can: add dt support

[Marc Kleine-Budde]

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

On 03/11/2013 04:39 PM, Ludovic Desroches wrote:
> On Mon, Mar 11, 2013 at 04:12:46PM +0100, Marc Kleine-Budde wrote:
>> On 03/11/2013 10:17 AM, Ludovic Desroches wrote:
>>> On Fri, Mar 08, 2013 at 06:44:05PM +0100, Marc Kleine-Budde wrote:
>>>> On 03/08/2013 06:30 PM, ludovic.desroches@atmel.com wrote:
>>>>> From: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>>
>>>>> Add device tree support.
>>>>>
>>>>> Signed-off-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>> ---
>>>>>  .../devicetree/bindings/net/can/atmel-can.txt      | 14 ++++
>>>>>  drivers/net/can/at91_can.c                         | 75 ++++++++++++++++------
>>>>>  2 files changed, 70 insertions(+), 19 deletions(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/net/can/atmel-can.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/can/atmel-can.txt b/Documentation/devicetree/bindings/net/can/atmel-can.txt
>>>>> new file mode 100644
>>>>> index 0000000..69381db
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/can/atmel-can.txt
>>>>> @@ -0,0 +1,14 @@
>>>>> +* AT91 CAN *
>>>>> +
>>>>> +Required properties:
>>>>> +  - compatible: Should be "atmel,at91sam9263-can" or "atmel,at91sam9x5-can"
>>>>
>>>> On imx we use the oldest SoC with that IP available. Which strategy are
>>>> you following on at91?
>>>>
>>>
>>> We are using the same strategy.
>>
>> But "atmel,at91sam9x5-can" isn't a specific SoC, probably a SoC family.
>> Or are you using "atmel,at91sam9x5-can" for the other devices in the DT,
>> too.
>>
> 
> You are right at91sam9x5 is not a specific SoC but a family of SoCs:
> at91sam9g25, at91sam9g35, at91sam9x25 and at91sam9x35. This compatible string
> is used for all the devices from and after the at91sam9x5 family: at91sam9n12,
> samad3x, etc.
> 
>> I picked at91sam9x5-can for the non devicetree driver out of the blue,
>> as there was no scheme established, yet. I'm not hanging on that name,
>> I'd rather appreciate to follow the common at91 DT rules.
> 
> It follows at91 Dt rules, we have atmel,at91sam9x5-i2c, atmel,at91sam9x5-gpio
> and others.

That's okay then, just wanted to clarify this. Can you send a v2, then
I'll apply the patches.

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