[PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Pankaj Bansal]

The clocking information is missing from flexcan device tree bindings.
This information is needed to be able to use flexcan. Document the same.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    Dependencies:
    - https://lore.kernel.org/patchwork/cover/1024536/
    
    V2:
    - Removed clock-frequency from optional properties
    - Added Dependencies in Notes

 .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index a04168605998..73c616ca0f9a 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -12,9 +12,42 @@ Required properties:
 - reg : Offset and length of the register set for this device
 - interrupts : Interrupt tuple for this device
 
-Optional properties:
+Clocking information is must for flexcan. please refer below info for
+understanding clocking in flexcan:
+
+• The FLEXCAN module is divided into two blocks. Controller host interface
+  ("CHI") and Protocol Engine ("PE")
+• Both these blocks require clock.
+• CHI is responsible for registers read write including MB read/write.
+  While PE is responsible for Transfer/receive data on CAN bus.
+• The clocks feeding to these two blocks can be synchronous (i.e. same clock)
+  or asynchronous (i.e. separate clocks).
+• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
+  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
+  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
+                 clock is not used.
+• If this bit is not implemented in SOC, then SOC only supports asynchronous
+  clocks.
+• Either of the clock can be generated by any of the clock source.
+• When the two clocks are asynchronous, then following restrictions apply to
+  PE clock.
+  - PE clock must be less than CHI clock.
+• If low jitter is required on CAN bus, dedicated oscillator can be used to
+  provide PE clock, but it must be less than CHI clock.
+
+Base on above information clocking info in flexcan can be defined in two ways:
+
+Method 1(Preferred):
+  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
+    - "ipg": Protocol Engine clock
+    - "per": Controller host interface clock
+  - clock-names: Must contain the clock names described just above.
 
-- clock-frequency : The oscillator frequency driving the flexcan device
+Method 2(Not Preferred):
+  - clock-frequency : The synchronous clock frequency supplied to both
+  Controller host interface and Protocol Engine
+
+Optional properties:
 
 - xceiver-supply: Regulator that powers the CAN transceiver
 
@@ -47,5 +80,14 @@ Example:
 		interrupts = <48 0x2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
+	};
+
+	can@2180000 {
+		compatible = "fsl,lx2160ar1-flexcan";
+		reg = <0x0 0x2180000 0x0 0x10000>;
+		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&sysclk>, <&clockgen 4 7>;
+		clock-names = "ipg", "per";
 		fsl,clk-source = <0>; // select clock source 0 for PE
+		status = "disabled";
 	};
-- 
2.17.1

RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Joakim Zhang]

> -----Original Message-----
> From: linux-can-owner@vger.kernel.org <linux-can-owner@vger.kernel.org>
> On Behalf Of Pankaj Bansal
> Sent: 2019年5月16日 16:12
> To: Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark
> Rutland <mark.rutland@arm.com>; Wolfgang Grandegger
> <wg@grandegger.com>; Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> Subject: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> information
> 
> The clocking information is missing from flexcan device tree bindings.
> This information is needed to be able to use flexcan. Document the same.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
> 
> Notes:
>     Dependencies:
>     -
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fpatchwork%2Fcover%2F1024536%2F&amp;data=02%7C01%7Cqia
> ngqing.zhang%40nxp.com%7C42617583d2c242521a8e08d6d9d63ee7%7C68
> 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911534405181&a
> mp;sdata=UOC7mIHPsnZbK2742mTjYkL3%2BmC7EqUP4E127UM5bWY%3D&
> amp;reserved=0
> 
>     V2:
>     - Removed clock-frequency from optional properties
>     - Added Dependencies in Notes
> 
>  .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index a04168605998..73c616ca0f9a 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -12,9 +12,42 @@ Required properties:
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
> 
> -Optional properties:
> +Clocking information is must for flexcan. please refer below info for
> +understanding clocking in flexcan:
> +
> +• The FLEXCAN module is divided into two blocks. Controller host
> +interface
> +  ("CHI") and Protocol Engine ("PE")
> +• Both these blocks require clock.
> +• CHI is responsible for registers read write including MB read/write.
> +  While PE is responsible for Transfer/receive data on CAN bus.
> +• The clocks feeding to these two blocks can be synchronous (i.e. same
> +clock)
> +  or asynchronous (i.e. separate clocks).
> +• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
> +  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
> +  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
> +                 clock is not used.
> +• If this bit is not implemented in SOC, then SOC only supports
> +asynchronous
> +  clocks.
> +• Either of the clock can be generated by any of the clock source.
> +• When the two clocks are asynchronous, then following restrictions
> +apply to
> +  PE clock.
> +  - PE clock must be less than CHI clock.
> +• If low jitter is required on CAN bus, dedicated oscillator can be
> +used to
> +  provide PE clock, but it must be less than CHI clock.
> +
> +Base on above information clocking info in flexcan can be defined in two
> ways:
> +
> +Method 1(Preferred):
> +  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
> +    - "ipg": Protocol Engine clock
> +    - "per": Controller host interface clock

