dpaa2: read SFP+ module EEPROM through SIOCGI2C

[yarshure_gmail.com]

Summary

So that "ifconfig -v <dpni>" can show a transceiver's SFF-8472 identity and
diagnostics: vendor, part and serial number, and the module's temperature,
voltage and optical power.

The transceiver is a device in its own right. Firmware names it in the
DPMAC's node -- an "sfp" phandle under FDT, an "sfp" _DSD reference under
ACPI -- and sff(4) drives it, so all this handler does is ask the MC bus for
that device and read from it. It learns neither how the module is wired up
nor which flavour of firmware described it, and there is nothing for it to
configure.

Answer the ioctl before opening any MC object: this talks to an i2c bus, not
to the MC.

Tested on a SolidRun CEX7 (NXP LX2160A) booted through UEFI/ACPI, with the
two 10G SFP+ cages behind the board's PCA9547 mux.

Depends on D60068

Test Plan

arm64 GENERIC, and a variant with dpaa2/sff/pca954x/iicmux as modules.
Module loadability checked apart from the static build: every .ko's undefined
symbols resolve, and all 2334 MODULE_DEPEND records in GENERIC's modules are
satisfiable -- including MODULE_DEPEND(dpaa2_ni, sff).

Hardware: SolidRun CEX7 (NXP LX2160A) under UEFI/ACPI, two 10G SFP+ cages
behind the board's PCA9547 at 0x77 on I2C0. No tunables -- the transceiver is
resolved from firmware (see the preceding commits; this board's ACPI tables
had to be extended to describe the cages at all, which is documented there).

1. ifconfig -v dpni1 plugged: SFP/SFP+/SFP28 10G Base-LR (LC) vendor: HG GENUINE PN: MTRS-1E31-01 SN: MA20261090751 DATE: 2020-06-28 module temperature: 34.43 C voltage: 3.31 Volts
2. ifconfig -v dpni2 plugged: SFP/SFP+/SFP28 10G Base-LR (LC) vendor: HG GENUINE PN: MTRS-1E31-01 SN: MA19370742663 DATE: 2019-09-12

Each cage reads its own module: 150 concurrent "ifconfig -v" per interface
against the same mux, with zero cross-talk -- no read ever returned the other
cage's serial number.

Under that load a transient read failure appears at roughly 1e-4 per i2c
transaction. It is below this layer: driving /dev/iic6 and /dev/iic7 directly
with i2c(8), bypassing sff(4) and dpaa2 entirely, gives the same rate (2
failures in 20000 transactions), so it is the i2c stack or the controller, not
the transceiver path.

Values match an independent user-space SFF-8472 decoder reading the same
EEPROM bytes. dsl@ tested an earlier form of the FDT path on a Ten64.

[dsl]

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

[yarshure_gmail.com]

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

[dsl]

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.

[dsl]

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

[dsl]

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

[dsl]

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.

[yarshure_gmail.com]

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!

[yarshure_gmail.com]

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.

[yarshure_gmail.com]

@dsl — reviving this; v2 (D182262) has been up since Jul 20 and I think I now have a
proper answer to the one point we left open, rather than the tunables.

You objected to hw.dpaa2.dpni<N>.sfp_{bus,mux,chan,type} and suggested board detection
with defaults derived from ACPI. I went and looked at what the firmware actually
describes, and there is a third option that is strictly better than either: describe the
SFP in the DSDT, using the exact same idiom the DSDT already uses for PHYs.

What the CEX7 firmware has today

edk2-platforms, Silicon/NXP/LX2160A/AcpiTables/Dsdt-Cex7/:

I2c.asl  declares I2C0 (NXP0001) and the PCA9547 mux MUX0 (NXP0002 @ 0x77), but only
         channels CH01 (AMC6821 fan @0x18) and CH03 (SA56004 thermal @0x4A). The SFP+
         channels and their 0x50/0x51 EEPROMs are absent. This is deliberate, not an
         oversight — the comment at the top of the file lists "zQSFP+ Cage, SFP+ Cage"
         among the devices left out, with "Rest Devices on Mux1 are for debug purpose.
         These could be added in case of *debug only*".

Mc.asl   already exposes each DPMAC as its own ACPI device (\_SB.MCE0.PR07..PR10 for
         the 10G MACs, PR17 for the 1G RGMII) with DT-style _DSD properties, and PR17
         carries a reference property:

             Package () {"phy-handle", Package (){\_SB.MDI0.PHY1}}

