2015-11-18 10:09:47

by Wenyou Yang

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

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


2015-11-18 10:10:05

by Wenyou Yang

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

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 <[email protected]>
---

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

2015-11-18 10:12:34

by Wenyou Yang

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

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 <[email protected]>
---

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

2015-11-18 10:23:19

by Marc Kleine-Budde

[permalink] [raw]
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 <[email protected]>

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 |


Attachments:
signature.asc (455.00 B)
OpenPGP digital signature

2015-11-18 10:26:08

by Marc Kleine-Budde

[permalink] [raw]
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.
>
> Signed-off-by: Wenyou Yang <[email protected]>

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 |


Attachments:
signature.asc (455.00 B)
OpenPGP digital signature

2015-11-19 04:25:07

by Wenyou Yang

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

Hi Marc,

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:[email protected]]
> Sent: 2015年11月18日 18:23
> To: Yang, Wenyou; Wolfgang Grandegger
> Cc: [email protected]; [email protected]; linux-
> [email protected]; 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 <[email protected]>
>
> 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

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-11-19 04:25:28

by Wenyou Yang

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

Hi Marc,

Thank you for prompt feedback.

> -----Original Message-----
> From: Marc Kleine-Budde [mailto:[email protected]]
> Sent: 2015年11月18日 18:26
> To: Yang, Wenyou; Wolfgang Grandegger
> Cc: [email protected]; [email protected]; linux-
> [email protected]; 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 <[email protected]>
>
> 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

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?