2017-08-18 19:45:39

by Franklin S Cooper Jr

[permalink] [raw]
Subject: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

During test transmitting using CAN-FD at high bitrates (4 Mbps) only
resulted in errors. Scoping the signals I noticed that only a single bit
was being transmitted and with a bit more investigation realized the actual
MCAN IP would go back to initialization mode automatically.

It appears this issue is due to the MCAN needing to use the Transmitter
Delay Compensation Mode as defined in the MCAN User's Guide. When this
mode is used the User's Guide indicates that the Transmitter Delay
Compensation Offset register should be set. The document mentions that this
register should be set to (1/dbitrate)/2*(Func Clk Freq).

Additional CAN-CIA's "Bit Time Requirements for CAN FD" document indicates
that this TDC mode is only needed for data bit rates above 2.5 Mbps.
Therefore, only enable this mode and only set TDCO when the data bit rate
is above 2.5 Mbps.

Signed-off-by: Franklin S Cooper Jr <[email protected]>
---
I'm pretty surprised that this hasn't been implemented already since
the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
supports up to 10 Mbps.

So it will be nice to get comments from users of this driver to understand
if they have been able to use CAN-FD beyond 2.5 Mbps without this patch.
If they haven't what did they do to get around it if they needed higher
speeds.

Meanwhile I plan on testing this using a more "realistic" CAN bus to insure
everything still works at 5 Mbps which is the max speed of my CAN
transceiver.

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

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index f4947a7..720e073 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -126,6 +126,12 @@ enum m_can_mram_cfg {
#define DBTP_DSJW_SHIFT 0
#define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)

+/* Transmitter Delay Compensation Register (TDCR) */
+#define TDCR_TDCO_SHIFT 8
+#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
+#define TDCR_TDCF_SHIFT 0
+#define TDCR_TDCF_MASK (0x7F << TDCR_TDCO_SHIFT)
+
/* Test Register (TEST) */
#define TEST_LBCK BIT(4)

@@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct net_device *dev)
const struct can_bittiming *dbt = &priv->can.data_bittiming;
u16 brp, sjw, tseg1, tseg2;
u32 reg_btp;
+ u32 enable_tdc = 0;
+ u32 tdco;

brp = bt->brp - 1;
sjw = bt->sjw - 1;
@@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct net_device *dev)
sjw = dbt->sjw - 1;
tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
tseg2 = dbt->phase_seg2 - 1;
+
+ /* TDC is only needed for bitrates beyond 2.5 MBit/s
+ * Specified in the "Bit Time Requirements for CAN FD" document
+ */
+ if (dbt->bitrate > 2500000) {
+ enable_tdc = DBTP_TDC;
+ /* Equation based on Bosch's M_CAN User Manual's
+ * Transmitter Delay Compensation Section
+ */
+ tdco = priv->can.clock.freq / (dbt->bitrate * 2);
+ m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
+ }
+
reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
(tseg1 << DBTP_DTSEG1_SHIFT) |
- (tseg2 << DBTP_DTSEG2_SHIFT);
+ (tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
+
m_can_write(priv, M_CAN_DBTP, reg_btp);
}

--
2.9.4.dirty


2017-09-13 21:58:40

by Franklin S Cooper Jr

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates



On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
> resulted in errors. Scoping the signals I noticed that only a single bit
> was being transmitted and with a bit more investigation realized the actual
> MCAN IP would go back to initialization mode automatically.
>
> It appears this issue is due to the MCAN needing to use the Transmitter
> Delay Compensation Mode as defined in the MCAN User's Guide. When this
> mode is used the User's Guide indicates that the Transmitter Delay
> Compensation Offset register should be set. The document mentions that this
> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>
> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document indicates
> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
> Therefore, only enable this mode and only set TDCO when the data bit rate
> is above 2.5 Mbps.
>
> Signed-off-by: Franklin S Cooper Jr <[email protected]>
> ---
> I'm pretty surprised that this hasn't been implemented already since
> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
> supports up to 10 Mbps.
>
> So it will be nice to get comments from users of this driver to understand
> if they have been able to use CAN-FD beyond 2.5 Mbps without this patch.
> If they haven't what did they do to get around it if they needed higher
> speeds.
>
> Meanwhile I plan on testing this using a more "realistic" CAN bus to insure
> everything still works at 5 Mbps which is the max speed of my CAN
> transceiver.

ping. Anyone has any thoughts on this?
>
> drivers/net/can/m_can/m_can.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index f4947a7..720e073 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -126,6 +126,12 @@ enum m_can_mram_cfg {
> #define DBTP_DSJW_SHIFT 0
> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>
> +/* Transmitter Delay Compensation Register (TDCR) */
> +#define TDCR_TDCO_SHIFT 8
> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
> +#define TDCR_TDCF_SHIFT 0
> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCO_SHIFT)
> +
> /* Test Register (TEST) */
> #define TEST_LBCK BIT(4)
>
> @@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct net_device *dev)
> const struct can_bittiming *dbt = &priv->can.data_bittiming;
> u16 brp, sjw, tseg1, tseg2;
> u32 reg_btp;
> + u32 enable_tdc = 0;
> + u32 tdco;
>
> brp = bt->brp - 1;
> sjw = bt->sjw - 1;
> @@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct net_device *dev)
> sjw = dbt->sjw - 1;
> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
> tseg2 = dbt->phase_seg2 - 1;
> +
> + /* TDC is only needed for bitrates beyond 2.5 MBit/s
> + * Specified in the "Bit Time Requirements for CAN FD" document
> + */
> + if (dbt->bitrate > 2500000) {
> + enable_tdc = DBTP_TDC;
> + /* Equation based on Bosch's M_CAN User Manual's
> + * Transmitter Delay Compensation Section
> + */
> + tdco = priv->can.clock.freq / (dbt->bitrate * 2);
> + m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
> + }
> +
> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
> (tseg1 << DBTP_DTSEG1_SHIFT) |
> - (tseg2 << DBTP_DTSEG2_SHIFT);
> + (tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
> +
> m_can_write(priv, M_CAN_DBTP, reg_btp);
> }
>
>

2017-09-14 05:06:50

by Sekhar Nori

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates


On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>
>
> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>> resulted in errors. Scoping the signals I noticed that only a single bit
>> was being transmitted and with a bit more investigation realized the actual
>> MCAN IP would go back to initialization mode automatically.
>>
>> It appears this issue is due to the MCAN needing to use the Transmitter
>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>> mode is used the User's Guide indicates that the Transmitter Delay
>> Compensation Offset register should be set. The document mentions that this
>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>
>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document indicates
>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>> Therefore, only enable this mode and only set TDCO when the data bit rate
>> is above 2.5 Mbps.
>>
>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>> ---
>> I'm pretty surprised that this hasn't been implemented already since
>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>> supports up to 10 Mbps.
>>
>> So it will be nice to get comments from users of this driver to understand
>> if they have been able to use CAN-FD beyond 2.5 Mbps without this patch.
>> If they haven't what did they do to get around it if they needed higher
>> speeds.
>>
>> Meanwhile I plan on testing this using a more "realistic" CAN bus to insure
>> everything still works at 5 Mbps which is the max speed of my CAN
>> transceiver.
>
> ping. Anyone has any thoughts on this?

I added Dong who authored the m_can driver and Wenyou who added the only
in-kernel user of the driver for any help.

Thanks,
Sekhar

>>
>> drivers/net/can/m_can/m_can.c | 24 +++++++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index f4947a7..720e073 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -126,6 +126,12 @@ enum m_can_mram_cfg {
>> #define DBTP_DSJW_SHIFT 0
>> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>>
>> +/* Transmitter Delay Compensation Register (TDCR) */
>> +#define TDCR_TDCO_SHIFT 8
>> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
>> +#define TDCR_TDCF_SHIFT 0
>> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCO_SHIFT)
>> +
>> /* Test Register (TEST) */
>> #define TEST_LBCK BIT(4)
>>
>> @@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct net_device *dev)
>> const struct can_bittiming *dbt = &priv->can.data_bittiming;
>> u16 brp, sjw, tseg1, tseg2;
>> u32 reg_btp;
>> + u32 enable_tdc = 0;
>> + u32 tdco;
>>
>> brp = bt->brp - 1;
>> sjw = bt->sjw - 1;
>> @@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct net_device *dev)
>> sjw = dbt->sjw - 1;
>> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
>> tseg2 = dbt->phase_seg2 - 1;
>> +
>> + /* TDC is only needed for bitrates beyond 2.5 MBit/s
>> + * Specified in the "Bit Time Requirements for CAN FD" document
>> + */
>> + if (dbt->bitrate > 2500000) {
>> + enable_tdc = DBTP_TDC;
>> + /* Equation based on Bosch's M_CAN User Manual's
>> + * Transmitter Delay Compensation Section
>> + */
>> + tdco = priv->can.clock.freq / (dbt->bitrate * 2);
>> + m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
>> + }
>> +
>> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
>> (tseg1 << DBTP_DTSEG1_SHIFT) |
>> - (tseg2 << DBTP_DTSEG2_SHIFT);
>> + (tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
>> +
>> m_can_write(priv, M_CAN_DBTP, reg_btp);
>> }
>>
>>

2017-09-18 03:47:33

by Wenyou Yang

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates



On 2017/9/14 13:06, Sekhar Nori wrote:
> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>
>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>> resulted in errors. Scoping the signals I noticed that only a single bit
>>> was being transmitted and with a bit more investigation realized the actual
>>> MCAN IP would go back to initialization mode automatically.
>>>
>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>> mode is used the User's Guide indicates that the Transmitter Delay
>>> Compensation Offset register should be set. The document mentions that this
>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>
>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document indicates
>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>> Therefore, only enable this mode and only set TDCO when the data bit rate
>>> is above 2.5 Mbps.
>>>
>>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>>> ---
>>> I'm pretty surprised that this hasn't been implemented already since
>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>> supports up to 10 Mbps.
>>>
>>> So it will be nice to get comments from users of this driver to understand
>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this patch.
>>> If they haven't what did they do to get around it if they needed higher
>>> speeds.
>>>
>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to insure
>>> everything still works at 5 Mbps which is the max speed of my CAN
>>> transceiver.
>> ping. Anyone has any thoughts on this?
> I added Dong who authored the m_can driver and Wenyou who added the only
> in-kernel user of the driver for any help.
I tested it on SAMA5D2 Xplained board both with and without this patch, 
both work with the 4M bps data bit rate.

>
> Thanks,
> Sekhar
>
>>> drivers/net/can/m_can/m_can.c | 24 +++++++++++++++++++++++-
>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>>> index f4947a7..720e073 100644
>>> --- a/drivers/net/can/m_can/m_can.c
>>> +++ b/drivers/net/can/m_can/m_can.c
>>> @@ -126,6 +126,12 @@ enum m_can_mram_cfg {
>>> #define DBTP_DSJW_SHIFT 0
>>> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>>>
>>> +/* Transmitter Delay Compensation Register (TDCR) */
>>> +#define TDCR_TDCO_SHIFT 8
>>> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
>>> +#define TDCR_TDCF_SHIFT 0
>>> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCO_SHIFT)
>>> +
>>> /* Test Register (TEST) */
>>> #define TEST_LBCK BIT(4)
>>>
>>> @@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct net_device *dev)
>>> const struct can_bittiming *dbt = &priv->can.data_bittiming;
>>> u16 brp, sjw, tseg1, tseg2;
>>> u32 reg_btp;
>>> + u32 enable_tdc = 0;
>>> + u32 tdco;
>>>
>>> brp = bt->brp - 1;
>>> sjw = bt->sjw - 1;
>>> @@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct net_device *dev)
>>> sjw = dbt->sjw - 1;
>>> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
>>> tseg2 = dbt->phase_seg2 - 1;
>>> +
>>> + /* TDC is only needed for bitrates beyond 2.5 MBit/s
>>> + * Specified in the "Bit Time Requirements for CAN FD" document
>>> + */
>>> + if (dbt->bitrate > 2500000) {
>>> + enable_tdc = DBTP_TDC;
>>> + /* Equation based on Bosch's M_CAN User Manual's
>>> + * Transmitter Delay Compensation Section
>>> + */
>>> + tdco = priv->can.clock.freq / (dbt->bitrate * 2);
>>> + m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
>>> + }
>>> +
>>> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw << DBTP_DSJW_SHIFT) |
>>> (tseg1 << DBTP_DTSEG1_SHIFT) |
>>> - (tseg2 << DBTP_DTSEG2_SHIFT);
>>> + (tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
>>> +
>>> m_can_write(priv, M_CAN_DBTP, reg_btp);
>>> }
>>>
>>>

