[xcvr]: Detect coherent modules via CoherentPagesSupported bit by gs1571 · Pull Request #720 · sonic-net/sonic-platform-common

gs1571 · 2026-07-17T05:19:02Z

Description

is_coherent_module() decided coherency purely by searching for the substring
'ZR' in the free-text module media interface name. This breaks for any
coherent module whose active application advertises a media interface name
without 'ZR' in it (e.g. FOIC-only names such as
'FOIC1.4-DO (G.709.3/Y.1331.3)').

Added the CoherentPagesSupported bit — Page 01h byte 142 bit 4, defined in
OIF-CMIS-05.3, section 8.4.4 "Supported Pages Advertisement", Table 8-46
"Supported Pages Advertising (Page 01h)" ("Banked Pages 30h-4Fh (partially)
supported") — to the Page 01h mem map, alongside the existing
VdmSupported/DiagPageSupportAdvtField bits already defined on that same
byte. Spec: https://www.oiforum.com/wp-content/uploads/OIF-CMIS-05.3.pdf

is_coherent_module() now reads this bit first, since it is defined by spec
rather than inferred from a name, and falls back to matching 'ZR'/'FOIC'
in the media interface name when the module/CMIS revision does not expose
the bit (read returns None).

Motivation and Context

Found bringing up an 800G ZR+ coherent module (CMIS 5.3) whose default active
Application advertises a media interface name containing 'FOIC' but not
'ZR' — is_coherent_module() returned False, so CCmisApi was never
instantiated and the module's laser could never be configured.

* is_coherent_module() decided coherency purely by searching for the substring 'ZR' in the free-text module media interface name. This breaks for any coherent module whose active application advertises a media interface name without 'ZR' in it (e.g. FOIC-only names such as 'FOIC1.4-DO (G.709.3/Y.1331.3)'). * Add the CoherentPagesSupported bit (Page 01h byte 142 bit 4, OIF-CMIS 5.x+) to the Page 01h mem map, alongside the existing VdmSupported/ DiagPageSupportAdvtField bits on the same byte. * is_coherent_module() now reads this bit first, since it is defined by spec rather than inferred from a name. Falls back to matching 'ZR' or 'FOIC' in the media interface name when the module/CMIS revision does not expose the bit (read returns None). Signed-off-by: Grigory Solovyev <gs1571@gmail.com>

mssonicbld · 2026-07-17T05:19:09Z

/azp run

azure-pipelines · 2026-07-17T05:19:10Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

azure-pipelines · 2026-07-17T05:19:18Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-07-17T11:23:12Z

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

_{---Powered by SONiC BuildBot}

aditya-nexthop

Please can you post in the testing section whether your change still correctly detects coherent (ZR or otherwise) modules advertising CMIS 5.2 or earlier with the Coherent pages supported bit set to both 0 and 1 ?

* Page 01h byte 142 bit 4 is Reserved prior to CMIS 5.3 (OIF-CMIS-05.2 Table 8-41); it only became CoherentPagesSupported in 5.3 (OIF-CMIS-05.3 Table 8-46). XcvrEeprom.read() only returns None on an actual I2C/EEPROM read failure, not based on CMIS revision, so on a real CMIS <=5.2 module the previous code would read the reserved bit as a real 0 or 1 instead of None, short-circuiting past the 'ZR'/'FOIC' string-match fallback and misdetecting existing coherent modules as non-coherent. * Only trust the bit when the module reports CMIS 5.3 or later; fall back to the string match unconditionally otherwise, matching behavior prior to this bit's introduction. * Add tests covering pre-5.3 modules with the reserved bit read as 0, and unreadable CMIS revision fields. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>

mssonicbld · 2026-07-18T07:36:31Z

/azp run

azure-pipelines · 2026-07-18T07:36:40Z

Azure Pipelines: Successfully started running 1 pipeline(s).

* Rework is_coherent_module() so the 'ZR'/'FOIC' media interface name match returns True on its own and the CoherentPagesSupported bit only adds coherent modules whose name contains neither keyword. Previously the bit's value was returned directly on CMIS 5.3+, which could newly classify a coherent module that mis-advertises the bit as 0 (but names a 'ZR'/'FOIC' interface) as non-coherent. * Add a test for a CMIS 5.3+ module reporting the bit as 0 with a coherent media interface name. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>

mssonicbld · 2026-07-21T07:45:32Z

/azp run

azure-pipelines · 2026-07-21T07:45:41Z

Azure Pipelines: Successfully started running 1 pipeline(s).

gs1571 · 2026-07-24T08:51:34Z

Hi @aditya-nexthop,

Just checking in on this one — following your last comment, I pushed 2acf42e which reworks is_coherent_module() so detection is additive (name match OR bit), instead of the bit overriding the name on CMIS 5.3+. This should fix the regression you pointed out: a coherent module reporting CoherentPagesSupported=0 while still naming a ZR/FOIC media interface will now still be detected correctly, since the name match alone is enough on its own.

Copilot

Pull request overview

This PR improves coherent optics detection in the CMIS transceiver API by adding spec-defined detection via the CMIS 5.3 CoherentPagesSupported bit (Page 01h, byte 142, bit 4), while retaining legacy name-based detection as a fallback/union signal. This fits into sonic_platform_base/sonic_xcvr by strengthening the shared CMIS abstraction so coherent modules are recognized reliably across vendors and applications.

Changes:

Add COHERENT_PAGES_SUPPORTED field constant and map it into CMIS Page 01h advertising byte 142.
Update CCmisApi.is_coherent_module() to treat media-interface name matches ('ZR'/'FOIC') and the CMIS 5.3+ coherent-pages bit as a union of signals.
Extend unit tests to cover CMIS 5.3 bit behavior, pre-5.3 reserved-bit handling, unknown revision fallback, and FOIC-only interface names.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/sonic_xcvr/test_cmis.py	Adds/extends unit tests for coherent-module detection across CMIS revisions and naming cases.
sonic_platform_base/sonic_xcvr/mem_maps/public/cmis/pages/page01.py	Adds the `CoherentPagesSupported` bit mapping on Page 01h byte 142.
sonic_platform_base/sonic_xcvr/fields/consts.py	Introduces the `COHERENT_PAGES_SUPPORTED` field identifier constant.
sonic_platform_base/sonic_xcvr/api/public/cmis.py	Updates `is_coherent_module()` detection logic and documentation to incorporate the CMIS 5.3+ coherent-pages bit.

gs1571 · 2026-07-28T18:13:51Z

        mintf = self.get_module_media_interface()
-        return False if 'ZR' not in mintf else True
+        if any(kw in mintf for kw in ('ZR', 'FOIC')):
+            return True


Restored the None guard so a module whose media-interface name can't be read still falls through
to the bit check.

gs1571 · 2026-07-28T18:15:43Z

            NumberRegField(consts.PAGE_SUPPORT_ADVT_FIELD, self.getaddr(142),
                RegBitField(consts.VDM_SUPPORTED, 6),
                RegBitField(consts.DIAG_PAGE_SUPPORT_ADVT_FIELD, 5),
+                RegBitField(consts.COHERENT_PAGES_SUPPORTED, 4),
            ),


Agreed, this is backward-incompatible: NumberRegField.decode() right-shifts by
min(bitpos) of its sub-fields, so adding bit 4 moves Supported Pages Advertisement
from >>5 to >>4 and widens its mask — the same EEPROM byte now decodes to a
different number for any existing consumer. Moved CoherentPagesSupported to its own
standalone RegBitField at the same offset (142), mirroring the existing single-bit
TUNING_IN_PROGRESS/WAVELENGTH_UNLOCKED fields, so the neighbouring field is untouched:

NumberRegField(consts.PAGE_SUPPORT_ADVT_FIELD, self.getaddr(142), RegBitField(consts.VDM_SUPPORTED, 6), RegBitField(consts.DIAG_PAGE_SUPPORT_ADVT_FIELD, 5), ), RegBitField(consts.COHERENT_PAGES_SUPPORTED, offset=self.getaddr(142), bitpos=4),

This also makes read(COHERENT_PAGES_SUPPORTED) resolve directly to the bit. Added two
tests: is_coherent_module() returning True when the media-interface name is None
but the coherent bit is set on CMIS ≥ 5.3, and a decode-stability check that
PAGE_SUPPORT_ADVT_FIELD is unchanged when bit 4 is set.

* is_coherent_module(): guard get_module_media_interface() against None before the 'ZR'/'FOIC' name check. A failed media-interface read returns None, and `kw in None` raised TypeError before the code could fall through to the CoherentPagesSupported bit; now a coherent module with an unreadable name is still detected via the bit on CMIS 5.3+. * page01.py: move CoherentPagesSupported (byte 142 bit 4) out of the PAGE_SUPPORT_ADVT_FIELD NumberRegField into its own standalone RegBitField at the same offset. Folding the bit into the existing field changed that field's decoded value, because NumberRegField.decode right-shifts by the lowest sub-field bit position (5 -> 4) and widens the mask - a backward-incompatible change for any consumer reading Supported Pages Advertisement. * Add tests: is_coherent_module() with a None media-interface name and the bit set, and a decode-stability check that PAGE_SUPPORT_ADVT_FIELD is unaffected when byte 142 bit 4 is set. Signed-off-by: Grigory Solovyev <gs1571@gmail.com>

mssonicbld · 2026-07-28T18:10:50Z

/azp run

azure-pipelines · 2026-07-28T18:10:59Z

Azure Pipelines: Successfully started running 1 pipeline(s).

aditya-nexthop reviewed Jul 17, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/api/public/cmis.py Outdated

Comment thread tests/sonic_xcvr/test_cmis.py

aditya-nexthop reviewed Jul 20, 2026

View reviewed changes

Comment thread sonic_platform_base/sonic_xcvr/api/public/cmis.py

prgeor requested a review from Copilot July 28, 2026 14:54

Copilot started reviewing on behalf of prgeor July 28, 2026 14:55 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

aditya-nexthop approved these changes Jul 29, 2026

View reviewed changes

Uh oh!

Conversation

gs1571 commented Jul 17, 2026

Description

Motivation and Context

Uh oh!

mssonicbld commented Jul 17, 2026

Uh oh!

azure-pipelines Bot commented Jul 17, 2026

Uh oh!

azure-pipelines Bot commented Jul 17, 2026

Uh oh!

mssonicbld commented Jul 17, 2026

Uh oh!

aditya-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 18, 2026

Uh oh!

azure-pipelines Bot commented Jul 18, 2026

Uh oh!

Uh oh!

mssonicbld commented Jul 21, 2026

Uh oh!

azure-pipelines Bot commented Jul 21, 2026

Uh oh!

gs1571 commented Jul 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

gs1571 Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

gs1571 Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants