Re: [net-next v5 0/6] net: marvell: prestera: Add Switchdev driver for Prestera family ASIC device 98DX326x (AC3x)

[Chris Packham]

Hi Vadym,

On 26/08/20 12:20 am, Vadym Kochan wrote:
> Marvell Prestera 98DX326x integrates up to 24 ports of 1GbE with 8
> ports of 10GbE uplinks or 2 ports of 40Gbps stacking for a largely
> wireless SMB deployment.
I think there's a typo here or possibly in patch 1. The AC3x family has 
model numbers 98DX3255, 98DX3256, 98DX3257, 98DX3258, 98DX3259, 
98DX3265, 98DX3268. I'm not sure which variant you're specifically 
targeting but in patch 1 you add the PCI device 0xC804 which corresponds 
to the 98DX3255.
> Prestera Switchdev is a firmware based driver that operates via PCI bus.  The
> current implementation supports only boards designed for the Marvell Switchdev
> solution and requires special firmware.
>
> This driver implementation includes only L1, basic L2 support, and RX/TX.
>
> The core Prestera switching logic is implemented in prestera_main.c, there is
> an intermediate hw layer between core logic and firmware. It is
> implemented in prestera_hw.c, the purpose of it is to encapsulate hw
> related logic, in future there is a plan to support more devices with
> different HW related configurations.
>
> The following Switchdev features are supported:
>
>      - VLAN-aware bridge offloading
>      - VLAN-unaware bridge offloading
>      - FDB offloading (learning, ageing)
>      - Switchport configuration
>
> The original firmware image is uploaded to the linux-firmware repository.
>
> PATCH v5:
>      0) add Co-developed tags for people who was involved in development.
>
>      1) Make SPDX license as separate comment
>
>      2) Change 'u8 *' -> 'void *', It allows to avoid not-needed u8* casting.
>
>      3) Remove "," in terminated enum's.
>
>      4) Use GENMASK(end, start) where it is applicable in.
>
>      5) Remove not-needed 'u8 *' casting.
>
>      6) Apply common error-check pattern
>
>      7) Use ether_addr_copy instead of memcpy
>
>      8) Use define for maximum MAC address range (255)
>
>      9) Simplify prestera_port_state_set() in prestera_main.c by
>         using separate if-blocks for state setting:
>      
>          if (is_up) {
>          ...
>          } else {
>          ...
>          }
>
>        which makes logic more understandable.
>
>      10) Simplify sdma tx wait logic when checking/updating tx_ring->burst.
>
>      11) Remove not-needed packed & aligned attributes
>
>      12) Use USEC_PER_MSEC as multiplier when converting ms -> usec on calling
>          readl_poll_timeout.
>
>      13) Simplified some error path handling by simple return error code in.
>
>      14) Remove not-needed err assignment in.
>
>      15) Use dev_err() in prestera_devlink_register(...).
>
>      Patches updated:
>          [1] net: marvell: prestera: Add driver for Prestera family ASIC devices
> 	[2] net: marvell: prestera: Add PCI interface support
>          [3] net: marvell: prestera: Add basic devlink support
> 	[4] net: marvell: prestera: Add ethtool interface support
> 	[5] net: marvell: prestera: Add Switchdev driver implementation
>
> PATCH v4:
>      1) Use prestera_ prefix in netdev_ops variable.
>
>      2) Kconfig: use 'default PRESTERA' build type for CONFIG_PRESTERA_PCI to be
>         synced by default with prestera core module.
>
>      3) Use memcpy_xxio helpers in prestera_pci.c for IO buffer copying.
>
>      4) Generate fw image path via snprintf() instead of macroses.
>
>      5) Use pcim_ helpers in prestera_pci.c which simplified the
>         probe/remove logic.
>
>      6) Removed not needed initializations of variables which are used in
>         readl_poll_xxx() helpers.
>
>      7) Fixed few grammar mistakes in patch[2] description.
>
>      8) Export only prestera_ethtool_ops struct instead of each
>         ethtool handler.
>
>      9) Add check for prestera_dev_check() in switchdev event handling to
>         make sure there is no wrong topology.
>
>      Patches updated:
>          [1] net: marvell: prestera: Add driver for Prestera family ASIC devices
> 	[2] net: marvell: prestera: Add PCI interface support
> 	[4] net: marvell: prestera: Add ethtool interface support
> 	[5] net: marvell: prestera: Add Switchdev driver implementation
>
> PATCH v3:
>      1) Simplify __be32 type casting in prestera_dsa.c
>
>      2) Added per-patch changelog under "---" line.
>
> PATCH v2:
>      1) Use devlink_port_type_clear()
>
>      2) Add _MS prefix to timeout defines.
>
>      3) Remove not-needed packed attribute from the firmware ipc structs,
>         also the firmware image needs to be uploaded too (will do it soon).
>
>      4) Introduce prestera_hw_switch_fini(), to be mirrored with init and
>         do simple validation if the event handlers are unregistered.
>
>      5) Use kfree_rcu() for event handler unregistering.
>
>      6) Get rid of rcu-list usage when dealing with ports, not needed for
>         now.
>
>      7) Little spelling corrections in the error/info messages.
>
>      8) Make pci probe & remove logic mirrored.
>
>      9) Get rid of ETH_FCS_LEN in headroom setting, not needed.
>
> PATCH:
>      1) Fixed W=1 warnings
>
>      2) Renamed PCI driver name to be more generic "Prestera DX" because
>         there will be more devices supported.
>
>      3) Changed firmware image dir path: marvell/ -> mrvl/prestera/
>         to be aligned with location in linux-firmware.git (if such
>         will be accepted).
>
> RFC v3:
>      1) Fix prestera prefix in prestera_rxtx.c
>
>      2) Protect concurrent access from multiple ports on multiple CPU system
>         on tx path by spinlock in prestera_rxtx.c
>
>      3) Try to get base mac address from device-tree, otherwise use a random generated one.
>
>      4) Move ethtool interface support into separate prestera_ethtool.c file.
>
>      5) Add basic devlink support and get rid of physical port naming ops.
>
>      6) Add STP support in Switchdev driver.
>
>      7) Removed MODULE_AUTHOR
>
>      8) Renamed prestera.c -> prestera_main.c, and kernel module to
>         prestera.ko
>
> RFC v2:
>      1) Use "pestera_" prefix in struct's and functions instead of mvsw_pr_
>
>      2) Original series split into additional patches for Switchdev ethtool support.
>
>      3) Use major and minor firmware version numbers in the firmware image filename.
>
>      4) Removed not needed prints.
>
>      5) Use iopoll API for waiting on register's value in prestera_pci.c
>
>      6) Use standart approach for describing PCI ID matching section instead of using
>         custom wrappers in prestera_pci.c
>
>      7) Add RX/TX support in prestera_rxtx.c.
>
>      8) Rewritten prestera_switchdev.c with following changes:
>         - handle netdev events from prestera.c
>
>         - use struct prestera_bridge for bridge objects, and get rid of
>           struct prestera_bridge_device which may confuse.
>
>         - use refcount_t
>
>      9) Get rid of macro usage for sending fw requests in prestera_hw.c
>
>      10) Add base_mac setting as module parameter. base_mac is required for
>          generation default port's mac.
>
> Vadym Kochan (6):
>    net: marvell: prestera: Add driver for Prestera family ASIC devices
>    net: marvell: prestera: Add PCI interface support
>    net: marvell: prestera: Add basic devlink support
>    net: marvell: prestera: Add ethtool interface support
>    net: marvell: prestera: Add Switchdev driver implementation
>    dt-bindings: marvell,prestera: Add description for device-tree
>      bindings
>
>   .../bindings/net/marvell,prestera.txt         |   34 +
>   drivers/net/ethernet/marvell/Kconfig          |    1 +
>   drivers/net/ethernet/marvell/Makefile         |    1 +
>   drivers/net/ethernet/marvell/prestera/Kconfig |   25 +
>   .../net/ethernet/marvell/prestera/Makefile    |    7 +
>   .../net/ethernet/marvell/prestera/prestera.h  |  208 +++
>   .../marvell/prestera/prestera_devlink.c       |  114 ++
>   .../marvell/prestera/prestera_devlink.h       |   26 +
>   .../ethernet/marvell/prestera/prestera_dsa.c  |  106 ++
>   .../ethernet/marvell/prestera/prestera_dsa.h  |   37 +
>   .../marvell/prestera/prestera_ethtool.c       |  759 ++++++++++
>   .../marvell/prestera/prestera_ethtool.h       |   14 +
>   .../ethernet/marvell/prestera/prestera_hw.c   | 1227 ++++++++++++++++
>   .../ethernet/marvell/prestera/prestera_hw.h   |  185 +++
>   .../ethernet/marvell/prestera/prestera_main.c |  646 +++++++++
>   .../ethernet/marvell/prestera/prestera_pci.c  |  778 ++++++++++
>   .../ethernet/marvell/prestera/prestera_rxtx.c |  866 +++++++++++
>   .../ethernet/marvell/prestera/prestera_rxtx.h |   21 +
>   .../marvell/prestera/prestera_switchdev.c     | 1289 +++++++++++++++++
>   .../marvell/prestera/prestera_switchdev.h     |   16 +
>   20 files changed, 6360 insertions(+)
>   create mode 100644 drivers/net/ethernet/marvell/prestera/Kconfig
>   create mode 100644 drivers/net/ethernet/marvell/prestera/Makefile
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_devlink.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_devlink.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_dsa.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_dsa.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_ethtool.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_ethtool.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_main.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_pci.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_rxtx.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_rxtx.h
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
>   create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_switchdev.h
>