Re: [PATCH 1/2] can: rcar_can: Add r8a7795 support

On 02/29/2016 10:21 AM, Ramesh Shanmugasundaram wrote:

Added r8a7795 SoC support.

For smoother bit timing calculation, the rounded clock frequency is used.

Why this? From my point of view this introduces a clock error.

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   |

Hi Marc,

On 02/29/2016 10:21 AM, Ramesh Shanmugasundaram wrote:

quoted

Added r8a7795 SoC support.

For smoother bit timing calculation, the rounded clock frequency is

used.

Why this? From my point of view this introduces a clock error.

Thanks for the comments. 

Without the rounded clock, can_calc_bittiming returns different BRP values and reports high sampling error percentage. However, the rounded clock shows no such issues and it works fine for all bitrates.

Below is an e.g. from my board for an input clock differs by 2Hz.

root@salvator-x:~# can-calc-bit-timing -c 39999998 rcar_can  
Bit timing parameters for rcar_can with 39.999998 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error      CiBCR
1000000    125   2    3    2   1   5  999999  0.0% 75.0% 75.0%  0.0% 0x40040100
 800000    125   3    4    2   1   5  799999  0.0% 80.0% 80.0%  0.0% 0x60040100
 500000    250   2    3    2   1  10  499999  0.0% 87.5% 75.0% 14.3% 0x40090100
 250000    500   2    3    2   1  20  249999  0.0% 87.5% 75.0% 14.3% 0x40130100
 125000   1000   2    3    2   1  40  124999  0.0% 87.5% 75.0% 14.3% 0x40270100
 100000   1250   2    3    2   1  50   99999  0.0% 87.5% 75.0% 14.3% 0x40310100
  50000   2500   2    3    2   1 100   49999  0.0% 87.5% 75.0% 14.3% 0x40630100
  20000   6250   2    3    2   1 250   19999  0.0% 87.5% 75.0% 14.3% 0x40f90100
  10000  12500   2    3    2   1 500    9999  0.0% 87.5% 75.0% 14.3% 0x41f30100

root@salvator-x:~# can-calc-bit-timing -c 40000000 rcar_can
Bit timing parameters for rcar_can with 40.000000 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error      CiBCR
1000000     50   7    7    5   1   2 1000000  0.0% 75.0% 75.0%  0.0% 0xd0010400
 800000    125   3    4    2   1   5  800000  0.0% 80.0% 80.0%  0.0% 0x60040100
 500000    125   6    7    2   1   5  500000  0.0% 87.5% 87.5%  0.0% 0xc0040100
 250000    250   6    7    2   1  10  250000  0.0% 87.5% 87.5%  0.0% 0xc0090100
 125000    500   6    7    2   1  20  125000  0.0% 87.5% 87.5%  0.0% 0xc0130100
 100000    625   6    7    2   1  25  100000  0.0% 87.5% 87.5%  0.0% 0xc0180100
  50000   1250   6    7    2   1  50   50000  0.0% 87.5% 87.5%  0.0% 0xc0310100
  20000   3125   6    7    2   1 125   20000  0.0% 87.5% 87.5%  0.0% 0xc07c0100
  10000   6250   6    7    2   1 250   10000  0.0% 87.5% 87.5%  0.0% 0xc0f90100

Thanks,
Ramesh

On 02/29/2016 10:56 AM, Ramesh Shanmugasundaram wrote:

quoted

On 02/29/2016 10:21 AM, Ramesh Shanmugasundaram wrote:

quoted

Added r8a7795 SoC support.

For smoother bit timing calculation, the rounded clock frequency is

used.

Why this? From my point of view this introduces a clock error.

Thanks for the comments. 

Without the rounded clock, can_calc_bittiming returns different BRP
values and reports high sampling error percentage. However, the
rounded clock shows no such issues and it works fine for all
bitrates.

Below is an e.g. from my board for an input clock differs by 2Hz.

I think it's better to fix the algorithm. I'll look into it.

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   |

On 02/29/2016 10:56 AM, Ramesh Shanmugasundaram wrote:

root@salvator-x:~# can-calc-bit-timing -c 39999998 rcar_can  

Can you send me the patches to support the rcar_can. On the linux-can ML
or github pull request.

Thanks,
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   |

On 02/29/2016 11:00 AM, Marc Kleine-Budde wrote:

I think it's better to fix the algorithm. I'll look into it.

What about:

./can-calc-bit-timing -c 39999998 rcar_can                                                         
Bit timing parameters for rcar_can with 39.999998 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error      CiBCR
1000000    125   2    3    2   1   5  999999  0.0% 75.0% 75.0%  0.0% 0x40040100
 800000    125   3    4    2   1   5  799999  0.0% 80.0% 80.0%  0.0% 0x60040100
 500000    125   6    7    2   1   5  499999  0.0% 87.5% 87.5%  0.0% 0xc0040100
 250000    250   6    7    2   1  10  249999  0.0% 87.5% 87.5%  0.0% 0xc0090100
 125000    500   6    7    2   1  20  124999  0.0% 87.5% 87.5%  0.0% 0xc0130100
 100000    625   6    7    2   1  25   99999  0.0% 87.5% 87.5%  0.0% 0xc0180100
  50000   1250   6    7    2   1  50   49999  0.0% 87.5% 87.5%  0.0% 0xc0310100
  20000   3125   6    7    2   1 125   19999  0.0% 87.5% 87.5%  0.0% 0xc07c0100
  10000   6250   6    7    2   1 250    9999  0.0% 87.5% 87.5%  0.0% 0xc0f90100

But I've to port the algorithm from can-calc-bit-timing to the kernel.

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   |

Hi Marc,

On 02/29/2016 11:00 AM, Marc Kleine-Budde wrote:

quoted

I think it's better to fix the algorithm. I'll look into it.

What about:

quoted

./can-calc-bit-timing -c 39999998 rcar_can
Bit timing parameters for rcar_can with 39.999998 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error

CiBCR

quoted

1000000    125   2    3    2   1   5  999999  0.0% 75.0% 75.0%  0.0%

0x40040100

quoted

 800000    125   3    4    2   1   5  799999  0.0% 80.0% 80.0%  0.0%

0x60040100

quoted

 500000    125   6    7    2   1   5  499999  0.0% 87.5% 87.5%  0.0%

0xc0040100

quoted

 250000    250   6    7    2   1  10  249999  0.0% 87.5% 87.5%  0.0%

0xc0090100

quoted

 125000    500   6    7    2   1  20  124999  0.0% 87.5% 87.5%  0.0%

0xc0130100

quoted

 100000    625   6    7    2   1  25   99999  0.0% 87.5% 87.5%  0.0%

0xc0180100

quoted

  50000   1250   6    7    2   1  50   49999  0.0% 87.5% 87.5%  0.0%

0xc0310100

quoted

  20000   3125   6    7    2   1 125   19999  0.0% 87.5% 87.5%  0.0%

0xc07c0100

quoted

  10000   6250   6    7    2   1 250    9999  0.0% 87.5% 87.5%  0.0%

0xc0f90100

I think, it should be correct. Can I have the patch please(can-utils or kernel)? I'll give it a go.

Manual e.g. shows, for 1Mbps and 40MHz clock, two settings are possible
10Tq -> BRP 4
20Tq -> BRP 2

Condition is TSEG1 > TSEG2 >= SJW & bitrate = clock freq/(brp * number of Tq)

Your e.g. satisfies the condition and hopefully it should be OK.

Thanks,
Ramesh