[PATCH] can: flexcan: add hardware controller version support

[PATCH] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

From: Hui Wang <jason77.wang@gmail.com>

At least in the i.MX series, the flexcan contrller divides into ver_3
and ver_10, current driver is for ver_3 controller.

i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
Its reset value is 0xffffffff, this means ID Filter Table must be
checked when receive a packet, but the driver is designed to accept
everything during the chip start, we need to clear this register to
follow this design.

Use the data entry of the struct of_device_id to point chip specific
info, we can set hardware version for each platform.

Cc: linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Hui Wang <jason77.wang@gmail.com>
[mkl: add id_table support]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hui Wang,

can you please test if this works for you on mx6.

regards, Marc


 drivers/net/can/flexcan.c |   61 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index b429b3f..696ff8e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -34,6 +34,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pinctrl/consumer.h>
 
@@ -165,10 +166,21 @@ struct flexcan_regs {
 	u32 imask1;		/* 0x28 */
 	u32 iflag2;		/* 0x2c */
 	u32 iflag1;		/* 0x30 */
-	u32 _reserved2[19];
+	u32 crl2;		/* 0x34 */
+	u32 esr2;		/* 0x38 */
+	u32 imeur;		/* 0x3c */
+	u32 lrfr;		/* 0x40 */
+	u32 crcr;		/* 0x44 */
+	u32 rxfgmask;		/* 0x48 */
+	u32 rxfir;		/* 0x4c */
+	u32 _reserved3[12];
 	struct flexcan_mb cantxfg[64];
 };
 