Regards,
Wenyou Yang

2017-09-20 20:20:02

by Franklin S Cooper Jr

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

Hi Wenyou,

On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>
>
> On 2017/9/14 13:06, Sekhar Nori wrote:
>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>
>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>> bit
>>>> was being transmitted and with a bit more investigation realized the
>>>> actual
>>>> MCAN IP would go back to initialization mode automatically.
>>>>
>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>> Compensation Offset register should be set. The document mentions
>>>> that this
>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>
>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>> indicates
>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>> rate
>>>> is above 2.5 Mbps.
>>>>
>>>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>>>> ---
>>>> I'm pretty surprised that this hasn't been implemented already since
>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>> supports up to 10 Mbps.
>>>>
>>>> So it will be nice to get comments from users of this driver to
>>>> understand
>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>> patch.
>>>> If they haven't what did they do to get around it if they needed higher
>>>> speeds.
>>>>
>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>> insure
>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>> transceiver.
>>> ping. Anyone has any thoughts on this?
>> I added Dong who authored the m_can driver and Wenyou who added the only
>> in-kernel user of the driver for any help.
> I tested it on SAMA5D2 Xplained board both with and without this patch,
> both work with the 4M bps data bit rate.

Thank you for testing this out. Its interesting that you have been able
to use higher speeds without this patch. What is the CAN transceiver
being used on the SAMA5D2 Xplained board? I tried looking at the
schematic but it seems the CAN signals are used on an extension board
which I can't find the schematic for. Also do you mind sharing your test
setup? Were you doing a short point to point test?

Thank You,
Franklin

