Page MenuHomeFreeBSD

dpaa2: add SIOCGI2C support to read SFP+ module EEPROM
Needs ReviewPublic

Authored by yarshure_gmail.com on Wed, Jul 15, 2:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 22, 2:28 PM
Unknown Object (File)
Wed, Jul 22, 1:50 PM
Unknown Object (File)
Sun, Jul 19, 3:15 AM
Unknown Object (File)
Sun, Jul 19, 2:38 AM
Unknown Object (File)
Sat, Jul 18, 12:40 AM
Unknown Object (File)
Thu, Jul 16, 6:02 AM
Unknown Object (File)
Thu, Jul 16, 6:02 AM
Subscribers
None

Details

Reviewers
dsl
jhibbits
bz
Summary

Implement the SIOCGI2C interface ioctl on dpaa2 network interfaces so
"ifconfig -v dpniX" can report an attached SFP+ module's SFF-8472
identity (vendor, part/serial number, date) and diagnostics
(temperature, voltage, TX bias, RX/TX optical power). ifconfig(8)
already decodes the raw EEPROM bytes; only the kernel side to fetch them
over i2c was missing.

The module EEPROM (i2c addresses 0xA0/0xA2) is reached over the i2c bus
the DPMAC's SFP cage sits on, resolved in one of two ways:

  • FDT: the DPMAC node's "sfp" phandle references an "sff,sfp" node whose "i2c-bus" property names the (possibly muxed) i2c bus. A new dpaa2_mc bus method, get_sfp_dev(), resolves a DPMAC id to that i2c bus device; any i2c mux is switched transparently by the i2c framework on request.
  • ACPI: firmware provides no such association, so it is supplied per interface by loader tunables and the i2c mux (if any) is switched explicitly (hw.dpaa2.dpni<unit>.sfp_bus / sfp_mux / sfp_chan / sfp_type).

The read holds the i2c bus across the optional mux channel select, the
SFF-8472 offset write (repeat-start) and the data read. Note iicbus
slave addresses are 8-bit left-aligned, so the ifi2creq dev_addr
(0xA0/0xA2) is used directly.

Developed with AI assistance; the design and SFF-8472 byte offsets were
reviewed by hand against the spec and an independent user-space decoder.

MFC after: 2 weeks

Test Plan

Hardware-tested on a SolidRun HoneyComb (LX2160A) under ACPI, where two
SFP+ cages sit behind a PCA9547 mux at 0x77 on iic0. With the tunables
set (dpni1: sfp_bus=0/sfp_mux=0x77/sfp_chan=5, dpni2: chan=6),
"ifconfig -v dpni1"/"dpni2" print, e.g.:

plugged: SFP/SFP+/SFP28 10G Base-LR (LC)
vendor: HG GENUINE PN: MTRS-1E31-01 SN: MA20261090751 ...
module temperature: 34.3 C voltage: 3.31 Volts
lane 1: RX power: 0.47 mW (-3.27 dBm) TX bias: 30.4 mA

Values match an independent user-space SFF-8472 decoder reading the same
EEPROM bytes; each dpni reads its own cage (no cross-talk); the mux is
restored after each read; reads run fine while the link carries traffic;
an interface with no association returns ENXIO (no panic).

The FDT resolution path (DPAA2_MC_GET_SFP_DEV via the "sfp" phandle) is
compile-tested only -- no DT-booted DPAA2 board was available -- and
would benefit from a check on hardware that provides the "sff,sfp"
association.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

yarshure_gmail.com created this revision.

Okay, thanks for opening a review. Please, provide a full context for the patch, i.e. -U999999.

Re-uploaded with full context (git diff -U999999) as requested. Thanks for taking a look.

sys/dev/dpaa2/dpaa2_ni.c
2731

There's a switch-case below in the function. Please, add another case there and open/close RC and NI on demand keeping return statement at the end of the function. Btw, opening and closing those DPAA2 objects is a common set of operations which can be moved to separate helper routines as well.