So the ACPI reference-property mechanism is already established on this platform — and
we already consume it: dpaa2_mc_acpi.c:118 reads "phy-handle" with DEVICE_PROP_HANDLE,
the same generic newbus call dpaa2_mc_fdt.c:142 uses to read "sfp". The only thing
missing is that nobody ever described the SFP side.

Proposed shape

Firmware (one ASL patch, follows the existing CH01/CH03 pattern):

Device (CH05) {
  Name(_ADR, 5)
  Name(_UID, 5)
  Device(SFP0) {
    Name(_HID, "PRP0001")
    Name(_DSD, Package () {
      ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
      Package() {
        Package() {"compatible", "sff,sfp"},
        Package() {"i2c-bus", Package(){\_SB.I2C0.MUX0.CH05}},
      }
    })
  }
}
/* CH06 / SFP1 likewise */

and on the consuming DPMAC, exactly parallel to phy-handle:

Scope(\_SB.MCE0.PR07) { ... Package () {"sfp", Package(){\_SB.I2C0.MUX0.CH05.SFP0}} }

Kernel: dpaa2_mc_acpi.c gains the same three-line "sfp" DEVICE_PROP_HANDLE lookup the
FDT front-end already has, and sys/dev/sff gains an sfp_acpi.c sibling to sfp_fdt.c
implementing SFF_READ_EEPROM. Same property name, same API, on both buses. The tunables
go away entirely.

Is the firmware side realistic? Yes — more so than when I wrote v2. SolidRun discontinued
their EDK2 project, but the stack has since been forward-ported to current upstream by
Liz Fong-Jones (ATF 2.12, edk2-stable202608, OP-TEE 4.9, Secure Boot working), with a
release as recent as 2026-09-09:
https://gist.github.com/lizthegrey/9344dc71dc4ac11f9d6c79d7c143e535
That is a live tree to land the ASL in, and the change is small enough to be worth
proposing to edk2-platforms upstream too.

Questions for you

1. Is that the shape you want? If so I'll do it, and the ACPI fallback in this review loses its tunables completely.
2. How do you want it staged? Either (a) drop the tunables from this review now and let the ACPI path land once the firmware describes it — leaving D58258 as the clean, self-describing FDT path you already tested on your Ten64 — or (b) hold this review until the ACPI side is implemented and tested and land both together. I lean (a), since the FDT half is done and reviewed and there's no reason for it to wait on firmware I don't control.

Either way the FDT path is unchanged from v2.

[adrian]

this looks fine; please just remove teh BSD copyright text itself as the SPDX + your copyright name/email is enough. Then we should be fine for landing it!

[adrian]

sys/dev/dpaa2/dpaa2_mc_if.m

171 ↗

