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. > > This series is based on TI's 5.10 SDK [1]. > > The first version of this patch series can be found in [2]. > > [1]: https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/?h=ti-linux-5.10.y > [2]: https://lore.kernel.org/all/20231219174548.3481-1-diogo.ivo@xxxxxxxxxxx/ > > Changes in v2: > - Addressed Krzysztof's comments on the dt-binding > - Removed explicit references to SR2.0 > - Added static keyword as indicated by the kernel test robot > > Diogo Ivo (8): > dt-bindings: net: Add support for AM65x SR1.0 in ICSSG > net: ti: icssg-config: add SR1.0-specific configuration bits > net: ti: icssg-prueth: add SR1.0-specific configuration bits > net: ti: icssg-classifier: Add support for SR1.0 > net: ti: icssg-config: Add SR1.0 configuration functions > net: ti: icssg-ethtool: Adjust channel count for SR1.0 > net: ti: iccsg-prueth: Add necessary functions for SR1.0 support > net: ti: icssg-prueth: Wire up support for SR1.0 > > .../bindings/net/ti,icssg-prueth.yaml | 29 +- > .../net/ethernet/ti/icssg/icssg_classifier.c | 113 +++- > drivers/net/ethernet/ti/icssg/icssg_config.c | 86 ++- > drivers/net/ethernet/ti/icssg/icssg_config.h | 55 ++ > drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 10 +- > drivers/net/ethernet/ti/icssg/icssg_prueth.c | 556 ++++++++++++++++-- > drivers/net/ethernet/ti/icssg/icssg_prueth.h | 21 +- > 7 files changed, 788 insertions(+), 82 deletions(-) > -- cheers, -roger