[Joakim Zhang] In flexcan driver now, "ipg" is for CHI clock, "per" is for PE clock. I think it's unnecessary to exchange here.

Best Regards,
Joakim Zhang
> +  - clock-names: Must contain the clock names described just above.
> 
> -- clock-frequency : The oscillator frequency driving the flexcan device
> +Method 2(Not Preferred):
> +  - clock-frequency : The synchronous clock frequency supplied to both
> +  Controller host interface and Protocol Engine
> +
> +Optional properties:
> 
>  - xceiver-supply: Regulator that powers the CAN transceiver
> 
> @@ -47,5 +80,14 @@ Example:
>  		interrupts = <48 0x2>;
>  		interrupt-parent = <&mpic>;
>  		clock-frequency = <200000000>; // filled in by bootloader
> +	};
> +
> +	can@2180000 {
> +		compatible = "fsl,lx2160ar1-flexcan";
> +		reg = <0x0 0x2180000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&sysclk>, <&clockgen 4 7>;
> +		clock-names = "ipg", "per";
>  		fsl,clk-source = <0>; // select clock source 0 for PE
> +		status = "disabled";
>  	};
> --
> 2.17.1

RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Pankaj Bansal]

> -----Original Message-----
> From: Joakim Zhang
> Sent: Friday, 17 May, 2019 06:41 AM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li <leoyang.li@nxp.com>;
> Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> Wolfgang Grandegger <wg@grandegger.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> information
> 
> 
> > -----Original Message-----
> > From: linux-can-owner@vger.kernel.org
> > <linux-can-owner@vger.kernel.org> On Behalf Of Pankaj Bansal
> > Sent: 2019年5月16日 16:12
> > To: Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> > Mark Rutland <mark.rutland@arm.com>; Wolfgang Grandegger
> > <wg@grandegger.com>; Marc Kleine-Budde <mkl@pengutronix.de>
> > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> > information
> >
> > The clocking information is missing from flexcan device tree bindings.
> > This information is needed to be able to use flexcan. Document the same.
> >
> > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > ---
> >
> > Notes:
> >     Dependencies:
> >     -
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .ker
> nel.org%2Fpatchwork%2Fcover%2F1024536%2F&amp;data=02%7C01%7Cqia
> > ngqing.zhang%40nxp.com%7C42617583d2c242521a8e08d6d9d63ee7%7C68
> > 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911534405181&a
> > mp;sdata=UOC7mIHPsnZbK2742mTjYkL3%2BmC7EqUP4E127UM5bWY%3D&
> > amp;reserved=0
> >
> >     V2:
> >     - Removed clock-frequency from optional properties
> >     - Added Dependencies in Notes
> >
> >  .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
> >  1 file changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > index a04168605998..73c616ca0f9a 100644
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > @@ -12,9 +12,42 @@ Required properties:
> >  - reg : Offset and length of the register set for this device
> >  - interrupts : Interrupt tuple for this device
> >
> > -Optional properties:
> > +Clocking information is must for flexcan. please refer below info for
> > +understanding clocking in flexcan:
> > +
> > +• The FLEXCAN module is divided into two blocks. Controller host
> > +interface
> > +  ("CHI") and Protocol Engine ("PE")
> > +• Both these blocks require clock.
> > +• CHI is responsible for registers read write including MB read/write.
> > +  While PE is responsible for Transfer/receive data on CAN bus.
> > +• The clocks feeding to these two blocks can be synchronous (i.e.
> > +same
> > +clock)
> > +  or asynchronous (i.e. separate clocks).
> > +• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
> > +  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
> > +  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
> > +                 clock is not used.
> > +• If this bit is not implemented in SOC, then SOC only supports
> > +asynchronous
> > +  clocks.
> > +• Either of the clock can be generated by any of the clock source.
> > +• When the two clocks are asynchronous, then following restrictions
> > +apply to
> > +  PE clock.
> > +  - PE clock must be less than CHI clock.
> > +• If low jitter is required on CAN bus, dedicated oscillator can be
> > +used to
> > +  provide PE clock, but it must be less than CHI clock.
> > +
> > +Base on above information clocking info in flexcan can be defined in
> > +two
> > ways:
> > +
> > +Method 1(Preferred):
> > +  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
> > +    - "ipg": Protocol Engine clock
> > +    - "per": Controller host interface clock
> 
> [Joakim Zhang] In flexcan driver now, "ipg" is for CHI clock, "per" is for PE clock.
> I think it's unnecessary to exchange here.

