Re: [PATCH net-next v6] Add Mellanox BlueField Gigabit Ethernet driver

On Tue, Jun 01, 2021 at 08:24:55AM -0400, David Thompson wrote:

This patch adds build and driver logic for the "mlxbf_gige"
Ethernet driver from Mellanox Technologies. The second
generation BlueField SoC from Mellanox supports an
out-of-band GigaBit Ethernet management port to the Arm
subsystem.  This driver supports TCP/IP network connectivity
for that port, and provides back-end routines to handle
basic ethtool requests.

The driver interfaces to the Gigabit Ethernet block of
BlueField SoC via MMIO accesses to registers, which contain
control information or pointers describing transmit and
receive resources.  There is a single transmit queue, and
the port supports transmit ring sizes of 4 to 256 entries.
There is a single receive queue, and the port supports
receive ring sizes of 32 to 32K entries. The transmit and
receive rings are allocated from DMA coherent memory. There
is a 16-bit producer and consumer index per ring to denote
software ownership and hardware ownership, respectively.

The main driver logic such as probe(), remove(), and netdev
ops are in "mlxbf_gige_main.c".  Logic in "mlxbf_gige_rx.c"
and "mlxbf_gige_tx.c" handles the packet processing for
receive and transmit respectively.

The logic in "mlxbf_gige_ethtool.c" supports the handling
of some basic ethtool requests: get driver info, get ring
parameters, get registers, and get statistics.

The logic in "mlxbf_gige_mdio.c" is the driver controlling
the Mellanox BlueField hardware that interacts with a PHY
device via MDIO/MDC pins.  This driver does the following:
  - At driver probe time, it configures several BlueField MDIO
    parameters such as sample rate, full drive, voltage and MDC
  - It defines functions to read and write MDIO registers and
    registers the MDIO bus.
  - It defines the phy interrupt handler reporting a
    link up/down status change
  - This driver's probe is invoked from the main driver logic
    while the phy interrupt handler is registered in ndo_open.

Driver limitations
  - Only supports 1Gbps speed
  - Only supports GMII protocol
  - Supports maximum packet size of 2KB
  - Does not support scatter-gather buffering

Testing
  - Successful build of kernel for ARM64, ARM32, X86_64
  - Tested ARM64 build on FastModels & Palladium
  - Tested ARM64 build on several Mellanox boards that are built with
    the BlueField-2 SoC.  The testing includes coverage in the areas
    of networking (e.g. ping, iperf, ifconfig, route), file transfers
    (e.g. SCP), and various ethtool options relevant to this driver.

v5 -> v6

Please put changelog under "---" below your SOBs. We don't need to see
this history in the git log.

  Fixed use of COMPILE_TEST for ARM32 build; changed driver to not
  depend on CONFIG_ACPI for ARM32 build
v4 -> v5
  Created a separate interrupt controller for the GPIO PHY interrupt
  and as a result, the GIGE driver no longer depends on GPIO driver
  Updated the logic in mlxbf_gige_adjust_link() to store the negotiated
  pause settings into the driver's private settings.
  Modified logic to only change enable bit in RX_DMA register
  Changed logic to only map and unmap the actual length of the TX SKB,
  instead using the default size.
  Added better error handling to open() method
  Modified receive packet logic to use polarity bit to signify ownership
  (software vs. hardware) of the RX CQE slot
v3 -> v4
  Main driver module broken out into rx, tx, intr, and ethtool modules
  Removed some GPIO PHY interrupt logic; moved to GPIO_MLXBF2 driver
v2 -> v3
  Added logic to handle PHY link up/down interrupts
  Use streaming DMA mapping for packet buffers
  Changed logic to use standard iopoll methods
  Changed PHY logic to not allow C45 transactions
  Enhanced the error handling in open() method
  Enhanced start_xmit() method to use xmit_more mechanism
  Added support for ndo_get_stats64
  Removed standard stats from "ethtool -S" output
v1 -> v2:
  Fixed all warnings raised by "make C=1" and "make W=1"
    a) Changed logic in mlxbf_gige_rx_deinit() and mlxbf_gige_tx_deinit()
       to initialize relevant pointers as NULL, not 0
    b) Change mlxbf_gige_get_mac_rx_filter() to return void,
       as this function's return status is not used by caller
    c) Fixed type definition of "buff" in mlxbf_gige_get_regs()

Signed-off-by: David Thompson <davthompson@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
Reviewed-by: Liming Sun <redacted>
---

The patch generates checkpatch warnings.

 CHECK: spinlock_t definition without comment
 #272: FILE: drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h:87:
 +	spinlock_t lock;

 CHECK: spinlock_t definition without comment
 #273: FILE: drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h:88:
 +	spinlock_t gpio_lock;

 CHECK: Macro argument 'tx_wqe_addr' may be better as '(tx_wqe_addr)' to avoid precedence issues
 #328: FILE: drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h:143:
 +#define MLXBF_GIGE_TX_WQE_PKT_LEN(tx_wqe_addr) \
 +	(*(tx_wqe_addr + 1) & MLXBF_GIGE_TX_WQE_PKT_LEN_MASK)

 CHECK: multiple assignments should be avoided
 #1271: FILE: drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c:374:
 +	phydev->irq = priv->mdiobus->irq[addr] = priv->phy_irq;

Thanks