[PATCH 0/2] can: flexcan: add PE clock source select support

[PATCH 0/2] can: flexcan: add PE clock source select support

[Joakim Zhang]

The patch set intends to add support for PE clock source select. The
FlexCAN controller can parse "fsl,clk-source" property from device tree
to select clock source.

Dong Aisheng (2):
  Documentation: can: flexcan: add PE clock source property to device
    tree
  can: flexcan: add support for PE clock source select

 .../bindings/net/can/fsl-flexcan.txt           |  8 ++++++++
 drivers/net/can/flexcan.c                      | 18 ++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree

[Joakim Zhang]

From: Dong Aisheng <aisheng.dong@nxp.com>

The FlexCAN controller can parse clock source property from DTS file to
select PE clock source.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index bc77477c6878..a04168605998 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -32,6 +32,13 @@ Optional properties:
 		 ack_gpr is the gpr register offset of CAN stop acknowledge.
 		 ack_bit is the bit offset of CAN stop acknowledge.
 
+- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
+		  It's SoC Implementation dependent. Refer to RM for detailed
+		  definition. If this property is not set in device tree node
+		  then driver selects clock source 1 by default.
+		  0: clock source 0 (oscillator clock)
+		  1: clock source 1 (peripheral clock)
+
 Example:
 
 	can@1c000 {
@@ -40,4 +47,5 @@ Example:
 		interrupts = <48 0x2>;
 		interrupt-parent = <&mpic>;
 		clock-frequency = <200000000>; // filled in by bootloader
+		fsl,clk-source = <0>; // select clock source 0 for PE
 	};
-- 
2.17.1

[PATCH 2/2] can: flexcan: add support for PE clock source select

[Joakim Zhang]

From: Dong Aisheng <aisheng.dong@nxp.com>