+struct flexcan_devtype_data {
+	u32 hw_ver;	/* hardware controller version */
+};
+
 struct flexcan_priv {
 	struct can_priv can;
 	struct net_device *dev;
@@ -180,6 +192,15 @@ struct flexcan_priv {
 
 	struct clk *clk;
 	struct flexcan_platform_data *pdata;
+	struct flexcan_devtype_data *devtype_data;
+};
+
+static struct flexcan_devtype_data fsl_p1010_devtype_data = {
+	.hw_ver = 3,
+};
+
+static struct flexcan_devtype_data fsl_imx6q_devtype_data = {
+	.hw_ver = 10,
 };
 
 static struct can_bittiming_const flexcan_bittiming_const = {
@@ -750,6 +771,9 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_write(0x0, &regs->rx14mask);
 	flexcan_write(0x0, &regs->rx15mask);
 
+	if (priv->devtype_data->hw_ver >= 10)
+		flexcan_write(0x0, &regs->rxfgmask);
+
 	flexcan_transceiver_switch(priv, 1);
 
 	/* synchronize with the can bus */
@@ -922,8 +946,21 @@ static void __devexit unregister_flexcandev(struct net_device *dev)
 	unregister_candev(dev);
 }
 
+static const struct of_device_id flexcan_of_match[] = {
+	{ .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
+	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
+	{ /* sentinel */ },
+};
+
+static const struct platform_device_id flexcan_id_table[] = {
+	{ .name = "flexcan", .driver_data = (kernel_ulong_t)&fsl_p1010_devtype_data, },
+	{ /* sentinel */ },
+};
+
 static int __devinit flexcan_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id;
+	struct flexcan_devtype_data *devtype_data;
 	struct net_device *dev;
 	struct flexcan_priv *priv;
 	struct resource *mem;
@@ -977,11 +1014,23 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 		goto failed_alloc;
 	}
 
+	of_id = of_match_device(flexcan_of_match, &pdev->dev);
+	if (of_id) {
+		devtype_data = of_id->data;
+	} else if (pdev->id_entry->driver_data) {
+		devtype_data = (struct flexcan_devtype_data *)
+			pdev->id_entry->driver_data;
+	} else {
+		err = -ENODEV;
+		goto failed_devtype;
+	}
+
 	dev->netdev_ops = &flexcan_netdev_ops;
 	dev->irq = irq;
 	dev->flags |= IFF_ECHO;
 
 	priv = netdev_priv(dev);
+	priv->devtype_data = devtype_data;
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
@@ -993,6 +1042,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	priv->dev = dev;
 	priv->clk = clk;
 	priv->pdata = pdev->dev.platform_data;
+	priv->devtype_data = devtype_data;
 
 	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
 
@@ -1011,6 +1061,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	return 0;
 
  failed_register:
+ failed_devtype:
 	free_candev(dev);
  failed_alloc:
 	iounmap(base);
@@ -1044,13 +1095,6 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct of_device_id flexcan_of_match[] = {
-	{
-		.compatible = "fsl,p1010-flexcan",
-	},
-	{},
-};
-
 #ifdef CONFIG_PM
 static int flexcan_suspend(struct platform_device *pdev, pm_message_t state)
 {
@@ -1097,6 +1141,7 @@ static struct platform_driver flexcan_driver = {
 	.remove = __devexit_p(flexcan_remove),
 	.suspend = flexcan_suspend,
 	.resume = flexcan_resume,
+	.id_table = flexcan_id_table,
 };
 
 module_platform_driver(flexcan_driver);
-- 
1.7.10

Re: [PATCH] can: flexcan: add hardware controller version support

[Hui Wang]

Marc Kleine-Budde wrote:
> From: Hui Wang <jason77.wang@gmail.com>
>
> At least in the i.MX series, the flexcan contrller divides into ver_3
> and ver_10, current driver is for ver_3 controller.
>
> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
> Its reset value is 0xffffffff, this means ID Filter Table must be
> checked when receive a packet, but the driver is designed to accept
> everything during the chip start, we need to clear this register to
> follow this design.
>
> Use the data entry of the struct of_device_id to point chip specific
> info, we can set hardware version for each platform.
>
> Cc: linux-can@vger.kernel.org
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
> [mkl: add id_table support]
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hui Wang,
>
> can you please test if this works for you on mx6.
>   
Sorry for reply late.

This patch works well on the mx6. But i found a minor problem, please 
see below.
> regards, Marc
>
>
>  drivers/net/can/flexcan.c |   61 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>
>   
<snip>
>  	}
>  
> +	of_id = of_match_device(flexcan_of_match, &pdev->dev);
> +	if (of_id) {
> +		devtype_data = of_id->data;
> +	} else if (pdev->id_entry->driver_data) {
> +		devtype_data = (struct flexcan_devtype_data *)
> +			pdev->id_entry->driver_data;
> +	} else {
> +		err = -ENODEV;
> +		goto failed_devtype;
> +	}
> +
>  	dev->netdev_ops = &flexcan_netdev_ops;
>  	dev->irq = irq;
>  	dev->flags |= IFF_ECHO;
>  
>  	priv = netdev_priv(dev);
> +	priv->devtype_data = devtype_data;
>  	priv->can.clock.freq = clock_freq;
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
> @@ -993,6 +1042,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	priv->dev = dev;
>  	priv->clk = clk;
>  	priv->pdata = pdev->dev.platform_data;
> +	priv->devtype_data = devtype_data;
>   
Two priv->devtype_data = devtype_data; ?

It seems one of them is redundant and is not needed.

Regards,
Hui.
>  
>  	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
>  
> @@ -1011,6 +1061,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	return 0;
>  
>   failed_register:
> + failed_devtype:
>  	free_candev(dev);
>   failed_alloc:
>  	iounmap(base);
> @@ -1044,13 +1095,6 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct of_device_id flexcan_of_match[] = {
> -	{
> -		.compatible = "fsl,p1010-flexcan",
> -	},
> -	{},
> -};
> -
>  #ifdef CONFIG_PM
>  static int flexcan_suspend(struct platform_device *pdev, pm_message_t state)
>  {
> @@ -1097,6 +1141,7 @@ static struct platform_driver flexcan_driver = {
>  	.remove = __devexit_p(flexcan_remove),
>  	.suspend = flexcan_suspend,
>  	.resume = flexcan_resume,
> +	.id_table = flexcan_id_table,
>  };
>  
>  module_platform_driver(flexcan_driver);
>   

Re: [PATCH] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

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

On 07/02/2012 04:24 AM, Hui Wang wrote:
> Marc Kleine-Budde wrote:
>> From: Hui Wang <jason77.wang@gmail.com>
>>
>> At least in the i.MX series, the flexcan contrller divides into ver_3
>> and ver_10, current driver is for ver_3 controller.
>>
>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>> Its reset value is 0xffffffff, this means ID Filter Table must be
>> checked when receive a packet, but the driver is designed to accept
>> everything during the chip start, we need to clear this register to
>> follow this design.
>>
>> Use the data entry of the struct of_device_id to point chip specific
>> info, we can set hardware version for each platform.
>>
>> Cc: linux-can@vger.kernel.org
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>> [mkl: add id_table support]
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Hui Wang,
>>
>> can you please test if this works for you on mx6.
>>   
> Sorry for reply late.
> 
> This patch works well on the mx6. But i found a minor problem, please
> see below.
>> regards, Marc
>>
>>
>>  drivers/net/can/flexcan.c |   61
>> +++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 53 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>
>>   
> <snip>
>>      }
>>  
>> +    of_id = of_match_device(flexcan_of_match, &pdev->dev);
>> +    if (of_id) {
>> +        devtype_data = of_id->data;
>> +    } else if (pdev->id_entry->driver_data) {
>> +        devtype_data = (struct flexcan_devtype_data *)
>> +            pdev->id_entry->driver_data;
>> +    } else {
>> +        err = -ENODEV;
>> +        goto failed_devtype;
>> +    }
>> +
>>      dev->netdev_ops = &flexcan_netdev_ops;
>>      dev->irq = irq;
>>      dev->flags |= IFF_ECHO;
>>  
>>      priv = netdev_priv(dev);
>> +    priv->devtype_data = devtype_data;
>>      priv->can.clock.freq = clock_freq;
>>      priv->can.bittiming_const = &flexcan_bittiming_const;
>>      priv->can.do_set_mode = flexcan_set_mode;
>> @@ -993,6 +1042,7 @@ static int __devinit flexcan_probe(struct
>> platform_device *pdev)
>>      priv->dev = dev;
>>      priv->clk = clk;
>>      priv->pdata = pdev->dev.platform_data;
>> +    priv->devtype_data = devtype_data;
>>   
> Two priv->devtype_data = devtype_data; ?
> 
> It seems one of them is redundant and is not needed.

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

Re: [PATCH] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

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

On 07/02/2012 04:24 AM, Hui Wang wrote:
> Marc Kleine-Budde wrote:
>> From: Hui Wang <jason77.wang@gmail.com>
>>
>> At least in the i.MX series, the flexcan contrller divides into ver_3
>> and ver_10, current driver is for ver_3 controller.
>>
>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>> Its reset value is 0xffffffff, this means ID Filter Table must be
>> checked when receive a packet, but the driver is designed to accept
>> everything during the chip start, we need to clear this register to
>> follow this design.
>>
>> Use the data entry of the struct of_device_id to point chip specific
>> info, we can set hardware version for each platform.
>>
>> Cc: linux-can@vger.kernel.org
>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>> [mkl: add id_table support]
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>> Hui Wang,
>>
>> can you please test if this works for you on mx6.
>>   
> Sorry for reply late.
> 
> This patch works well on the mx6. But i found a minor problem, please
> see below.

I've fixed the issue in the patch series I just send (forgot to mention
that in the patch). Can I add your Acked-by and Tested-by?

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

