Re: [RFC PATCH 1/3] can: m_can: Create m_can core to leverage common code

Hello Dan,

Am 31.10.2018 um 21:15 schrieb Dan Murphy:

Wolfgang

Thanks for the review

On 10/27/2018 09:19 AM, Wolfgang Grandegger wrote:

quoted

Hello Dan,

for the RFC, could you please just do the necessary changes to the
existing code. We can discuss about better names, etc. later. For
the review if the common code I quickly did:

  mv m_can.c m_can_platform.c
  mv m_can_core.c m_can.c

The file names are similar to what we have for the C_CAN driver.

  s/classdev/priv/
  variable name s/m_can_dev/priv/

Then your patch 1/3 looks as shown below. I'm going to comment on that
one. The comments start with "***"....

So you would like me to align the names with the c_can driver?

That would be the obvious choice.

<snip>

quoted

*** I didn't review the rest of the patch for now.

snipped the code to reply to the comment.

quoted

Looking to the generic code, you didn't really change the way
the driver is accessing the registers. Also the interrupt handling
and rx polling is as it was before. Does that work properly using
the SPI interface of the TCAN4x5x?

I don't want to change any of that yet.  Maybe my cover letter was not clear
or did not go through.

But the intention was just to break out the functionality to create a MCAN framework
that can be used by devices that contain the Bosch MCAN core and provider their own protocal to access
the registers in the device.

I don't want to do any functional changes at this time on the IP code itself until we have a framework.
There should be no regression in the io mapped code.

I did comment on the interrupt handling and asked if a threaded work queue would affect CAN timing.
For the original TCAN driver this was the way it was implemented.

Do threaded interrupts with RX polling make sense? I think we need a
common interface allowing to select hard-irqs+napi or threaded-irqs.

Wolfgang.