[PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Robin Holt]

This patch cleans up the documentation of the device-tree binding for
the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
properties are not used by the driver so we are removing them.

Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |   69 ++++---------------
 arch/powerpc/boot/dts/p1010rdb.dts                 |   10 +--
 arch/powerpc/boot/dts/p1010si.dtsi                 |   10 +--
 3 files changed, 21 insertions(+), 68 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 1a729f0..c78dcbb 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -1,61 +1,22 @@
-CAN Device Tree Bindings
-------------------------
-2011 Freescale Semiconductor, Inc.
+Flexcan CAN contoller on Freescale's ARM and PowerPC processors
 
-fsl,flexcan-v1.0 nodes
------------------------
-In addition to the required compatible-, reg- and interrupt-properties, you can
-also specify which clock source shall be used for the controller.
+Required properties:
 
-CPI Clock- Can Protocol Interface Clock
-	This CLK_SRC bit of CTRL(control register) selects the clock source to
-	the CAN Protocol Interface(CPI) to be either the peripheral clock
-	(driven by the PLL) or the crystal oscillator clock. The selected clock
-	is the one fed to the prescaler to generate the Serial Clock (Sclock).
-	The PRESDIV field of CTRL(control register) controls a prescaler that
-	generates the Serial Clock (Sclock), whose period defines the
-	time quantum used to compose the CAN waveform.
+- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
 
-Can Engine Clock Source
-	There are two sources for CAN clock
-	- Platform Clock  It represents the bus clock
-	- Oscillator Clock
+  An implementation should also claim any of the following compatibles
+  that it is fully backwards compatible with:
 
-	Peripheral Clock (PLL)
-	--------------
-		     |
-		    ---------		      -------------
-		    |       |CPI Clock	      | Prescaler |       Sclock
-		    |       |---------------->| (1.. 256) |------------>
-		    ---------		      -------------
-                     |  |
-	--------------  ---------------------CLK_SRC
-	Oscillator Clock
+  - fsl,p1010-flexcan
 
-- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
-			     the peripheral clock. PLL clock is fed to the
-			     prescaler to generate the Serial Clock (Sclock).
-			     Valid values are "oscillator" and "platform"
-			     "oscillator": CAN engine clock source is oscillator clock.
-			     "platform" The CAN engine clock source is the bus clock
-		             (platform clock).
+- reg : Offset and length of the register set for this device
+- interrupts : Interrupt tuple for this device
 
-- fsl,flexcan-clock-divider : for the reference and system clock, an additional
-			      clock divider can be specified.
-- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
+Example:
 
-Note:
-	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
-	- P1010 does not have oscillator as the Clock Source.So the default
-	  Clock Source is platform clock.
-Examples:
-
-	can0@1c000 {
-		compatible = "fsl,flexcan-v1.0";
-		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2>;
-		interrupt-parent = <&mpic>;
-		fsl,flexcan-clock-source = "platform";
-		fsl,flexcan-clock-divider = <2>;
-		clock-frequency = <fixed by u-boot>;
-	};
+  can@1c000 {
+          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
+          reg = <0x1c000 0x1000>;
+          interrupts = <48 0x2>;
+          interrupt-parent = <&mpic>;
+  };
diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
index 6b33b73..d6c669c 100644
--- a/arch/powerpc/boot/dts/p1010rdb.dts
+++ b/arch/powerpc/boot/dts/p1010rdb.dts
@@ -23,6 +23,8 @@
 		ethernet2 = &enet2;
 		pci0 = &pci0;
 		pci1 = &pci1;
+		can0 = &can0;
+		can1 = &can1;
 	};
 
 	memory {
@@ -169,14 +171,6 @@
 			};
 		};
 
-		can0@1c000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
-		can1@1d000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
 		usb@22000 {
 			phy_type = "utmi";
 		};
diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
index 7f51104..f00076b 100644
--- a/arch/powerpc/boot/dts/p1010si.dtsi
+++ b/arch/powerpc/boot/dts/p1010si.dtsi
@@ -140,20 +140,18 @@
 			interrupt-parent = <&mpic>;
 		};
 
-		can0@1c000 {
-			compatible = "fsl,flexcan-v1.0";
+		can0: can@1c000 {
+			compatible = "fsl,p1010-flexcan", "fsl,flexcan";
 			reg = <0x1c000 0x1000>;
 			interrupts = <48 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
-		can1@1d000 {
-			compatible = "fsl,flexcan-v1.0";
+		can1: can@1d000 {
+			compatible = "fsl,p1010-flexcan", "fsl,flexcan";
 			reg = <0x1d000 0x1000>;
 			interrupts = <61 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
 		L2: l2-cache-controller@20000 {
-- 
1.7.2.1

[PATCH v11 4/6] flexcan: Add of_match to platform_device definition.

[Robin Holt]

On powerpc, the OpenFirmware devices are not matched without specifying
an of_match array.  Introduce that array as that is used for matching
on the Freescale P1010 processor.

Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
Cc: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
---
 drivers/net/can/flexcan.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 68cbe52..662f832 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1027,8 +1027,19 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id flexcan_of_match[] = {
+	{
+		.compatible = "fsl,flexcan",
+	},
+	{},
+};
+
 static struct platform_driver flexcan_driver = {
-	.driver.name = DRV_NAME,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = flexcan_of_match,
+	},
 	.probe = flexcan_probe,
 	.remove = __devexit_p(flexcan_remove),
 };
-- 
1.7.2.1

[PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Robin Holt]

If our CAN device's device tree node has a clock-frequency property,
then use that value for the can devices clock frequency.  If not, fall
back to asking the platform/mach code for the clock frequency associated
with the flexcan device.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Kumar Gala <galak@kernel.crashing.org>
To: Wolfgang Grandegger <wg@grandegger.com>,
To: Marc Kleine-Budde <mkl@pengutronix.de>,
To: U Bhaskar-B22300 <B22300@freescale.com>
To: Scott Wood <scottwood@freescale.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: devicetree-discuss@lists.ozlabs.org
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
 drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index c78dcbb..a4382c7 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -11,6 +11,7 @@ Required properties:
 
 - reg : Offset and length of the register set for this device
 - interrupts : Interrupt tuple for this device
+- clock-frequency : The oscillator frequency driving the flexcan device
 
 Example:
 
@@ -19,4 +20,5 @@ Example:
           reg = <0x1c000 0x1000>;
           interrupts = <48 0x2>;
           interrupt-parent = <&mpic>;
+          clock-frequency = <0x0bebc1fc>;
   };
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 662f832..d40c38e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -33,6 +33,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 
 #define DRV_NAME			"flexcan"
@@ -929,12 +930,26 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	void __iomem *base;
 	resource_size_t mem_size;
 	int err, irq;
+	u32 clock_freq = 0;
 
-	clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "no clock defined\n");
-		err = PTR_ERR(clk);
-		goto failed_clock;
+	if (pdev->dev.of_node) {
+		const u32 *clock_freq_p;
+
+		clk = NULL;
+		clock_freq_p = of_get_property(pdev->dev.of_node,
+						"clock-frequency", NULL);
+		if (clock_freq_p)
+			clock_freq = *clock_freq_p;
+	}
+
+	if (!clock_freq) {
+		clk = clk_get(&pdev->dev, NULL);
+		if (IS_ERR(clk)) {
+			dev_err(&pdev->dev, "no clock defined\n");
+			err = PTR_ERR(clk);
+			goto failed_clock;
+		}
+		clock_freq = clk_get_rate(clk);
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -967,7 +982,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
 
 	priv = netdev_priv(dev);
-	priv->can.clock.freq = clk_get_rate(clk);
+	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
 	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
@@ -1002,7 +1017,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	clk_put(clk);
+	if (clk)
+		clk_put(clk);
  failed_clock:
 	return err;
 }
@@ -1020,7 +1036,8 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	clk_put(priv->clk);
+	if (priv->clk)
+		clk_put(priv->clk);
 
 	free_candev(dev);
 
-- 
1.7.2.1

Re: [PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 4851 bytes --]

On 08/11/2011 06:07 PM, Robin Holt wrote:
> If our CAN device's device tree node has a clock-frequency property,
> then use that value for the can devices clock frequency.  If not, fall
> back to asking the platform/mach code for the clock frequency associated
> with the flexcan device.

nitpicking follows inline:

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
> To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> To: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
> Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>  drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index c78dcbb..a4382c7 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -11,6 +11,7 @@ Required properties:
>  
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
> +- clock-frequency : The oscillator frequency driving the flexcan device
>  
>  Example:
>  
> @@ -19,4 +20,5 @@ Example:
>            reg = <0x1c000 0x1000>;
>            interrupts = <48 0x2>;
>            interrupt-parent = <&mpic>;
> +          clock-frequency = <0x0bebc1fc>;

Does the device tree support dec coded integers? IMHO a frequency is
best expressed in decimal.


>    };
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 662f832..d40c38e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -33,6 +33,7 @@
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  
>  #define DRV_NAME			"flexcan"
> @@ -929,12 +930,26 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	resource_size_t mem_size;
>  	int err, irq;
> +	u32 clock_freq = 0;
>  
> -	clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(clk)) {
> -		dev_err(&pdev->dev, "no clock defined\n");
> -		err = PTR_ERR(clk);
> -		goto failed_clock;
> +	if (pdev->dev.of_node) {
> +		const u32 *clock_freq_p;
> +
> +		clk = NULL;

Hmmm - what about moving the clk = NULL into the definition of clk?

> +		clock_freq_p = of_get_property(pdev->dev.of_node,
> +						"clock-frequency", NULL);
> +		if (clock_freq_p)
> +			clock_freq = *clock_freq_p;
> +	}
> +
> +	if (!clock_freq) {
> +		clk = clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(clk)) {
> +			dev_err(&pdev->dev, "no clock defined\n");
> +			err = PTR_ERR(clk);
> +			goto failed_clock;
> +		}
> +		clock_freq = clk_get_rate(clk);
>  	}
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -967,7 +982,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
>  
>  	priv = netdev_priv(dev);
> -	priv->can.clock.freq = clk_get_rate(clk);
> +	priv->can.clock.freq = clock_freq;
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
>  	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
> @@ -1002,7 +1017,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>   failed_map:
>  	release_mem_region(mem->start, mem_size);
>   failed_get:
> -	clk_put(clk);
> +	if (clk)
> +		clk_put(clk);
>   failed_clock:
>  	return err;
>  }
> @@ -1020,7 +1036,8 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> -	clk_put(priv->clk);
> +	if (priv->clk)
> +		clk_put(priv->clk);
>  
>  	free_candev(dev);
>  

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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

Re: [PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Wolfgang Grandegger]

On 08/11/2011 06:07 PM, Robin Holt wrote:
> If our CAN device's device tree node has a clock-frequency property,
> then use that value for the can devices clock frequency.  If not, fall
> back to asking the platform/mach code for the clock frequency associated
> with the flexcan device.
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
> To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> To: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
> Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>  drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index c78dcbb..a4382c7 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -11,6 +11,7 @@ Required properties:
>  
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
> +- clock-frequency : The oscillator frequency driving the flexcan device
>  
>  Example:
>  
> @@ -19,4 +20,5 @@ Example:
>            reg = <0x1c000 0x1000>;
>            interrupts = <48 0x2>;
>            interrupt-parent = <&mpic>;
> +          clock-frequency = <0x0bebc1fc>;

             clock-frequency = <200000000>; // filled in by bootloader

Is better readable. You should also add the comment.
	
Wolfgang.

Re: [PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Grant Likely]

On Thu, Aug 11, 2011 at 10:07 AM, Robin Holt <holt@sgi.com> wrote:
> This patch cleans up the documentation of the device-tree binding for
> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> properties are not used by the driver so we are removing them.
>
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> To: Grant Likely <grant.likely@secretlab.ca>
> To: Kumar Gala <galak@kernel.crashing.org>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: devicetree-discuss@lists.ozlabs.org
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   69 ++++---------------
>  arch/powerpc/boot/dts/p1010rdb.dts                 |   10 +--
>  arch/powerpc/boot/dts/p1010si.dtsi                 |   10 +--
>  3 files changed, 21 insertions(+), 68 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index 1a729f0..c78dcbb 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -1,61 +1,22 @@
> -CAN Device Tree Bindings
> -------------------------
> -2011 Freescale Semiconductor, Inc.
> +Flexcan CAN contoller on Freescale's ARM and PowerPC processors
>
> -fsl,flexcan-v1.0 nodes
> ------------------------
> -In addition to the required compatible-, reg- and interrupt-properties, you can
> -also specify which clock source shall be used for the controller.
> +Required properties:
>
> -CPI Clock- Can Protocol Interface Clock
> -       This CLK_SRC bit of CTRL(control register) selects the clock source to
> -       the CAN Protocol Interface(CPI) to be either the peripheral clock
> -       (driven by the PLL) or the crystal oscillator clock. The selected clock
> -       is the one fed to the prescaler to generate the Serial Clock (Sclock).
> -       The PRESDIV field of CTRL(control register) controls a prescaler that
> -       generates the Serial Clock (Sclock), whose period defines the
> -       time quantum used to compose the CAN waveform.
> +- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"

Don't do this.  "fsl,flexcan" is far too generic.  Be specific to the
soc part number or the ip core implementation version.

>
> -Can Engine Clock Source
> -       There are two sources for CAN clock
> -       - Platform Clock  It represents the bus clock
> -       - Oscillator Clock
> +  An implementation should also claim any of the following compatibles
> +  that it is fully backwards compatible with:
>
> -       Peripheral Clock (PLL)
> -       --------------
> -                    |
> -                   ---------                 -------------
> -                   |       |CPI Clock        | Prescaler |       Sclock
> -                   |       |---------------->| (1.. 256) |------------>
> -                   ---------                 -------------
> -                     |  |
> -       --------------  ---------------------CLK_SRC
> -       Oscillator Clock
> +  - fsl,p1010-flexcan
>
> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> -                            the peripheral clock. PLL clock is fed to the
> -                            prescaler to generate the Serial Clock (Sclock).
> -                            Valid values are "oscillator" and "platform"
> -                            "oscillator": CAN engine clock source is oscillator clock.
> -                            "platform" The CAN engine clock source is the bus clock
> -                            (platform clock).
> +- reg : Offset and length of the register set for this device
> +- interrupts : Interrupt tuple for this device
>
> -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> -                             clock divider can be specified.
> -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> +Example:
>
> -Note:
> -       - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> -       - P1010 does not have oscillator as the Clock Source.So the default
> -         Clock Source is platform clock.
> -Examples:
> -
> -       can0@1c000 {
> -               compatible = "fsl,flexcan-v1.0";
> -               reg = <0x1c000 0x1000>;
> -               interrupts = <48 0x2>;
> -               interrupt-parent = <&mpic>;
> -               fsl,flexcan-clock-source = "platform";
> -               fsl,flexcan-clock-divider = <2>;
> -               clock-frequency = <fixed by u-boot>;
> -       };
> +  can@1c000 {
> +          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> +          reg = <0x1c000 0x1000>;
> +          interrupts = <48 0x2>;
> +          interrupt-parent = <&mpic>;
> +  };

The diffstat for this patch looks too big because the whitespace has
changed.  Try to restrict whitespace changes so that the patch is
friendly to reviewers.

g.

Re: [PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Kumar Gala]

On Aug 11, 2011, at 11:18 AM, Marc Kleine-Budde wrote:

