linux-kernel - Re: [RFC PATCH 1/3] can: m_can: Create m

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 3 Nov 2018 11:45:50 +0100
From:   Wolfgang Grandegger <wg@...ndegger.com>
To:     Dan Murphy <dmurphy@...com>, mkl@...gutronix.de,
        davem@...emloft.net
Cc:     linux-can@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: 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:
>> 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.