[RFC] ptp: ocp: exposing the TimeCard X2 AXI Ethernet port - which approach?

Hi,

Before posting patches I would like to check which of two approaches you would
prefer, since they differ a lot in how much code lands and where.

The ADVA TimeCard X2 is a PCIe card whose FPGA image contains a Xilinx AXI
Ethernet MAC and an AXI DMA engine alongside the timing blocks ptp_ocp already
supports. We want to expose that Ethernet port as a netdevice.

Approach A - let xilinx_axienet drive it.

ptp_ocp creates a platform device describing the two register windows and the
three MSI-X vectors, and xilinx_axienet binds to it. This is what ptp_ocp
already does for the Xilinx SPI and I2C cores on the same card, see
ptp_ocp_i2c_bus() and ptp_ocp_register_spi().

There is no device tree, so a software node stands in for one: "phy-mode"
replaces a phy-handle, since the MAC is wired to the fabric internally with no
MDIO and no PHY, and a "fixed-link" child node pins the link at 1 Gbit/s full
duplex.

This needs two things from xilinx_axienet:

  1. Read its configuration through the generic device property API instead of
     the OF-specific one, so it can be described by a software node. On a
     device-tree system dev_fwnode() resolves to the OF fwnode and the reads
     land in the same OF code as today, so this is a no-op for existing users.
     It is also in line with the wider move from of_* to
     device_property_*/fwnode_*.

  2. A dma_dev in struct axienet_local. Descriptors and buffers must be mapped
     against the device that masters the bus, which for a platform device
     created by a PCIe parent is that parent - a platform device does not
     inherit its parent's DMA ops or IOMMU domain. For every existing user
     dma_dev would equal dev, so no behavioural change.

That comes to about 50 changed lines in xilinx_axienet and about 180 new lines
in ptp_ocp.

Approach B - a separate driver under drivers/ptp.

Reimplement the descriptor rings, NAPI and ethtool support against the same IP,
roughly 2500 lines, and share xilinx_axienet's register definitions out of
drivers/net/ethernet/xilinx/.

We have both working. Approach A is tested on 7.0.0-rc1 with the card:

  xilinx_axienet xilinx_axienet.512 eth0: configuring for
fixed/internal link mode
  xilinx_axienet xilinx_axienet.512 eth0: Link is Up - 1Gbps/Full -
flow control off

ping at 0% loss, a 90 s bulk TCP transfer moving ~9000 packets each way with
zero errors and no TX timeouts, ethtool -S/-g/-c/-a all working, and clean
module unload and reload.

We would much rather do A than maintain a duplicate of an existing driver, so
the questions are:

  1. Is instantiating xilinx_axienet from a PCIe parent as a platform device
     acceptable, or would you prefer the driver split so the device can be
     created on the auxiliary bus? The platform device route keeps the change
     to xilinx_axienet small and matches what ptp_ocp already does for the SPI
     and I2C cores, but auxiliary bus is the more usual choice for sub-devices
     of a PCIe function and would make the DMA parent explicit.

  2. For the dma_dev, we currently infer it by testing whether the parent is a
     PCI device. That is concise but implicit; we are happy to pass it in
     explicitly if you would prefer it were not inferred.

I can post the series straight away if you would rather look at the code.

Thanks,
Sagi Maimon