Re: [RFC PATCH 2/2] net: selpcimac: Add driver for SEL PCIe network adapter

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hello,

On Mon, 28 Oct 2024 15:35:08 -0700
Robert Joslyn <robert_joslyn@selinc.com> wrote:

> Add support for SEL FPGA based network adapters. The network device is
> implemented as an FPGA IP core and enumerated by the selpvmf driver.
> This is used on multiple devices, including:
>  - SEL-3350 mainboard
>  - SEL-3390E4 card
>  - SEL-3390T card

Make sure that you get the right people as recipients for this
patchset. You would need at least the net maintainers, make sure to use
the scripts/get_maintainers.pl tool to know who to send the patch to.

> 
> Signed-off-by: Robert Joslyn <robert_joslyn@selinc.com>
> ---
>  MAINTAINERS                                  |   1 +
>  drivers/net/ethernet/Kconfig                 |   1 +
>  drivers/net/ethernet/Makefile                |   1 +
>  drivers/net/ethernet/sel/Kconfig             |  31 +
>  drivers/net/ethernet/sel/Makefile            |  22 +
>  drivers/net/ethernet/sel/ethtool.c           | 404 ++++++++
>  drivers/net/ethernet/sel/ethtool.h           |  17 +
>  drivers/net/ethernet/sel/hw_interface.c      | 410 ++++++++
>  drivers/net/ethernet/sel/hw_interface.h      |  46 +
>  drivers/net/ethernet/sel/mac_main.c          | 155 +++
>  drivers/net/ethernet/sel/mdio.c              | 166 ++++
>  drivers/net/ethernet/sel/mdio.h              |  15 +
>  drivers/net/ethernet/sel/mii.c               | 422 +++++++++
>  drivers/net/ethernet/sel/mii.h               |  21 +
>  drivers/net/ethernet/sel/mii_interface.c     | 133 +++
>  drivers/net/ethernet/sel/mii_interface.h     |  23 +
>  drivers/net/ethernet/sel/netdev.c            | 946 +++++++++++++++++++
>  drivers/net/ethernet/sel/netdev.h            |  24 +
>  drivers/net/ethernet/sel/netdev_isr.c        | 245 +++++
>  drivers/net/ethernet/sel/netdev_isr.h        |  20 +
>  drivers/net/ethernet/sel/netdev_rx.c         | 785 +++++++++++++++
>  drivers/net/ethernet/sel/netdev_rx.h         |  17 +
>  drivers/net/ethernet/sel/netdev_tx.c         | 647 +++++++++++++
>  drivers/net/ethernet/sel/netdev_tx.h         |  22 +
>  drivers/net/ethernet/sel/pci_mac.h           | 290 ++++++
>  drivers/net/ethernet/sel/pci_mac_hw_regs.h   | 370 ++++++++
>  drivers/net/ethernet/sel/pci_mac_sysfs.c     | 642 +++++++++++++
>  drivers/net/ethernet/sel/pci_mac_sysfs.h     |  14 +
>  drivers/net/ethernet/sel/sel_pci_mac_ioctl.h |  25 +
>  drivers/net/ethernet/sel/sel_phy.h           |  91 ++
>  drivers/net/ethernet/sel/sel_phy_broadcom.c  | 316 +++++++
>  drivers/net/ethernet/sel/sel_phy_broadcom.h  |  15 +
>  drivers/net/ethernet/sel/sel_phy_marvell.c   | 458 +++++++++
>  drivers/net/ethernet/sel/sel_phy_marvell.h   |  15 +
>  drivers/net/ethernet/sel/sel_phy_ti.c        | 419 ++++++++
>  drivers/net/ethernet/sel/sel_phy_ti.h        |  14 +
>  drivers/net/ethernet/sel/sel_soft_phy.c      |  98 ++
>  drivers/net/ethernet/sel/sel_soft_phy.h      |  15 +
>  drivers/net/ethernet/sel/semaphore.h         |  85 ++
>  drivers/net/ethernet/sel/sfp.c               | 615 ++++++++++++
>  drivers/net/ethernet/sel/sfp.h               |  61 ++

I haven't reviewed the code itself as this is a biiiiig patch, I
suggest you try to split it into more digestable patches, focusing on
individual aspects of the driver.

One thing is the PHY support as you mention in the cover-letter, in the
current state this driver re-implements PHY drivers from what I
understand. You definitely need to use the kernel infra for PHY
handling.

As it seems this driver also re-implements SFP entirely, I suggest you
look into phylink [1]. This will help you supporting the PHYs and SFPs.
You can take a look at the mvneta.c and mvpp2 drivers for examples.

Make sure you handle the mdio bus control using the dedicated framework
(see mii_bus et al.).

I'd be happy to give you more reviews, but this would be a more
manageable task with smaller patches :)

Best regards,

Maxime