Add support to select the clock source for CAN Protocol Engine (PE).
It's Soc Implementation dependent. Refer to RM for detailed definition
of each Soc. We select clock source 1 (peripheral clock) by default in
driver now, this patch add support to prase clock source in DTS file.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 0f36eafe3ac1..2bca867bcfaa 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -273,6 +273,8 @@ struct flexcan_priv {
 	u8 tx_mb_idx;
 	u8 mb_count;
 	u8 mb_size;
+	/* Select clock source to CAN Protocol Engine */
+	u8 clk_src;
 	u32 reg_ctrl_default;
 	u32 reg_imask1_default;
 	u32 reg_imask2_default;
@@ -1361,9 +1363,12 @@ static int register_flexcandev(struct net_device *dev)
 	err = flexcan_chip_disable(priv);
 	if (err)
 		goto out_disable_per;
-	reg = priv->read(&regs->ctrl);
-	reg |= FLEXCAN_CTRL_CLK_SRC;
-	priv->write(reg, &regs->ctrl);
+
+	if (priv->clk_src) {
+		reg = priv->read(&regs->ctrl);
+		reg |= FLEXCAN_CTRL_CLK_SRC;
+		priv->write(reg, &regs->ctrl);
+	}
 
 	err = flexcan_chip_enable(priv);
 	if (err)
@@ -1488,6 +1493,7 @@ static int flexcan_probe(struct platform_device *pdev)
 	struct clk *clk_ipg = NULL, *clk_per = NULL;
 	struct flexcan_regs __iomem *regs;
 	int err, irq;
+	u8 clk_src = 1;
 	u32 clock_freq = 0;
 
 	reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
@@ -1496,9 +1502,12 @@ static int flexcan_probe(struct platform_device *pdev)
 	else if (IS_ERR(reg_xceiver))
 		reg_xceiver = NULL;
 
-	if (pdev->dev.of_node)
+	if (pdev->dev.of_node) {
 		of_property_read_u32(pdev->dev.of_node,
 				     "clock-frequency", &clock_freq);
+		of_property_read_u8(pdev->dev.of_node,
+				    "fsl,clk-source", &clk_src);
+	}
 
 	if (!clock_freq) {
 		clk_ipg = devm_clk_get(&pdev->dev, "ipg");
@@ -1556,6 +1565,7 @@ static int flexcan_probe(struct platform_device *pdev)
 		priv->write = flexcan_write_le;
 	}
 
+	priv->clk_src = clk_src;
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
-- 
2.17.1

Re: [PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree

[Rob Herring]

On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote:
> From: Dong Aisheng <aisheng.dong@nxp.com>
> 
> The FlexCAN controller can parse clock source property from DTS file to
> select PE clock source.
> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
>  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index bc77477c6878..a04168605998 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -32,6 +32,13 @@ Optional properties:
>  		 ack_gpr is the gpr register offset of CAN stop acknowledge.
>  		 ack_bit is the bit offset of CAN stop acknowledge.
>  
> +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
> +		  It's SoC Implementation dependent. Refer to RM for detailed

If SoC dependent, then it should be implied by the SoC specific 
compatible. Also, seems like you should add clock binding support here 
if you need more clock control.

> +		  definition. If this property is not set in device tree node
> +		  then driver selects clock source 1 by default.
> +		  0: clock source 0 (oscillator clock)
> +		  1: clock source 1 (peripheral clock)
> +
>  Example:
>  
>  	can@1c000 {
> @@ -40,4 +47,5 @@ Example:
>  		interrupts = <48 0x2>;
>  		interrupt-parent = <&mpic>;
>  		clock-frequency = <200000000>; // filled in by bootloader
> +		fsl,clk-source = <0>; // select clock source 0 for PE
>  	};
> -- 
> 2.17.1
> 

RE: [PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree

[Aisheng Dong]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Friday, December 21, 2018 4:23 AM
> 
> On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote:
> > From: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > The FlexCAN controller can parse clock source property from DTS file
> > to select PE clock source.
> >
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > index bc77477c6878..a04168605998 100644
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > @@ -32,6 +32,13 @@ Optional properties:
> >  		 ack_gpr is the gpr register offset of CAN stop acknowledge.
> >  		 ack_bit is the bit offset of CAN stop acknowledge.
> >
> > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
> > +		  It's SoC Implementation dependent. Refer to RM for detailed
> 
> If SoC dependent, then it should be implied by the SoC specific compatible.
> Also, seems like you should add clock binding support here if you need more
> clock control.

The clock source selection is done by a register bit inside the IP block:
BIT13 CLKSRC CAN Engine Clock Source
0b - The CAN engine clock source is the oscillator clock. 
1b - The CAN engine clock source is the peripheral clock.

Currently it's written 1 by default during driver initialization.
drivers/net/can/flexcan.c
/* select "bus clock", chip must be disabled */
reg = priv->read(&regs->ctrl);
reg |= FLEXCAN_CTRL_CLK_SRC;
priv->write(reg, &regs->ctrl);

I'm not sure if it's a typical case to abstract CLKSRC bit into a common clock mux.
(Is there any similar case in kernel?)
But I think we can also use SoC specific compatible to write 0 for those special
Ones (currently only imx8qxp). Then this patch may not be needed.

Marc,
Please let us know if you have a different idea.

Regards
Dong Aisheng

> 
> > +		  definition. If this property is not set in device tree node
> > +		  then driver selects clock source 1 by default.
> > +		  0: clock source 0 (oscillator clock)
> > +		  1: clock source 1 (peripheral clock)
> > +
> >  Example:
> >
> >  	can@1c000 {
> > @@ -40,4 +47,5 @@ Example:
> >  		interrupts = <48 0x2>;
> >  		interrupt-parent = <&mpic>;
> >  		clock-frequency = <200000000>; // filled in by bootloader
> > +		fsl,clk-source = <0>; // select clock source 0 for PE
> >  	};
> > --
> > 2.17.1
> >

RE: [PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree

[Aisheng Dong]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Friday, December 21, 2018 4:23 AM
> 
> On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote:
> > From: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > The FlexCAN controller can parse clock source property from DTS file
> > to select PE clock source.
> >
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > index bc77477c6878..a04168605998 100644
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > @@ -32,6 +32,13 @@ Optional properties:
> >  		 ack_gpr is the gpr register offset of CAN stop acknowledge.
> >  		 ack_bit is the bit offset of CAN stop acknowledge.
> >
> > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
> > +		  It's SoC Implementation dependent. Refer to RM for detailed
> 
> If SoC dependent, then it should be implied by the SoC specific compatible.
> Also, seems like you should add clock binding support here if you need more
> clock control.

The clock source selection is done by a register bit inside the IP block:
BIT13 CLKSRC CAN Engine Clock Source
0b - The CAN engine clock source is the oscillator clock. 
1b - The CAN engine clock source is the peripheral clock.

Currently it's written 1 by default during driver initialization.
drivers/net/can/flexcan.c
/* select "bus clock", chip must be disabled */
reg = priv->read(&regs->ctrl);
reg |= FLEXCAN_CTRL_CLK_SRC;
priv->write(reg, &regs->ctrl);

I'm not sure if it's a typical case to abstract CLKSRC bit into a common clock mux.
(Is there any similar case in kernel?)
But I think we can also use SoC specific compatible to write 0 for those special
Ones (currently only imx8qxp). Then this patch may not be needed.

Marc,
Please let us know if you have a different idea.

Regards
Dong Aisheng

> 
> > +		  definition. If this property is not set in device tree node
> > +		  then driver selects clock source 1 by default.
> > +		  0: clock source 0 (oscillator clock)
> > +		  1: clock source 1 (peripheral clock)
> > +
> >  Example:
> >
> >  	can@1c000 {
> > @@ -40,4 +47,5 @@ Example:
> >  		interrupts = <48 0x2>;
> >  		interrupt-parent = <&mpic>;
> >  		clock-frequency = <200000000>; // filled in by bootloader
> > +		fsl,clk-source = <0>; // select clock source 0 for PE
> >  	};
> > --
> > 2.17.1
> >

RE: [PATCH 1/2] Documentation: can: flexcan: add PE clock source property to device tree

[Joakim Zhang]

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 2018年12月21日 4:23
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: mkl@pengutronix.de; linux-can@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>
> Subject: Re: [PATCH 1/2] Documentation: can: flexcan: add PE clock source
> property to device tree
> 
> On Thu, Dec 13, 2018 at 07:07:57AM +0000, Joakim Zhang wrote:
> > From: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > The FlexCAN controller can parse clock source property from DTS file
> > to select PE clock source.
> >
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > index bc77477c6878..a04168605998 100644
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > @@ -32,6 +32,13 @@ Optional properties:
> >  		 ack_gpr is the gpr register offset of CAN stop acknowledge.
> >  		 ack_bit is the bit offset of CAN stop acknowledge.
> >
> > +- fsl,clk-source: Select the clock source to the CAN Protocol Engine (PE).
> > +		  It's SoC Implementation dependent. Refer to RM for detailed
> 
> If SoC dependent, then it should be implied by the SoC specific compatible.
> Also, seems like you should add clock binding support here if you need more
> clock control.

Hi,

My expression here might not be very accurate, this property just help to select the clock source for
Protocol Engine (PE). FlexCAN ip support two PE clock sources, so we have to add this property to
let driver know that SoC has selected which one.

Best Regards,
Joakim Zhang

> > +		  definition. If this property is not set in device tree node
> > +		  then driver selects clock source 1 by default.
> > +		  0: clock source 0 (oscillator clock)
> > +		  1: clock source 1 (peripheral clock)
> > +
> >  Example:
> >
> >  	can@1c000 {
> > @@ -40,4 +47,5 @@ Example:
> >  		interrupts = <48 0x2>;
> >  		interrupt-parent = <&mpic>;
> >  		clock-frequency = <200000000>; // filled in by bootloader
> > +		fsl,clk-source = <0>; // select clock source 0 for PE
> >  	};
> > --
> > 2.17.1
> >

RE: [PATCH 0/2] can: flexcan: add PE clock source select support

[Joakim Zhang]

Kindly Ping...

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Joakim Zhang
> Sent: 2018年12月13日 15:08
> To: mkl@pengutronix.de; robh@kernel.org
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Joakim
> Zhang <qiangqing.zhang@nxp.com>
> Subject: [PATCH 0/2] can: flexcan: add PE clock source select support
> 
> The patch set intends to add support for PE clock source select. The FlexCAN
> controller can parse "fsl,clk-source" property from device tree to select clock
> source.
> 
> Dong Aisheng (2):
>   Documentation: can: flexcan: add PE clock source property to device
>     tree
>   can: flexcan: add support for PE clock source select
> 
>  .../bindings/net/can/fsl-flexcan.txt           |  8 ++++++++
>  drivers/net/can/flexcan.c                      | 18 ++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> --
> 2.17.1

RE: [PATCH 0/2] can: flexcan: add PE clock source select support

[Joakim Zhang]

Kindly Ping...

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Joakim Zhang
> Sent: 2019年1月17日 14:28
> To: mkl@pengutronix.de; robh@kernel.org
> Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH 0/2] can: flexcan: add PE clock source select support
> 
> 
> Kindly Ping...
> 
> Best Regards,
> Joakim Zhang
> 
> > -----Original Message-----
> > From: Joakim Zhang
> > Sent: 2018年12月13日 15:08
> > To: mkl@pengutronix.de; robh@kernel.org
> > Cc: linux-can@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Joakim
> > Zhang <qiangqing.zhang@nxp.com>
> > Subject: [PATCH 0/2] can: flexcan: add PE clock source select support
> >
> > The patch set intends to add support for PE clock source select. The
> > FlexCAN controller can parse "fsl,clk-source" property from device
> > tree to select clock source.
> >
> > Dong Aisheng (2):
> >   Documentation: can: flexcan: add PE clock source property to device
> >     tree
> >   can: flexcan: add support for PE clock source select
> >
> >  .../bindings/net/can/fsl-flexcan.txt           |  8 ++++++++
> >  drivers/net/can/flexcan.c                      | 18 ++++++++++++++----
> >  2 files changed, 22 insertions(+), 4 deletions(-)
> >
> > --
> > 2.17.1

Re: [PATCH 0/2] can: flexcan: add PE clock source select support

[Marc Kleine-Budde]

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

On 12/13/18 8:07 AM, Joakim Zhang wrote:
> The patch set intends to add support for PE clock source select. The
> FlexCAN controller can parse "fsl,clk-source" property from device tree
> to select clock source.
> 
> Dong Aisheng (2):
>   Documentation: can: flexcan: add PE clock source property to device
>     tree
>   can: flexcan: add support for PE clock source select
> 
>  .../bindings/net/can/fsl-flexcan.txt           |  8 ++++++++
>  drivers/net/can/flexcan.c                      | 18 ++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)

Added to linux-can-next.

Tnx,
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: 488 bytes --]

Re: [PATCH 0/2] can: flexcan: add PE clock source select support

[Marc Kleine-Budde]

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

On 2/27/19 9:46 AM, Marc Kleine-Budde wrote:
> On 12/13/18 8:07 AM, Joakim Zhang wrote:
>> The patch set intends to add support for PE clock source select. The
>> FlexCAN controller can parse "fsl,clk-source" property from device tree
>> to select clock source.
>>
>> Dong Aisheng (2):
>>   Documentation: can: flexcan: add PE clock source property to device
>>     tree
>>   can: flexcan: add support for PE clock source select
>>
>>  .../bindings/net/can/fsl-flexcan.txt           |  8 ++++++++
>>  drivers/net/can/flexcan.c                      | 18 ++++++++++++++----
>>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> Added to linux-can-next.

With one change:

> @@ -1369,7 +1371,10 @@ static int register_flexcandev(struct net_device *dev)
>                 goto out_clks_disable;
> 
>         reg = priv->read(&regs->ctrl);
> -       reg |= FLEXCAN_CTRL_CLK_SRC;
> +       if (priv->clk_src)
> +               reg |= FLEXCAN_CTRL_CLK_SRC;
> +       else
> +               reg &= ~FLEXCAN_CTRL_CLK_SRC;

...the FLEXCAN_CTRL_CLK_SRC bit is cleared now.

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