(On Diff #186422)

is this enough for doxygen? eg I normally do @param dev blah blah blah ; @returns blah if blah; else blah

[dsl]

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

[adrian]

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

[yarshure_gmail.com]

v3 addresses @adrian's two points:

(1) Licence blocks. The two new files (sys/dev/sff/sff.c, sys/dev/sff/sff.h) now
carry only the SPDX tag and the copyright line; the BSD-2-Clause body text is
gone. No other file's header is touched.

(2) Doxygen. DPAA2_MC_GET_SFF_DEV() in sys/dev/dpaa2/dpaa2_mc_if.m now documents
its arguments with @param and its result with @returns, including the ENXIO case
(no "sfp" phandle, or a non-FDT system), instead of the hand-rolled list it had.

Comment-only changes: 59 lines across three files, no functional difference from
v2. Rebased onto current main (3a4e03798) and rebuilt; GENERIC arm64 builds clean
with -Werror, and dpaa2_mc_if.h still generates correctly from the .m file.

Re-tested on the LX2160A after the rebase, this time on a -CURRENT kernel
(16.0-CURRENT, main-b7541acacb66) rather than 15.1:

dpni1: plugged: SFP/SFP+/SFP28 10G Base-LR (LC)
       vendor: HG GENUINE PN: MTRS-1E31-01 SN: MA20261090751 DATE: 2020-06-28
       module temperature: 34.11 C voltage: 3.31 Volts
dpni2: plugged: SFP/SFP+/SFP28 10G Base-LR (LC)
       vendor: HG GENUINE PN: MTRS-1E31-01 SN: MA19370742663 DATE: 2019-09-12
       module temperature: 32.28 C voltage: 3.30 Volts

[yarshure_gmail.com]

@dsl - on maclink: I want to make sure I am answering the right objection, because
I cannot find maclink in the tree (only enum dpaa2_mac_link_type in dpaa2_mac.h),
so I am guessing at its shape. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

What I would like to establish is whether this change is actually in the way of
that, because I do not believe it is - it sits underneath it.

What this review adds is one primitive and one consumer:

• sff_read_eeprom() in sys/dev/sff - grab the i2c bus, select the mux channel, write the offset with NOSTOP, read, restore the mux. That is the only new mechanism, and it lives in sys/dev/sff because you asked for it to (D58258, 2026-07-16: "I wonder whether all of those sfp_i2c operations can be moved there and be hidden behind an API"). dpaa2_ni no longer includes iicbus at all; MODULE_DEPEND is on sff.
• SIOCGI2C in dpaa2_ni, which reads pages 0xA0/0xA2 so that ifconfig -v can print the module. It does not touch the MC, does not change link state, and does not make dpaa2_ni the owner of the SFP.

A maclink layer would need exactly that primitive to do its job - to read the
module's identity and rate before deciding how to bring the MAC up. So the natural
end state is that maclink becomes a second consumer of sff_read_eeprom(), and the
question is only who resolves the sff device for a given DPMAC. Today that is
DPAA2_MC_GET_SFF_DEV(), three lines reading the "sfp" phandle in the DPMAC node.
If maclink should own that association, that lookup moves behind it and neither
sff's API nor the ioctl changes.

So, concretely, what I would like from you:

1. What does maclink own - the sfp_bus lifecycle (presence, LOS, TX_DISABLE, rate selection), or also the EEPROM access itself?
2. Do you want DPAA2_MC_GET_SFF_DEV() to go away in favour of maclink resolving the transceiver, and if so should I hold this review until maclink exists, or land the sff primitive now and re-point the lookup when it does?
3. Is there anything in the current diff that you would have written differently regardless of maclink? I would rather fix that now than argue about layering.

For what it's worth I am happy to reshape this however you want it - it is your
subsystem and your sys/dev/sff skeleton that this builds on. I would just like the
disagreement to be about a design you can describe, so I can implement it.

[bz]

I believe the ACPI path was never tested on main - or we haven't flipped the defaults yet -- but for sure it'll fail to link modules due to unresolved symbols (in the future).

I do not understand why not both FDT and ACPI code paths are both fine with the bus function but ACPI is getting special cased afterwards. That's an architectural nightmare. The real architecture is for the MAC to provide the PHY access and if it isn't described and has to be hard coded it should then appear in the same way as an FDT node would; making this two different paths is not going to fly. That's just a lazy shortcut. I mean otherwise we also wouldn't need a get_sff_dev bus function on the MC if it's only used for FDT? We could just shortcut that as well, right?

DPAA2 is not a static architecture., so sadly the NI is the wrong place for the ioctl handler despite it may seem so based on the current code as a ni could possibly vanish on demand (not in the current code base yet). Similar problems already exist for the phy and I'd rather not duplicate that as it was taking quite some time to de-tangle and cleanup. It's a problem as we tie the SFF information to an ifnet for querying, which is a real problem if you don't have an ifnet at times and still need to know if there's an SFP and it's state (something entirely possible with DPAA2).

Just a few things I noticed while quickly scrolling through.
Does it work? Sure? Does that help long term, not much.

[bz]

In D58258#1367668, @yarshure_gmail.com wrote:

.. My reading is that you have in mind a MAC-link layer
for dpaa2 along the lines of Linux's phylink: one place that owns link state for a
DPMAC and drives it from whatever is attached - a PHY via MDIO, a fixed-link, or an
SFP cage - so that module presence/LOS, TX_DISABLE and rate selection are handled
there rather than in each consumer. If that is roughly it, please correct the
details and I will work to it.

That's already convoluting things.
LOS, etc. are a property of SFF.

[dsl]

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

MACLINK is supposed to be an abstraction which hides possibly complex topology of the devices and their interconnections which constitute a multi-gigabit link between the MAC (which is a part of a SoC usually) and a PHY/SFP+ on a PCB. This is the best explanation I've to date: https://github.com/mcusim/freebsd-src/blob/dpaa2/sys/dev/maclink/maclink.c#L31. All of the existing MACLINK bits live in https://github.com/mcusim/freebsd-src/tree/dpaa2/sys/dev/maclink, but haven't been tested yet.

Personally, I'd like a proper abstraction to be introduced first with a clear understanding how a maclink_bus can be attached by the network interface drivers and introduce new (specific?) maclink adapters which will be incapsulating all of the complex logic to discover PHYs, SFF/SFPs, PCSs, etc. and let the maclink bus (and the attaching network interface) know about high-level events, e.g. state changes, link's up/down, etc.

[adrian]

In D58258#1367937, @dsl wrote:

In D58258#1367599, @adrian wrote:

In D58258#1367549, @dsl wrote:

@adrian well, I'm not sure that the contributor actually understands the code. It seems AI/ML generated to me and isn't aligned with the idea of mine about maclink. I'm against the changes.

What do you mean by "maclink" ? They said they used AI assistance in developing it, but it seems mostly simple enough:

• sfp is an i2c device on a bus;
• the i2c bus doesn't HAVE to be hooked up to the MAC in any way; it in theory could be hanging off of some other i2c controller in the system;
• there's information about where said bus is linked to in FDT;
• some simple shenanigans are required to be able to talk to it and fetch configuration parameters.

What doesn't quite jive with your assumptions of stuff?

MACLINK is supposed to be an abstraction which hides possibly complex topology of the devices and their interconnections which constitute a multi-gigabit link between the MAC (which is a part of a SoC usually) and a PHY/SFP+ on a PCB. This is the best explanation I've to date: https://github.com/mcusim/freebsd-src/blob/dpaa2/sys/dev/maclink/maclink.c#L31. All of the existing MACLINK bits live in https://github.com/mcusim/freebsd-src/tree/dpaa2/sys/dev/maclink, but haven't been tested yet.

Personally, I'd like a proper abstraction to be introduced first with a clear understanding how a maclink_bus can be attached by the network interface drivers and introduce new (specific?) maclink adapters which will be incapsulating all of the complex logic to discover PHYs, SFF/SFPs, PCSs, etc. and let the maclink bus (and the attaching network interface) know about high-level events, e.g. state changes, link's up/down, etc.

Ok, so this isn't hooked up yet? Does having the EEPROM stuff exposed make it easier for you to finish up and test your maclink idea?

Like, I get it, you'd like this to be using an abstraction that's KIND of like our miibus for PHYs but not making assumptions its an MDIO bus interface.

But that's not yet in the tree, and so I don't think it's OK to block getting some more working code into the tree upon something that isn't yet in the tree itself.

This seems small enough that it wouldn't make it difficult to do your maclink idea. I like it, it would be nice for us to have a mii bus implementation that wasn't so MDIO clause 22 based. But at least with this code in the tree you can talk to SFP modules now and continue fleshing out your maclink idea with real hardware.

[yarshure_gmail.com]

@dsl @adrian — I took D58258#1368055 literally and built MACLINK, then ran it on
an LX2160A (SolidRun CEX7, UEFI/ACPI; dpni0 = dpmac.17 RGMII, dpni1 = dpmac.8 and
dpni2 = dpmac.9, both 10G SFP+ with modules in).

Copied sys/dev/maclink verbatim from the dpaa2 branch, added it to
sys/conf/files + device maclink, and called maclink_attach() from
dpaa2_ni_attach(). None of that exists in that tree — maclink is not in
sys/conf/files, there is no sys/modules/maclink, and nothing calls
maclink_attach() — so I think this is the first time it has been compiled into
a kernel.

It compiles cleanly: no errors, no warnings, -Werror, arm64.

Two small things stop it attaching:

1. maclink_attach() never assigns *mlbus. The *mlbus == NULL branch
creates the bus in the local bus, but the success path returns 0 without
writing it back, so a caller always sees NULL and a second call would create a
second bus:

dpaa2_ni0: maclink_attach() = 0, mlbus = 0
dpaa2_ni1: maclink_attach() = 0, mlbus = 0
dpaa2_ni2: maclink_attach() = 0, mlbus = 0

2. maclink_bus_driver is only DEFINE_CLASS_0()d — there is no
DRIVER_MODULE() anywhere, so the child maclink_attach() adds can never probe.
I assume that is deliberate and the consumer registers it, like miibus(4), but
there is no consumer to show it. Adding
DRIVER_MODULE(maclink_bus, dpaa2_ni, maclink_bus_driver, 0, 0); next to the
existing miibus line was enough.

With those two it comes up:

dpaa2_ni0 <DPAA2 Network Interface>
  maclink_bus0 <MACLINK bus>
    maclink_phy0 <MACLINK PHY adapter>
dpaa2_ni1 <DPAA2 Network Interface>
  maclink_bus1 <MACLINK bus>
    maclink_phy1 <MACLINK PHY adapter>
dpaa2_ni2 <DPAA2 Network Interface>
  maclink_bus2 <MACLINK bus>
    maclink_phy2 <MACLINK PHY adapter>

which also shows the next open piece: dpni1 and dpni2 are the two SFP+ ports,
both with a module in them, and both got maclink_phy. maclink_attach() adds a
single unnamed child and both adapters probe it unconditionally at
BUS_PROBE_DEFAULT, so which one wins is arbitrary — the discovery the comment
in maclink.c describes is still to do.

Smaller things noticed while reading: none of the six methods in the two .m
files has a DEFAULT, so forwarding ends in kobj_error_method (two of them
return void); struct maclink_data is the bus softc and its mtx and SLIST
head are never initialised; businfo/adpinfo are M_NOWAIT without M_ZERO;
and maclink_conf/maclink_state/maclink_link_state are each
{ int placeholder; }.

On how this review relates to it: maclink_sfp_attach() is
/* XXX-DSL: to be done. */. Whoever writes it needs two things — find the
transceiver firmware associates with the MAC, and read its EEPROM — which is what
this series adds, as DPAA2_MC_GET_SFF_DEV() and SFF_READ_EEPROM() on a real
sff(4) device. I do not think they are alternatives: MACLINK is link state and
the sequencing of PCS/PHY/SFP, sff(4) is talking to the module, and a finished
maclink_sfp would call into sff(4) rather than replace it. Happy to help with
the discovery part now that I have maclink building and attaching on hardware.

Unrelated, on testing coverage: I have an LX2160A DPU/SmartNIC arriving shortly.
It looks like 4x25G, so a different configuration from the Honeycomb — I will
report what it looks like once it boots, and I am happy to add it to the boards I
test these changes on.

[yarshure_gmail.com]

Reworked along the lines @bz asked for, and split into a stack so each piece can
be read on its own. This revision is now only the consumer -- the SIOCGI2C
handler -- and the tunables are gone.

The stack, oldest first:

D60060	iicmux: do not walk a bogus OFW node on systems described by ACPI
D60061	pca954x: attach to muxes described by ACPI as well as by FDT
D60062	acpi_iicbus: shift the ACPI slave address into the form iicbus(4) stores
D60063	acpi: read a child's properties through its ACPI handle
D60064	acpi_iicbus, pca954x: map ACPI namespace scopes onto i2c mux channels
D60065	dpaa2: pass the child up when memac_mdio reads an ivar
D60066	sff: add a shared EEPROM read helper and build sff(4) as a module
D60067	sff: add an ACPI front-end for SFP transceivers
D60068	dpaa2: resolve a DPMAC's SFP transceiver under ACPI too
D58258	this one: read the EEPROM through SIOCGI2C

D60060-D60065 have nothing to do with dpaa2 or SFP at all -- they are i2c and
ACPI fixes that happen to be what the transceiver path needs. Two of them are
visible on their own on this board: D60064 is why the fan controller and thermal
sensor the firmware describes were unattached devices on acpi0, and D60065 is
what the RGMII port's PHY needs once properties come from the child's handle.

On the two architectural points:

FDT and ACPI must not be two code paths. They are not any more. ACPI
describes the transceiver where ACPI describes an i2c device -- in the scope of
the bus its EEPROM answers on -- so sfp_acpi(4) has that bus as its parent
where sfp_fdt(4) has to follow an "i2c-bus" phandle. Above that they are the
same: read_eeprom is the same single call, DPAA2_MC_GET_SFF_DEV() resolves
either, and this handler cannot tell which described the hardware. No mux
programming anywhere -- iicbus(4) switches the mux as part of granting the bus.

Hard-coding should appear as a device. Nothing is hard-coded now. This
board's firmware does not describe the cages, so rather than detect the board in
the kernel I extended its ACPI tables, in the idiom they already use for PHYs,
and tested against that. The ASL is small and is the thing to ask SolidRun/NXP
for; the kernel carries no board knowledge.

Still open, and I would rather settle it separately: SFF state that does not go
through an ifnet. SIOCGI2C is an ifnet ioctl and every driver implementing it
is in the same position, so this keeps that shape. Ownership is where you asked
for it -- the transceiver is its own device, the MAC resolves it, the NI is a
thin consumer -- so a non-ifnet query interface can be added later without
moving anything.

Also rebased onto current main (it was 658 commits behind). Test plans on each
revision carry the hardware evidence; the MACLINK build-and-run report is in
D58258#1368155.