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

[Wolfgang Grandegger]

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:
>> 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>
>>
>> *** I didn't review the rest of the patch for now.
>>
> 
> snipped the code to reply to the comment.
> 
>> 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.