Re: [PATCH] can: flexcan: add hardware controller version support

[Hui Wang]

Marc Kleine-Budde wrote:
> On 07/02/2012 04:24 AM, Hui Wang wrote:
>   
>> Marc Kleine-Budde wrote:
>>     
>>> From: Hui Wang <jason77.wang@gmail.com>
>>>
>>> At least in the i.MX series, the flexcan contrller divides into ver_3
>>> and ver_10, current driver is for ver_3 controller.
>>>
>>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>>> Its reset value is 0xffffffff, this means ID Filter Table must be
>>> checked when receive a packet, but the driver is designed to accept
>>> everything during the chip start, we need to clear this register to
>>> follow this design.
>>>
>>> Use the data entry of the struct of_device_id to point chip specific
>>> info, we can set hardware version for each platform.
>>>
>>> Cc: linux-can@vger.kernel.org
>>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>>> Cc: Shawn Guo <shawn.guo@linaro.org>
>>> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
>>> [mkl: add id_table support]
>>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>> ---
>>> Hui Wang,
>>>
>>> can you please test if this works for you on mx6.
>>>   
>>>       
>> Sorry for reply late.
>>
>> This patch works well on the mx6. But i found a minor problem, please
>> see below.
>>     
>
> I've fixed the issue in the patch series I just send (forgot to mention
> that in the patch). Can I add your Acked-by and Tested-by?
>
>   
OK, no problem. :-).