I can read a DAC on my Ten64 now. Looks good!

dpni8: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=60002b<RXCSUM,TXCSUM,VLAN_MTU,JUMBO_MTU,RXCSUM_IPV6,TXCSUM_IPV6>
        ether 00:0a:fa:24:2b:28
ifconfig: dpni8: no media types?
        nd6 options=829<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL,STABLEADDR>
        drivername: dpni8
        plugged: SFP/SFP+/SFP28 Unknown (Copper pigtail)
        vendor: FS PN: SFPP-PC03 SN: F2210XXXXX-2 DATE: 2022-08-12
root@cannon-tower:~ # ifconfig -v dpni9
dpni9: flags=8802<BROADCAST,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=60002b<RXCSUM,TXCSUM,VLAN_MTU,JUMBO_MTU,RXCSUM_IPV6,TXCSUM_IPV6>
        ether 00:0a:fa:24:2b:27
ifconfig: dpni9: no media types?
        nd6 options=829<PERFORMNUD,IFDISABLED,AUTO_LINKLOCAL,STABLEADDR>
        drivername: dpni9
        plugged: SFP/SFP+/SFP28 Unknown (Copper pigtail)
        vendor: FS PN: SFPP-PC03 SN: F2210XXXXX-1 DATE: 2022-08-12

My Honeycomb doesn't have SFP cages exposes to the operating system, i.e. I cannot try it there directly. Besides, I don't like an idea to obtain info about the I2C config for SFPs using tunables on the ACPI systems. It is already described in the ACPI tables (partially, though):

dsl@castle:~ $ devinfo -v | grep I2C0
    vf_i2c_acpi0 <Vybrid Family Inter-Integrated Circuit (I2C)> pnpinfo _HID=NXP0001 _UID=0 _CID=none at handle=\_SB_.I2C0
        unknown pnpinfo _HID=NXP0002 _UID=0 _CID=none at addr=0x77 handle=\_SB_.I2C0.MUX0
    unknown pnpinfo _HID=PRP0001 _UID=0 _CID=none at handle=\_SB_.I2C0.MUX0.CH01.FAN1
    unknown pnpinfo _HID=PRP0001 _UID=1 _CID=none at handle=\_SB_.I2C0.MUX0.CH03.THE1

Only a fan controller (TI's amc6821) and a temperature sensor (NXP's sa56004) sitting on channels 1 and 3 respectively are described. However, according to the Honeycomb schematic, mux channels 4,5,6,7 are for the SFP cages. You can check whether a board you're booting up is LX2160A and set correct default values.

UPD: See https://github.com/SolidRun/edk2-platforms/blob/edk2-stable202102-lx2160acex7/Silicon/NXP/LX2160A/AcpiTables/Dsdt-Cex7/I2c.asl#L63

Several years ago I added a draft version of the "sff,sfp" driver to https://cgit.freebsd.org/src/tree/sys/dev/sff. I wonder whether all of those sfp_i2c operations can be moved there and be hidden behind an API. dpaa2_ni would only need to obtain "sff,sfp" device (via xref, I think) and call it.

Thanks a lot for testing it on the Ten64 and for the detailed feedback -- good to see it reading real modules on hardware I couldn't try myself (my board is ACPI-only, so this is the first confirmation of the FDT/OF path on a live system).

Agreed on all three points; here's how I'd like to proceed, with one question on the ACPI side.

1) ioctl structure. I'll rework dpaa2_ni_ioctl() to handle SIOCGI2C as a regular case and open/close the RC and NI objects on demand within the cases that actually need them, keeping a single return at the end. I'll also factor the RC/NI open+close into helper routines as you suggest -- that cleans up the other cases too.

