[PATCH 0/2] can: m_can: Add CAN clock generated by UPLLCK support

[PATCH 0/2] can: m_can: Add CAN clock generated by UPLLCK support

[Wenyou Yang]

This patch set is to make M_CAN work on SAMA5D2.

The orignal delay is not enough for M_CAN on SAMA5D2 to syschronize
the two clock domains. Increase the delay time to ensure the value
written to INIT can be read back.

Add CAN clock generated by UPLLCK(480 MHz) support, the implementation
doesn't affect the M_CAN without configuring additional CAN clock and
its generated clock.


Wenyou Yang (2):
  can: m_can: Increase delay to ensure written INIT accepted
  can: m_can: Add CAN clock generated by UPLLCK support

 drivers/net/can/m_can/m_can.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] can: m_can: Increase delay to ensure written INIT accepted

[Wenyou Yang]

Increase the delay time until the value written to INIT can be
read back to ensure that the previous value written to INIT has
been accepted.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

 drivers/net/can/m_can/m_can.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index ef65517..fd1caa0 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -320,7 +320,7 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,
 					  bool enable)
 {
 	u32 cccr = m_can_read(priv, M_CAN_CCCR);
-	u32 timeout = 10;
+	u32 timeout = 1000;
 	u32 val = 0;
 
 	if (enable) {
-- 
1.7.9.5

Re: [PATCH 1/2] can: m_can: Increase delay to ensure written INIT accepted

[Marc Kleine-Budde]

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

On 11/18/2015 11:04 AM, Wenyou Yang wrote:
> Increase the delay time until the value written to INIT can be
> read back to ensure that the previous value written to INIT has
> been accepted.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

The patch looks ok, can you please add to your commit message which SoCs
are affected by this problem.

> ---
> 
>  drivers/net/can/m_can/m_can.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index ef65517..fd1caa0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -320,7 +320,7 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,
>  					  bool enable)
>  {
>  	u32 cccr = m_can_read(priv, M_CAN_CCCR);
> -	u32 timeout = 10;
> +	u32 timeout = 1000;
>  	u32 val = 0;
>  
>  	if (enable) {
> 

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

RE: [PATCH 1/2] can: m_can: Increase delay to ensure written INIT accepted

[Yang, Wenyou]

Hi Marc,

Thank you for prompt feedback.

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: 2015年11月18日 18:26
> To: Yang, Wenyou; Wolfgang Grandegger
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Ferre, Nicolas
> Subject: Re: [PATCH 1/2] can: m_can: Increase delay to ensure written INIT
> accepted
> 
> On 11/18/2015 11:04 AM, Wenyou Yang wrote:
> > Increase the delay time until the value written to INIT can be read
> > back to ensure that the previous value written to INIT has been
> > accepted.

I tested it again with different clock frequencies, it works without this patch. Maybe I made a mistake.

Sorry about it. 

Please ignore.

> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> 
> The patch looks ok, can you please add to your commit message which SoCs are
> affected by this problem.
> 
> > ---
> >
> >  drivers/net/can/m_can/m_can.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c
> > b/drivers/net/can/m_can/m_can.c index ef65517..fd1caa0 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -320,7 +320,7 @@ static inline void m_can_config_endisable(const struct
> m_can_priv *priv,
> >  					  bool enable)
> >  {
> >  	u32 cccr = m_can_read(priv, M_CAN_CCCR);
> > -	u32 timeout = 10;
> > +	u32 timeout = 1000;
> >  	u32 val = 0;
> >
> >  	if (enable) {
> >
> 
> 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   |


Best Regards,
Wenyou Yang

[PATCH 2/2] can: m_can: Add CAN clock generated by UPLLCK support

[Wenyou Yang]

As said SAMA5D2 datasheet, it is recommended to use the CAN clock
at frequencies of 20, 40 or 80 MHz. To achieve these frequencies,
PMC GCK3 must select the UPLLCK(480 MHz) as source clock and
divide by 24, 12 or 6. In this patch the CAN clock at 20 MHz.

As it is configured through DT, it doesn't affect the M_CAN
without configuring CAN clock and its generated clock.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---

 drivers/net/can/m_can/m_can.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index fd1caa0..2ca47db 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -269,6 +269,8 @@ enum m_can_mram_cfg {
 #define TX_BUF_XTD		BIT(30)
 #define TX_BUF_RTR		BIT(29)
 
+#define AT91_CAN_CLK_FREQ	20000000
+
 /* address offset and element number for each FIFO/Buffer in the Message RAM */
 struct mram_cfg {
 	u16 off;
@@ -1188,7 +1190,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
 	struct m_can_priv *priv;
 	struct resource *res;
 	void __iomem *addr;
-	struct clk *hclk, *cclk;
+	struct clk *hclk, *cclk, *upll_clk;
 	int irq, ret;
 
 	hclk = devm_clk_get(&pdev->dev, "hclk");
@@ -1198,6 +1200,18 @@ static int m_can_plat_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	upll_clk = devm_clk_get(&pdev->dev, "upllclk");
+	if (!IS_ERR(upll_clk)) {
+		ret = clk_set_parent(cclk, upll_clk);
+		if (!ret) {
+			ret = clk_set_rate(cclk, AT91_CAN_CLK_FREQ);
+			if (ret) {
+				dev_err(&pdev->dev, "failed to set gck\n");
+				return -ENODEV;
+			}
+		}
+	}
+
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
 	addr = devm_ioremap_resource(&pdev->dev, res);
 	irq = platform_get_irq_byname(pdev, "int0");
-- 
1.7.9.5

Re: [PATCH 2/2] can: m_can: Add CAN clock generated by UPLLCK support

[Marc Kleine-Budde]

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

On 11/18/2015 11:04 AM, Wenyou Yang wrote:
> As said SAMA5D2 datasheet, it is recommended to use the CAN clock
> at frequencies of 20, 40 or 80 MHz. To achieve these frequencies,
> PMC GCK3 must select the UPLLCK(480 MHz) as source clock and
> divide by 24, 12 or 6. In this patch the CAN clock at 20 MHz.
> 
> As it is configured through DT, it doesn't affect the M_CAN
> without configuring CAN clock and its generated clock.
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>

NACK

Please do this setup in your SoC code, where you setup the clock
infrastructure or have a look at
Documentation/devicetree/bindings/clock/clock-bindings.txt "Assigned
clock parents and rates"

> ---
> 
>  drivers/net/can/m_can/m_can.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index fd1caa0..2ca47db 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -269,6 +269,8 @@ enum m_can_mram_cfg {
>  #define TX_BUF_XTD		BIT(30)
>  #define TX_BUF_RTR		BIT(29)
>  
> +#define AT91_CAN_CLK_FREQ	20000000
> +
>  /* address offset and element number for each FIFO/Buffer in the Message RAM */
>  struct mram_cfg {
>  	u16 off;
> @@ -1188,7 +1190,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
>  	struct m_can_priv *priv;
>  	struct resource *res;
>  	void __iomem *addr;
> -	struct clk *hclk, *cclk;
> +	struct clk *hclk, *cclk, *upll_clk;
>  	int irq, ret;
>  
>  	hclk = devm_clk_get(&pdev->dev, "hclk");
> @@ -1198,6 +1200,18 @@ static int m_can_plat_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	upll_clk = devm_clk_get(&pdev->dev, "upllclk");
> +	if (!IS_ERR(upll_clk)) {
> +		ret = clk_set_parent(cclk, upll_clk);
> +		if (!ret) {
> +			ret = clk_set_rate(cclk, AT91_CAN_CLK_FREQ);
> +			if (ret) {
> +				dev_err(&pdev->dev, "failed to set gck\n");
> +				return -ENODEV;
> +			}
> +		}
> +	}
> +
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
>  	addr = devm_ioremap_resource(&pdev->dev, res);
>  	irq = platform_get_irq_byname(pdev, "int0");
> 

regards,
Marc

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


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

RE: [PATCH 2/2] can: m_can: Add CAN clock generated by UPLLCK support

[Yang, Wenyou]

Hi Marc,

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: 2015年11月18日 18:23
> To: Yang, Wenyou; Wolfgang Grandegger
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Ferre, Nicolas
> Subject: Re: [PATCH 2/2] can: m_can: Add CAN clock generated by UPLLCK
> support
> 
> On 11/18/2015 11:04 AM, Wenyou Yang wrote:
> > As said SAMA5D2 datasheet, it is recommended to use the CAN clock at
> > frequencies of 20, 40 or 80 MHz. To achieve these frequencies, PMC
> > GCK3 must select the UPLLCK(480 MHz) as source clock and divide by 24,
> > 12 or 6. In this patch the CAN clock at 20 MHz.
> >
> > As it is configured through DT, it doesn't affect the M_CAN without
> > configuring CAN clock and its generated clock.
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> 
> NACK
> 
> Please do this setup in your SoC code, where you setup the clock infrastructure or
> have a look at Documentation/devicetree/bindings/clock/clock-bindings.txt
> "Assigned clock parents and rates"
Thank you for your advice. I will do this setup via DT.


> 
> > ---
> >
> >  drivers/net/can/m_can/m_can.c |   16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c
> > b/drivers/net/can/m_can/m_can.c index fd1caa0..2ca47db 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -269,6 +269,8 @@ enum m_can_mram_cfg {
> >  #define TX_BUF_XTD		BIT(30)
> >  #define TX_BUF_RTR		BIT(29)
> >
> > +#define AT91_CAN_CLK_FREQ	20000000
> > +
> >  /* address offset and element number for each FIFO/Buffer in the
> > Message RAM */  struct mram_cfg {
> >  	u16 off;
> > @@ -1188,7 +1190,7 @@ static int m_can_plat_probe(struct platform_device
> *pdev)
> >  	struct m_can_priv *priv;
> >  	struct resource *res;
> >  	void __iomem *addr;
> > -	struct clk *hclk, *cclk;
> > +	struct clk *hclk, *cclk, *upll_clk;
> >  	int irq, ret;
> >
> >  	hclk = devm_clk_get(&pdev->dev, "hclk"); @@ -1198,6 +1200,18 @@
> > static int m_can_plat_probe(struct platform_device *pdev)
> >  		return -ENODEV;
> >  	}
> >
> > +	upll_clk = devm_clk_get(&pdev->dev, "upllclk");
> > +	if (!IS_ERR(upll_clk)) {
> > +		ret = clk_set_parent(cclk, upll_clk);
> > +		if (!ret) {
> > +			ret = clk_set_rate(cclk, AT91_CAN_CLK_FREQ);
> > +			if (ret) {
> > +				dev_err(&pdev->dev, "failed to set gck\n");
> > +				return -ENODEV;
> > +			}
> > +		}
> > +	}
> > +
> >  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "m_can");
> >  	addr = devm_ioremap_resource(&pdev->dev, res);
> >  	irq = platform_get_irq_byname(pdev, "int0");
> >
> 
> regards,
> Marc
> 
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


Best Regards,
Wenyou Yang