> Marc
>   

Re: [PATCH] can: flexcan: add hardware controller version support

[Marc Kleine-Budde]

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

Hello,

On 07/01/2012 10:21 PM, Marc Kleine-Budde wrote:
> From: Hui Wang <jason77.wang@gmail.com>
> 
> At least in the i.MX series, the flexcan contrller divides into ver_3
> and ver_10, current driver is for ver_3 controller.

Has someone access to the freescale hardware people and can figure out
the controller versions used on:

imx28
imx25
imx35
imx53

Because I noticed that the FLEXCAN_CTRL_CLK_SRC bit (bit 13 of the
FLEXCAN control register) is marked as reserved in the mx53 datasheet
(although its meaning is documented) as well as in the imx6q. Is there a
Changelog of this core available?

> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
> Its reset value is 0xffffffff, this means ID Filter Table must be
> checked when receive a packet, but the driver is designed to accept
> everything during the chip start, we need to clear this register to
> follow this design.
> 
> Use the data entry of the struct of_device_id to point chip specific
> info, we can set hardware version for each platform.

cheers, 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: 262 bytes --]

Re: [PATCH] can: flexcan: add hardware controller version support

[Hui Wang]

Add Richard.Zhao in, maybe he can give a help.

Regards,
Hui.

Marc Kleine-Budde wrote:
> Hello,
>
> On 07/01/2012 10:21 PM, Marc Kleine-Budde wrote:
>   
>> From: Hui Wang <jason77.wang@gmail.com>
>>
>> At least in the i.MX series, the flexcan contrller divides into ver_3
>> and ver_10, current driver is for ver_3 controller.
>>     
>
> Has someone access to the freescale hardware people and can figure out
> the controller versions used on:
>
> imx28
> imx25
> imx35
> imx53
>
> Because I noticed that the FLEXCAN_CTRL_CLK_SRC bit (bit 13 of the
> FLEXCAN control register) is marked as reserved in the mx53 datasheet
> (although its meaning is documented) as well as in the imx6q. Is there a
> Changelog of this core available?
>
>   
>> i.MX6 has ver_10 controller, it has more reigsters than ver_3 has.
>> The rxfgmask (Rx FIFO Global Mask) register is one of the new added.
>> Its reset value is 0xffffffff, this means ID Filter Table must be
>> checked when receive a packet, but the driver is designed to accept
>> everything during the chip start, we need to clear this register to
>> follow this design.
>>
>> Use the data entry of the struct of_device_id to point chip specific
>> info, we can set hardware version for each platform.
>>     
>
> cheers, Marc
>
>