In flexcan driver now, same is clock supplied to both CHI and PE blocks using CTRL[CLK_SRC] = 1
https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan.c#L1365

And clock_freq is being calculated using "per".
https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan.c#L1515

Therefore, "per" is the CHI clock and "ipg" is PE clock.
> 
> Best Regards,
> Joakim Zhang
> > +  - clock-names: Must contain the clock names described just above.
> >
> > -- clock-frequency : The oscillator frequency driving the flexcan
> > device
> > +Method 2(Not Preferred):
> > +  - clock-frequency : The synchronous clock frequency supplied to
> > +both
> > +  Controller host interface and Protocol Engine
> > +
> > +Optional properties:
> >
> >  - xceiver-supply: Regulator that powers the CAN transceiver
> >
> > @@ -47,5 +80,14 @@ Example:
> >  		interrupts = <48 0x2>;
> >  		interrupt-parent = <&mpic>;
> >  		clock-frequency = <200000000>; // filled in by bootloader
> > +	};
> > +
> > +	can@2180000 {
> > +		compatible = "fsl,lx2160ar1-flexcan";
> > +		reg = <0x0 0x2180000 0x0 0x10000>;
> > +		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&sysclk>, <&clockgen 4 7>;
> > +		clock-names = "ipg", "per";
> >  		fsl,clk-source = <0>; // select clock source 0 for PE
> > +		status = "disabled";
> >  	};
> > --
> > 2.17.1

RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Joakim Zhang]