>
>>
>> Thanks,
>> Sekhar
>>
>>>> drivers/net/can/m_can/m_can.c | 24 +++++++++++++++++++++++-
>>>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/can/m_can/m_can.c
>>>> b/drivers/net/can/m_can/m_can.c
>>>> index f4947a7..720e073 100644
>>>> --- a/drivers/net/can/m_can/m_can.c
>>>> +++ b/drivers/net/can/m_can/m_can.c
>>>> @@ -126,6 +126,12 @@ enum m_can_mram_cfg {
>>>> #define DBTP_DSJW_SHIFT 0
>>>> #define DBTP_DSJW_MASK (0xf << DBTP_DSJW_SHIFT)
>>>> +/* Transmitter Delay Compensation Register (TDCR) */
>>>> +#define TDCR_TDCO_SHIFT 8
>>>> +#define TDCR_TDCO_MASK (0x7F << TDCR_TDCO_SHIFT)
>>>> +#define TDCR_TDCF_SHIFT 0
>>>> +#define TDCR_TDCF_MASK (0x7F << TDCR_TDCO_SHIFT)
>>>> +
>>>> /* Test Register (TEST) */
>>>> #define TEST_LBCK BIT(4)
>>>> @@ -977,6 +983,8 @@ static int m_can_set_bittiming(struct
>>>> net_device *dev)
>>>> const struct can_bittiming *dbt = &priv->can.data_bittiming;
>>>> u16 brp, sjw, tseg1, tseg2;
>>>> u32 reg_btp;
>>>> + u32 enable_tdc = 0;
>>>> + u32 tdco;
>>>> brp = bt->brp - 1;
>>>> sjw = bt->sjw - 1;
>>>> @@ -991,9 +999,23 @@ static int m_can_set_bittiming(struct
>>>> net_device *dev)
>>>> sjw = dbt->sjw - 1;
>>>> tseg1 = dbt->prop_seg + dbt->phase_seg1 - 1;
>>>> tseg2 = dbt->phase_seg2 - 1;
>>>> +
>>>> + /* TDC is only needed for bitrates beyond 2.5 MBit/s
>>>> + * Specified in the "Bit Time Requirements for CAN FD"
>>>> document
>>>> + */
>>>> + if (dbt->bitrate > 2500000) {
>>>> + enable_tdc = DBTP_TDC;
>>>> + /* Equation based on Bosch's M_CAN User Manual's
>>>> + * Transmitter Delay Compensation Section
>>>> + */
>>>> + tdco = priv->can.clock.freq / (dbt->bitrate * 2);
>>>> + m_can_write(priv, M_CAN_TDCR, tdco << TDCR_TDCO_SHIFT);
>>>> + }
>>>> +
>>>> reg_btp = (brp << DBTP_DBRP_SHIFT) | (sjw <<
>>>> DBTP_DSJW_SHIFT) |
>>>> (tseg1 << DBTP_DTSEG1_SHIFT) |
>>>> - (tseg2 << DBTP_DTSEG2_SHIFT);
>>>> + (tseg2 << DBTP_DTSEG2_SHIFT) | enable_tdc;
>>>> +
>>>> m_can_write(priv, M_CAN_DBTP, reg_btp);
>>>> }
>>>>
>
> Regards,
> Wenyou Yang
>

2017-09-20 21:38:03

by Mario Hüttel

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates



On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
> Hi Wenyou,
>
> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>
>> On 2017/9/14 13:06, Sekhar Nori wrote:
>>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>>> bit
>>>>> was being transmitted and with a bit more investigation realized the
>>>>> actual
>>>>> MCAN IP would go back to initialization mode automatically.
>>>>>
>>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>>> Compensation Offset register should be set. The document mentions
>>>>> that this
>>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>>
>>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>>> indicates
>>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>>> rate
>>>>> is above 2.5 Mbps.
>>>>>
>>>>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>>>>> ---
>>>>> I'm pretty surprised that this hasn't been implemented already since
>>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>>> supports up to 10 Mbps.
>>>>>
>>>>> So it will be nice to get comments from users of this driver to
>>>>> understand
>>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>>> patch.
>>>>> If they haven't what did they do to get around it if they needed higher
>>>>> speeds.
>>>>>
>>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>>> insure
>>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>>> transceiver.
>>>> ping. Anyone has any thoughts on this?
>>> I added Dong who authored the m_can driver and Wenyou who added the only
>>> in-kernel user of the driver for any help.
>> I tested it on SAMA5D2 Xplained board both with and without this patch,
>> both work with the 4M bps data bit rate.
> Thank you for testing this out. Its interesting that you have been able
> to use higher speeds without this patch. What is the CAN transceiver
> being used on the SAMA5D2 Xplained board? I tried looking at the
> schematic but it seems the CAN signals are used on an extension board
> which I can't find the schematic for. Also do you mind sharing your test
> setup? Were you doing a short point to point test?
>
> Thank You,
> Franklin
Hello Franklin,

your patch definitely makes sense.

I forgot the TDC in my patches because it was not present in the
previous driver versions and because I didn't encounter any
problems when testing it myself.

The error is highly dependent on the hardware (transceiver) setup.
So it is definitely possible that some people don't encounter errors
without your patch.

Could you clarify what you meant with
> Scoping the signals I noticed that only a single bit was being transmitted

Do you mean one data bit (high bit rate)  or did the core already fail
in the arbitration phase?

There is also another aspect that can lead to errors:

