RE: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates

[Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>]

> >>>>>>> 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 <fcooper@ti.com>
> >>>>>>>> ---
> >>>>>>>> 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?
> >
> > Sounds reasonable. What's the status of this series?
> 
> I have had some offline discussions with Franklin on this, and I am not
> fully convinced that DT is the way to go here (although I don't have the
> agreement with Franklin there).
> 
> There are two components in configuring the secondary sample point. It is
> the transceiver loopback delay and an offset (example half of the bit time
> in data phase).
> 
> While the transceiver loopback delay is pretty board dependent (and thus
> amenable to DT encoding), I am not quite sure the offset can be configured
> in DT because its not really board dependent.
> 
> Unfortunately, offset calculation does not seem to be an exact science.
> There are recommendations ranging from using 50% of bit time to making it
> same as the sample point configured. This means users who need to change
> the SSP due to offset variations need to change  their DT even without
> anything changing on their board.
> 
> Since we have a netlink socket interface to configure sample point, I
> wonder if that should be extended to configure SSP too (or at least the
> offset part of SSP)?

+1

I also wonder how a default TDCO setting in DT would help. Is the driver going to hard code the Tq settings as well for the most commonly used bitrate? A link up start-up script (using netlink) would help setting a default TDCO along with other params if this is the main concern.

Thanks,
Ramesh