> -----Original Message-----
> From: Pankaj Bansal
> Sent: 2019年5月17日 17:12
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Leo Li <leoyang.li@nxp.com>;
> Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> Wolfgang Grandegger <wg@grandegger.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> information
> 
> 
> 
> > -----Original Message-----
> > From: Joakim Zhang
> > Sent: Friday, 17 May, 2019 06:41 AM
> > To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li
> > <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland
> > <mark.rutland@arm.com>; Wolfgang Grandegger <wg@grandegger.com>;
> Marc
> > Kleine-Budde <mkl@pengutronix.de>
> > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> > information
> >
> >
> > > -----Original Message-----
> > > From: linux-can-owner@vger.kernel.org
> > > <linux-can-owner@vger.kernel.org> On Behalf Of Pankaj Bansal
> > > Sent: 2019年5月16日 16:12
> > > To: Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> > > Mark Rutland <mark.rutland@arm.com>; Wolfgang Grandegger
> > > <wg@grandegger.com>; Marc Kleine-Budde <mkl@pengutronix.de>
> > > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > > Subject: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> > > information
> > >
> > > The clocking information is missing from flexcan device tree bindings.
> > > This information is needed to be able to use flexcan. Document the same.
> > >
> > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > ---
> > >
> > > Notes:
> > >     Dependencies:
> > >     -
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re
> > > .ker
> >
> nel.org%2Fpatchwork%2Fcover%2F1024536%2F&amp;data=02%7C01%7Cqia
> > >
> ngqing.zhang%40nxp.com%7C42617583d2c242521a8e08d6d9d63ee7%7C68
> > >
> 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911534405181&a
> > >
> mp;sdata=UOC7mIHPsnZbK2742mTjYkL3%2BmC7EqUP4E127UM5bWY%3D&
> > > amp;reserved=0
> > >
> > >     V2:
> > >     - Removed clock-frequency from optional properties
> > >     - Added Dependencies in Notes
> > >
> > >  .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
> > >  1 file changed, 44 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > index a04168605998..73c616ca0f9a 100644
> > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > @@ -12,9 +12,42 @@ Required properties:
> > >  - reg : Offset and length of the register set for this device
> > >  - interrupts : Interrupt tuple for this device
> > >
> > > -Optional properties:
> > > +Clocking information is must for flexcan. please refer below info
> > > +for understanding clocking in flexcan:
> > > +
> > > +• The FLEXCAN module is divided into two blocks. Controller host
> > > +interface
> > > +  ("CHI") and Protocol Engine ("PE") • Both these blocks require
> > > +clock.
> > > +• CHI is responsible for registers read write including MB read/write.
> > > +  While PE is responsible for Transfer/receive data on CAN bus.
> > > +• The clocks feeding to these two blocks can be synchronous (i.e.
> > > +same
> > > +clock)
> > > +  or asynchronous (i.e. separate clocks).
> > > +• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
> > > +  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
> > > +  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
> > > +                 clock is not used.
> > > +• If this bit is not implemented in SOC, then SOC only supports
> > > +asynchronous
> > > +  clocks.
> > > +• Either of the clock can be generated by any of the clock source.
> > > +• When the two clocks are asynchronous, then following restrictions
> > > +apply to
> > > +  PE clock.
> > > +  - PE clock must be less than CHI clock.
> > > +• If low jitter is required on CAN bus, dedicated oscillator can be
> > > +used to
> > > +  provide PE clock, but it must be less than CHI clock.
> > > +
> > > +Base on above information clocking info in flexcan can be defined
> > > +in two
> > > ways:
> > > +
> > > +Method 1(Preferred):
> > > +  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
> > > +    - "ipg": Protocol Engine clock
> > > +    - "per": Controller host interface clock
> >
> > [Joakim Zhang] In flexcan driver now, "ipg" is for CHI clock, "per" is for PE
> clock.
> > I think it's unnecessary to exchange here.
> 
> In flexcan driver now, same is clock supplied to both CHI and PE blocks using
> CTRL[CLK_SRC] = 1
> https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan.c#L136
> 5
> 
> And clock_freq is being calculated using "per".
> https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan.c#L151
> 5
> 
> Therefore, "per" is the CHI clock and "ipg" is PE clock.

As you said, clock_freq calculated by "per", and you can find that priv->can.clock.freq = clock_freq.
And can.clock.freq is used to calculate TQ. You know PE clock is for CAN bus. So I think "per" is for PE.

Did I misunderstand?

Best Regards,
Joakim Zhang
> > Best Regards,
> > Joakim Zhang
> > > +  - clock-names: Must contain the clock names described just above.
> > >
> > > -- clock-frequency : The oscillator frequency driving the flexcan
> > > device
> > > +Method 2(Not Preferred):
> > > +  - clock-frequency : The synchronous clock frequency supplied to
> > > +both
> > > +  Controller host interface and Protocol Engine
> > > +
> > > +Optional properties:
> > >
> > >  - xceiver-supply: Regulator that powers the CAN transceiver
> > >
> > > @@ -47,5 +80,14 @@ Example:
> > >  		interrupts = <48 0x2>;
> > >  		interrupt-parent = <&mpic>;
> > >  		clock-frequency = <200000000>; // filled in by bootloader
> > > +	};
> > > +
> > > +	can@2180000 {
> > > +		compatible = "fsl,lx2160ar1-flexcan";
> > > +		reg = <0x0 0x2180000 0x0 0x10000>;
> > > +		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> > > +		clocks = <&sysclk>, <&clockgen 4 7>;
> > > +		clock-names = "ipg", "per";
> > >  		fsl,clk-source = <0>; // select clock source 0 for PE
> > > +		status = "disabled";
> > >  	};
> > > --
> > > 2.17.1

RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Pankaj Bansal]

> -----Original Message-----
> From: Joakim Zhang
> Sent: Friday, 17 May, 2019 03:38 PM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li <leoyang.li@nxp.com>;
> Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> Wolfgang Grandegger <wg@grandegger.com>; Marc Kleine-Budde
> <mkl@pengutronix.de>
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> information
> 
> 
> > -----Original Message-----
> > From: Pankaj Bansal
> > Sent: 2019年5月17日 17:12
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>; Leo Li
> > <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland
> > <mark.rutland@arm.com>; Wolfgang Grandegger <wg@grandegger.com>;
> Marc
> > Kleine-Budde <mkl@pengutronix.de>
> > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> > information
> >
> >
> >
> > > -----Original Message-----
> > > From: Joakim Zhang
> > > Sent: Friday, 17 May, 2019 06:41 AM
> > > To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li
> > > <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland
> > > <mark.rutland@arm.com>; Wolfgang Grandegger <wg@grandegger.com>;
> > Marc
> > > Kleine-Budde <mkl@pengutronix.de>
> > > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > > Subject: RE: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan
> clocks'
> > > information
> > >
> > >
> > > > -----Original Message-----
> > > > From: linux-can-owner@vger.kernel.org
> > > > <linux-can-owner@vger.kernel.org> On Behalf Of Pankaj Bansal
> > > > Sent: 2019年5月16日 16:12
> > > > To: Leo Li <leoyang.li@nxp.com>; Rob Herring <robh+dt@kernel.org>;
> > > > Mark Rutland <mark.rutland@arm.com>; Wolfgang Grandegger
> > > > <wg@grandegger.com>; Marc Kleine-Budde <mkl@pengutronix.de>
> > > > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org
> > > > Subject: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks'
> > > > information
> > > >
> > > > The clocking information is missing from flexcan device tree bindings.
> > > > This information is needed to be able to use flexcan. Document the same.
> > > >
> > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > ---
> > > >
> > > > Notes:
> > > >     Dependencies:
> > > >     -
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > lo
> > > > re
> > > > .ker
> > >
> > nel.org%2Fpatchwork%2Fcover%2F1024536%2F&amp;data=02%7C01%7Cqia
> > > >
> > ngqing.zhang%40nxp.com%7C42617583d2c242521a8e08d6d9d63ee7%7C68
> > > >
> > 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636935911534405181&a
> > > >
> > mp;sdata=UOC7mIHPsnZbK2742mTjYkL3%2BmC7EqUP4E127UM5bWY%3D&
> > > > amp;reserved=0
> > > >
> > > >     V2:
> > > >     - Removed clock-frequency from optional properties
> > > >     - Added Dependencies in Notes
> > > >
> > > >  .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
> > > >  1 file changed, 44 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > index a04168605998..73c616ca0f9a 100644
> > > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > @@ -12,9 +12,42 @@ Required properties:
> > > >  - reg : Offset and length of the register set for this device
> > > >  - interrupts : Interrupt tuple for this device
> > > >
> > > > -Optional properties:
> > > > +Clocking information is must for flexcan. please refer below info
> > > > +for understanding clocking in flexcan:
> > > > +
> > > > +• The FLEXCAN module is divided into two blocks. Controller host
> > > > +interface
> > > > +  ("CHI") and Protocol Engine ("PE") • Both these blocks require
> > > > +clock.
> > > > +• CHI is responsible for registers read write including MB read/write.
> > > > +  While PE is responsible for Transfer/receive data on CAN bus.
> > > > +• The clocks feeding to these two blocks can be synchronous (i.e.
> > > > +same
> > > > +clock)
> > > > +  or asynchronous (i.e. separate clocks).
> > > > +• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
> > > > +  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
> > > > +  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
> > > > +                 clock is not used.
> > > > +• If this bit is not implemented in SOC, then SOC only supports
> > > > +asynchronous
> > > > +  clocks.
> > > > +• Either of the clock can be generated by any of the clock source.
> > > > +• When the two clocks are asynchronous, then following
> > > > +restrictions apply to
> > > > +  PE clock.
> > > > +  - PE clock must be less than CHI clock.
> > > > +• If low jitter is required on CAN bus, dedicated oscillator can
> > > > +be used to
> > > > +  provide PE clock, but it must be less than CHI clock.
> > > > +
> > > > +Base on above information clocking info in flexcan can be defined
> > > > +in two
> > > > ways:
> > > > +
> > > > +Method 1(Preferred):
> > > > +  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
> > > > +    - "ipg": Protocol Engine clock
> > > > +    - "per": Controller host interface clock
> > >
> > > [Joakim Zhang] In flexcan driver now, "ipg" is for CHI clock, "per"
> > > is for PE
> > clock.
> > > I think it's unnecessary to exchange here.
> >
> > In flexcan driver now, same is clock supplied to both CHI and PE
> > blocks using CTRL[CLK_SRC] = 1
> > https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan
> > .c#L136
> > 5
> >
> > And clock_freq is being calculated using "per".
> > https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan
> > .c#L151
> > 5
> >
> > Therefore, "per" is the CHI clock and "ipg" is PE clock.
> 
> As you said, clock_freq calculated by "per", and you can find that priv-
> >can.clock.freq = clock_freq.
> And can.clock.freq is used to calculate TQ. You know PE clock is for CAN bus. So
> I think "per" is for PE.

