[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b345be6-3bd0-4410-8255-97bf661fc890@kernel.org>
Date: Tue, 23 Jan 2024 14:15:47 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Diogo Ivo <diogo.ivo@...mens.com>, 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
Subject: Re: [PATCH v2 0/8] Add support for ICSSG-based Ethernet on SR1.0
devices
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@siemens.com/
>
> 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
Powered by blists - more mailing lists