Thread (7 messages) flat view 7 messages, 2 authors, 28d ago

Re: [PATCH net-next v2 2/2] net: sfp: add quirks for OEM XGSPONST2001 and FS XGS-SFP-ONT-MACI

From: Martino Dell'Ambrogio <hidden>
Date: 2026-08-12 15:12:55
Also in: lkml

On Mon, 10 Aug 2026 17:14:29 -0700, Jakub Kicinski wrote:
That leaves two possibilities for this module.  Either the PN field really
does read "XGS-SFP-ONT-MACI" and SFP_QUIRK_F() would have matched just as
well, so the commit message statement
[...]
does not hold for this entry, or the field really does contain garbage,
which then falls inside the 16 compared bytes and neither prefix nor exact
matching can ever fire, making the new entry dead code.

Which of the two is it, and could the comment and the commit message be
adjusted to describe what the code actually does here?
It's the first one: the field really does read "XGS-SFP-ONT-MACI". The
product name is XGS-SFP-ONT-MAC-I; the last hyphen does not fit the
16-byte field, so the field is fully occupied by legitimate characters
and there is nothing left to pad, with garbage or otherwise. I re-read
the module I have in service to make sure:

  vendor  "FS" + 14 spaces (properly padded)
  PN      58 47 53 2d 53 46 50 2d 4f 4e 54 2d 4d 41 43 49  "XGS-SFP-ONT-MACI"

So prefix and exact matching are indeed identical here, and the "Both
modules fail to space-pad" paragraph overclaims. In v3 this entry
becomes a plain SFP_QUIRK_F, and the commit message makes the
garbage-padding argument only for the XGSPONST2001, where it actually
holds: that stick returns the 12 legitimate characters followed by '!'
and non-printable bytes in the PN field on cold power-up (the same
module reads back clean and space-padded after a warm reseat). The
cold boot is exactly where the quirk has to land, otherwise TX_FAULT
is honored and the state machine disables the module.
Would it be worth either noting in the comments that the vendor field is
matched as a prefix too, or splitting the flag so prefix semantics only
apply to the part field?
Splitting it. v3 scopes the flag to the part field (see my reply on
1/2) and the comments follow.

Thanks,
Martino
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help