2) sys/dev/sff. Happy to move the SFP EEPROM access there and keep dpaa2_ni thin. My plan would be to extend sff_if with a read method (e.g. SFF_READ_EEPROM(dev, dev_addr, offset, buf, len)) implemented once in a shared sff.c on top of get_i2c_bus(), so dpaa2_ni only obtains the sff,sfp device (via the dpmac's sfp xref, exactly as you outlined) and calls it -- it wouldn't touch iicbus at all. Does that API shape work for you, or would you rather keep the interface at get_i2c_bus() and have callers do the transfer? On the FDT side sfp_fdt already registers the xref, so obtaining the device from the dpmac node is straightforward.

3) ACPI / tunables. I agree the tunables are unpleasant. I went through the CEX7 DSDT you linked: the mux (NXP0002, 0x77) is described, but only CH01 (amc6821 fan) and CH03 (sa56004 temp) have children -- channels 4-7 (the SFP cages, per the schematic) and the 0x50/0x51 pages aren't in the DSDT at all. So on ACPI the firmware doesn't describe the SFP i2c path, and something has to supply both "the mux is at 0x77 on I2C0" and "dpni N is on channel M" regardless of mechanism.

My preference, matching your suggestion, is to drop the raw per-interface tunables in favour of board detection: recognise the LX2160A SolidRun boards and provide the default channel mapping in the kernel (sourcing the mux from the ACPI NXP0002 device where possible rather than hardcoding 0x77), keeping a tunable only as a last-resort override for boards we don't yet know. Would that be acceptable, or would you rather NXP0002 get a proper ACPI i2c-mux driver first (channel iicbuses enumerated) and treat the SFP-channel mapping separately? Even with the mux driver the DSDT wouldn't say which channel a given dpmac uses, so some board knowledge seems unavoidable either way -- I'd like your steer before I restructure the ACPI path.

I'll bundle the sff rework and the ioctl cleanup into a v2 once we settle the ACPI approach. Thanks again!

Thanks for the review and for testing the FDT/OF path on your Ten64 — glad the DAC reads there.

v2 addresses the two structural points:

(1) dpaa2_ni_ioctl restructured. SIOCGI2C is now a plain case that never touches
the MC; the RC/NI command tokens are opened on demand (dpaa2_ni_cmd_open/_close)
only in SIOCSIFMTU, which is the sole path that needs them, and the function has a
single return. No more open-everything-up-front / goto scaffolding.

(2) The SFP I2C operations moved out of dpaa2_ni into sys/dev/sff, behind an API,
as you suggested. New sff.c holds the bus-held mux-select + offset-write(NOSTOP) +
read + mux-restore core as sff_read_eeprom(), exposed both as a plain function and
as an SFF_READ_EEPROM method on the sff_if interface (implemented by sfp_fdt.c).
dpaa2_ni no longer includes iicbus at all: on the FDT path it obtains the 'sff,sfp'
device via xref (DPAA2_MC_GET_SFF_DEV, renamed from get_sfp_dev) and calls
SFF_READ_EEPROM; MODULE_DEPEND is now on sff, not iicbus.

Remaining open point — ACPI association. On this SolidRun CEX7/LX2160A the firmware
does not self-describe the SFP i2c: the mux (NXP0002 @0x77) is in ACPI but the SFP
channels and the 0x50/0x51 EEPROMs are not declared, so there is no ACPI object to
walk. v2 keeps the loader tunables (hw.dpaa2.dpni<N>.sfp_{bus,mux,chan,type}) as an
interim mechanism, but it is isolated to the ACPI fallback only — the FDT path is
fully self-describing via the phandle. Happy to rework this to board detection or a
small ACPI iicmux/description shim per your preference; wanted to get the structural
changes up first.

Built clean and hardware-tested on the LX2160A (ACPI boot): both SFPs decode via the
moved sff_read_eeprom(), 5x stable repeated reads, SIOCSIFMTU set/get OK, mux left at
its boot value, no panic. Test log attached in the summary.