PER is being used for CHI as well as PE (because CTRL[CLK_SRC] = 1 https://elixir.bootlin.com/linux/v5.1.3/source/drivers/net/can/flexcan.c#L136)
ONLY CHI clock can be supplied to both CHI and PE. (PE clock can only be supplied to PE provided CTRL[CLK_SRC] = 0, and PE clock < CHI clock )

> 
> Did I misunderstand?
> 
> Best Regards,
> Joakim Zhang
> > > Best Regards,
> > > Joakim Zhang
> > > > +  - clock-names: Must contain the clock names described just above.
> > > >
> > > > -- clock-frequency : The oscillator frequency driving the flexcan
> > > > device
> > > > +Method 2(Not Preferred):
> > > > +  - clock-frequency : The synchronous clock frequency supplied to
> > > > +both
> > > > +  Controller host interface and Protocol Engine
> > > > +
> > > > +Optional properties:
> > > >
> > > >  - xceiver-supply: Regulator that powers the CAN transceiver
> > > >
> > > > @@ -47,5 +80,14 @@ Example:
> > > >  		interrupts = <48 0x2>;
> > > >  		interrupt-parent = <&mpic>;
> > > >  		clock-frequency = <200000000>; // filled in by bootloader
> > > > +	};
> > > > +
> > > > +	can@2180000 {
> > > > +		compatible = "fsl,lx2160ar1-flexcan";
> > > > +		reg = <0x0 0x2180000 0x0 0x10000>;
> > > > +		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> > > > +		clocks = <&sysclk>, <&clockgen 4 7>;
> > > > +		clock-names = "ipg", "per";
> > > >  		fsl,clk-source = <0>; // select clock source 0 for PE
> > > > +		status = "disabled";
> > > >  	};
> > > > --
> > > > 2.17.1

Re: [PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Rob Herring]

On Thu, May 16, 2019 at 08:12:26AM +0000, Pankaj Bansal wrote:
> The clocking information is missing from flexcan device tree bindings.
> This information is needed to be able to use flexcan. Document the same.

'dt-bindings: can: ...' for the subject.

> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
> 
> Notes:
>     Dependencies:
>     - https://lore.kernel.org/patchwork/cover/1024536/
>     
>     V2:
>     - Removed clock-frequency from optional properties
>     - Added Dependencies in Notes
> 
>  .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
>  1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index a04168605998..73c616ca0f9a 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -12,9 +12,42 @@ Required properties:
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
>  
> -Optional properties:
> +Clocking information is must for flexcan. please refer below info for
> +understanding clocking in flexcan:
> +
> +• The FLEXCAN module is divided into two blocks. Controller host interface
> +  ("CHI") and Protocol Engine ("PE")
> +• Both these blocks require clock.
> +• CHI is responsible for registers read write including MB read/write.
> +  While PE is responsible for Transfer/receive data on CAN bus.
> +• The clocks feeding to these two blocks can be synchronous (i.e. same clock)
> +  or asynchronous (i.e. separate clocks).
> +• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
> +  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
> +  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
> +                 clock is not used.
> +• If this bit is not implemented in SOC, then SOC only supports asynchronous
> +  clocks.
> +• Either of the clock can be generated by any of the clock source.
> +• When the two clocks are asynchronous, then following restrictions apply to
> +  PE clock.
> +  - PE clock must be less than CHI clock.
> +• If low jitter is required on CAN bus, dedicated oscillator can be used to
> +  provide PE clock, but it must be less than CHI clock.
> +
> +Base on above information clocking info in flexcan can be defined in two ways:

s/Base/Based/

> +
> +Method 1(Preferred):
> +  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
> +    - "ipg": Protocol Engine clock
> +    - "per": Controller host interface clock
> +  - clock-names: Must contain the clock names described just above.
>  
> -- clock-frequency : The oscillator frequency driving the flexcan device
> +Method 2(Not Preferred):
> +  - clock-frequency : The synchronous clock frequency supplied to both
> +  Controller host interface and Protocol Engine
> +
> +Optional properties:
>  
>  - xceiver-supply: Regulator that powers the CAN transceiver
>  
> @@ -47,5 +80,14 @@ Example:
>  		interrupts = <48 0x2>;
>  		interrupt-parent = <&mpic>;
>  		clock-frequency = <200000000>; // filled in by bootloader
> +	};
> +
> +	can@2180000 {
> +		compatible = "fsl,lx2160ar1-flexcan";
> +		reg = <0x0 0x2180000 0x0 0x10000>;
> +		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&sysclk>, <&clockgen 4 7>;
> +		clock-names = "ipg", "per";
>  		fsl,clk-source = <0>; // select clock source 0 for PE
> +		status = "disabled";

Don't show status in examples.

>  	};
> -- 
> 2.17.1
> 

[PATCH v2 1/2] Documentation: can: flexcan: Add flexcan clocks' information

[Pankaj Bansal]

The clocking information is missing from flexcan device tree bindings.
This information is needed to be able to use flexcan. Document the same.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---

Notes:
    Dependencies:
    - https://lore.kernel.org/patchwork/cover/1024536/
    
    V2:
    - Removed clock-frequency from optional properties
    - Added Dependencies in Notes

 .../bindings/net/can/fsl-flexcan.txt         | 46 +++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index a04168605998..73c616ca0f9a 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -12,9 +12,42 @@ Required properties:
 - reg : Offset and length of the register set for this device
 - interrupts : Interrupt tuple for this device
 
-Optional properties:
+Clocking information is must for flexcan. please refer below info for
+understanding clocking in flexcan:
+
+• The FLEXCAN module is divided into two blocks. Controller host interface
+  ("CHI") and Protocol Engine ("PE")
+• Both these blocks require clock.
+• CHI is responsible for registers read write including MB read/write.
+  While PE is responsible for Transfer/receive data on CAN bus.
+• The clocks feeding to these two blocks can be synchronous (i.e. same clock)
+  or asynchronous (i.e. separate clocks).
+• Selection is made in the CLK_SRC bit (bit 13) of Control 1 Register.
+  - CLK_SRC = 0, asynchronous i.e. separate clocks for CHI and PE
+  - CLK_SRC = 1, synchronous i.e. CHI clock is used for PE and PE
+                 clock is not used.
+• If this bit is not implemented in SOC, then SOC only supports asynchronous
+  clocks.
+• Either of the clock can be generated by any of the clock source.
+• When the two clocks are asynchronous, then following restrictions apply to
+  PE clock.
+  - PE clock must be less than CHI clock.
+• If low jitter is required on CAN bus, dedicated oscillator can be used to
+  provide PE clock, but it must be less than CHI clock.
+
+Base on above information clocking info in flexcan can be defined in two ways:
+
+Method 1(Preferred):
+  - clocks: phandle to the clocks feeding the flexcan. Two can be given:
+    - "ipg": Protocol Engine clock
+    - "per": Controller host interface clock
+  - clock-names: Must contain the clock names described just above.
 
-- clock-frequency : The oscillator frequency driving the flexcan device
+Method 2(Not Preferred):
+  - clock-frequency : The synchronous clock frequency supplied to both
+  Controller host interface and Protocol Engine
+
+Optional properties:
 
 - xceiver-supply: Regulator that powers the CAN transceiver
 
@@ -47,5 +80,14 @@ Example:
 		interrupts = <48 0x2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
+	};
+
+	can@2180000 {
+		compatible = "fsl,lx2160ar1-flexcan";
+		reg = <0x0 0x2180000 0x0 0x10000>;
+		interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&sysclk>, <&clockgen 4 7>;
+		clock-names = "ipg", "per";
 		fsl,clk-source = <0>; // select clock source 0 for PE
+		status = "disabled";
 	};
-- 
2.17.1