> On 08/11/2011 06:07 PM, Robin Holt wrote:
>> If our CAN device's device tree node has a clock-frequency property,
>> then use that value for the can devices clock frequency.  If not, fall
>> back to asking the platform/mach code for the clock frequency associated
>> with the flexcan device.
> 
> nitpicking follows inline:
> 
>> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
>> To: Kumar Gala <galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
>> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>,
>> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
>> To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> To: Scott Wood <scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> To: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
>> Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>
>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> ---
>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>> drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
>> 2 files changed, 27 insertions(+), 8 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> index c78dcbb..a4382c7 100644
>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> @@ -11,6 +11,7 @@ Required properties:
>> 
>> - reg : Offset and length of the register set for this device
>> - interrupts : Interrupt tuple for this device
>> +- clock-frequency : The oscillator frequency driving the flexcan device
>> 
>> Example:
>> 
>> @@ -19,4 +20,5 @@ Example:
>>           reg = <0x1c000 0x1000>;
>>           interrupts = <48 0x2>;
>>           interrupt-parent = <&mpic>;
>> +          clock-frequency = <0x0bebc1fc>;
> 
> Does the device tree support dec coded integers? IMHO a frequency is
> best expressed in decimal.

Yes it does, and agree that in the example a dec # might be better

- k

Re: [PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Thu, Aug 11, 2011 at 10:53:43AM -0600, Grant Likely wrote:
> On Thu, Aug 11, 2011 at 10:07 AM, Robin Holt <holt-sJ/iWh9BUns@public.gmane.org> wrote:
> > +- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
> 
> Don't do this.  "fsl,flexcan" is far too generic.  Be specific to the
> soc part number or the ip core implementation version.

I don't have any crumbs to go with here.  There is nothing in the
documentation I have found to indicate what this is or should be.
I looked at the documentation for the P1010 processor and there is nothing
in there which I noticed that indicates what I could possibly use other
than flexcan.  They don't even indicate the registers are equivalent or
identical to their i.MX implementations for i.MX25 and i.MX35.  The only
thing they call it is flexcan.  I have asked our local freescale rep and
he said "There is no 'chip', it is just flexcan.  flexcan is flexcan."
His tone was such that I got the feeling he thought the question was
crazy as flexcan is flexcan.

> > -Can Engine Clock Source
> > -       There are two sources for CAN clock
> > -       - Platform Clock  It represents the bus clock
> > -       - Oscillator Clock
> > +  An implementation should also claim any of the following compatibles
> > +  that it is fully backwards compatible with:
> >
> > -       Peripheral Clock (PLL)
> > -       --------------
> > -                    |
> > -                   ---------                 -------------
> > -                   |       |CPI Clock        | Prescaler |       Sclock
> > -                   |       |---------------->| (1.. 256) |------------>
> > -                   ---------                 -------------
> > -                     |  |
> > -       --------------  ---------------------CLK_SRC
> > -       Oscillator Clock
> > +  - fsl,p1010-flexcan
> >
> > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> > -                            the peripheral clock. PLL clock is fed to the
> > -                            prescaler to generate the Serial Clock (Sclock).
> > -                            Valid values are "oscillator" and "platform"
> > -                            "oscillator": CAN engine clock source is oscillator clock.
> > -                            "platform" The CAN engine clock source is the bus clock
> > -                            (platform clock).
> > +- reg : Offset and length of the register set for this device
> > +- interrupts : Interrupt tuple for this device
> >
> > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> > -                             clock divider can be specified.
> > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> > +Example:
> >
> > -Note:
> > -       - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> > -       - P1010 does not have oscillator as the Clock Source.So the default
> > -         Clock Source is platform clock.
> > -Examples:
> > -
> > -       can0@1c000 {
> > -               compatible = "fsl,flexcan-v1.0";
> > -               reg = <0x1c000 0x1000>;
> > -               interrupts = <48 0x2>;
> > -               interrupt-parent = <&mpic>;
> > -               fsl,flexcan-clock-source = "platform";
> > -               fsl,flexcan-clock-divider = <2>;
> > -               clock-frequency = <fixed by u-boot>;
> > -       };
> > +  can@1c000 {
> > +          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> > +          reg = <0x1c000 0x1000>;
> > +          interrupts = <48 0x2>;
> > +          interrupt-parent = <&mpic>;
> > +  };
> 
> The diffstat for this patch looks too big because the whitespace has
> changed.  Try to restrict whitespace changes so that the patch is
> friendly to reviewers.

Reworked the best I can.  That reduced the diffstat by 3 lines.

Robin