If the CAN clock 'cclk' is above the frequency of the interface/logic
clock 'hclk', the clock domain crossing of the CAN messages can't
work properly. However, I will throw this topic as an extra e-mail into
the round.

Mario

 


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

2017-09-21 00:49:19

by Franklin S Cooper Jr

[permalink] [raw]
Subject: Re: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates



On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>
>
> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>> Hi Wenyou,
>>
>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>
>>> On 2017/9/14 13:06, Sekhar Nori wrote:
>>>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>>>> bit
>>>>>> was being transmitted and with a bit more investigation realized the
>>>>>> actual
>>>>>> MCAN IP would go back to initialization mode automatically.
>>>>>>
>>>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>>>> Compensation Offset register should be set. The document mentions
>>>>>> that this
>>>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>>>
>>>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>>>> indicates
>>>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>>>> rate
>>>>>> is above 2.5 Mbps.
>>>>>>
>>>>>> Signed-off-by: Franklin S Cooper Jr <[email protected]>
>>>>>> ---
>>>>>> I'm pretty surprised that this hasn't been implemented already since
>>>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>>>> supports up to 10 Mbps.
>>>>>>
>>>>>> So it will be nice to get comments from users of this driver to
>>>>>> understand
>>>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>>>> patch.
>>>>>> If they haven't what did they do to get around it if they needed higher
>>>>>> speeds.
>>>>>>
>>>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>>>> insure
>>>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>>>> transceiver.
>>>>> ping. Anyone has any thoughts on this?
>>>> I added Dong who authored the m_can driver and Wenyou who added the only
>>>> in-kernel user of the driver for any help.
>>> I tested it on SAMA5D2 Xplained board both with and without this patch,
>>> both work with the 4M bps data bit rate.
>> Thank you for testing this out. Its interesting that you have been able
>> to use higher speeds without this patch. What is the CAN transceiver
>> being used on the SAMA5D2 Xplained board? I tried looking at the
>> schematic but it seems the CAN signals are used on an extension board
>> which I can't find the schematic for. Also do you mind sharing your test
>> setup? Were you doing a short point to point test?
>>
>> Thank You,
>> Franklin
> Hello Franklin,
>
> your patch definitely makes sense.
>
> I forgot the TDC in my patches because it was not present in the
> previous driver versions and because I didn't encounter any
> problems when testing it myself.
>
> The error is highly dependent on the hardware (transceiver) setup.
> So it is definitely possible that some people don't encounter errors
> without your patch.

So the Transmission Delay Compensation feature Value register is suppose
to take into consideration the transceiver delay automatically and add
the value of TDCO on top of that. So why would TDCO be dependent on the
transceiver? I've heard conflicting things regarding TDC so any
clarification on what actually impacts it would be appreciated.

Also part of the issue I'm having is how can we properly configure TDCO?
Configuring TDCO is essentially figuring out what Secondary Sample Point
to use. However, it is unclear what value to set SSP to and which use
cases a given SSP will work or doesn't work. I've seen various
recommendations from Bosch on choosing SSP but ultimately it seems they
suggestion "real world testing" to come up with a proper value. Not
setting TDCO causes problems for my device and improperly setting TDCO
causes problems for my device. So its likely any value I use could end
up breaking something for someone else.

Currently I leaning to a DT property that can be used for setting SSP.
Perhaps use a generic default value and allow individuals to override it
via DT?
>
> Could you clarify what you meant with
>> Scoping the signals I noticed that only a single bit was being transmitted
>
> Do you mean one data bit (high bit rate) or did the core already fail
> in the arbitration phase?

A single bit during the arbitration phase. Essentially the Start of
Frame bit is sent and then nothing else and the IP resets.
>
> There is also another aspect that can lead to errors:
>
> If the CAN clock 'cclk' is above the frequency of the interface/logic
> clock 'hclk', the clock domain crossing of the CAN messages can't
> work properly. However, I will throw this topic as an extra e-mail into
> the round.
>
> Mario
>
>
>