netdev - Re: [PATCH v2 0/8] Add support for ICSSG-based Ethernet on SR1.0 devices

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] [day] [month] [year] [list]

Message-ID: <8c4e1e69-210c-4eb7-bd54-97adb16e7c06@siemens.com>
Date: Wed, 24 Jan 2024 09:24:13 +0000
From: Diogo Ivo <diogo.ivo@...mens.com>
To: Roger Quadros <rogerq@...nel.org>, danishanwar@...com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, andrew@...n.ch, dan.carpenter@...aro.org,
 grygorii.strashko@...com, jacob.e.keller@...el.com, robh@...nel.org,
 robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
 linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
 devicetree@...r.kernel.org
Cc: jan.kiszka@...mens.com, diogo.ivo@...mens.com
Subject: Re: [PATCH v2 0/8] Add support for ICSSG-based Ethernet on SR1.0
 devices

Hi all, thank you for your input so far.

On 1/23/24 12:15, Roger Quadros wrote:
> Hello Diogo,
>
> On 17/01/2024 18:14, Diogo Ivo wrote:
>> Hello,
>>
>> This series extends the current ICSSG-based Ethernet driver to support
>> Silicon Revision 1.0 devices.
>>
>> Notable differences between the Silicon Revisions are that there is
>> no TX core in SR1.0 with this being handled by the firmware, requiring
>> extra DMA channels to communicate commands to the firmware (with the
>> firmware being different as well) and in the packet classifier.
>>
>> The motivation behind it is that a significant number of Siemens
>> devices containing SR1.0 silicon have been deployed in the field
>> and need to be supported and updated to newer kernel versions
>> without losing functionality.
> Adding SR1.0 support with all its ifdefs makes the driver more complicated
> than it should be.
>
> I think we need to treat SR1.0 and SR2.0 as different devices with their
> own independent drivers. While the data path is pretty much the same,
> also like in am65-cpsw-nuss.c, the initialization, firmware and other
> runtime logic is significantly different.
>
> How about introducing a new icssg_prueth_sr1.c and putting all the SR1 stuff
> there? You could still re-use the other helper files in net/ti/icssg/.
> It also warrants for it's own Kconfig symbol so it can be built only
> if required.
> Any common logic could still be moved to icssg_common.c and re-used in both drivers.

Yes, that sounds like a more reasonable approach. I will refactor the code

and come back with a v3, hopefully with all patches getting sent out :)


Best regards,

Diogo