[xcvr]: Detect coherent modules via CoherentPagesSupported bit

[gs1571]

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.

[mssonicbld]

/azp run

[azure-pipelines]

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

[azure-pipelines]

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

[mssonicbld]

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 ?

[mssonicbld]

/azp run

[azure-pipelines]

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

[aditya-nexthop]

we can just use get_cmis_rev here?

Also, being pedantic here, but if a current SONiC release runs a CMIS 5.3+ coherent module that does not correctly advertise COHERENT_PAGES_SUPPORTED bit, such a module would be detected as coherent before, but will not be detected as coherent after this change.

We could claim though, that in that case, that the module is not CMIS-compliant due to the mis-advertisement. It is a change of behavior though.

[gs1571]

You're right that
returning bool(CoherentPagesSupported) directly on 5.3+ was a regression: a
coherent module that reports the bit as 0 while still naming a ZR/FOIC
media interface would flip from coherent to non-coherent.

Reworked is_coherent_module() so detection is the union of the two signals
rather than the bit overriding the name. The name match returns True on its own (same as before the bit existed), and the bit now only adds coherent modules whose name contains neither keyword.
It can no longer drop a module the name match already covered, so the behavior
you flagged is gone. Added a test for a 5.3+ module reporting the bit as 0
with a coherent media interface name.

get_cmis_rev. I looked at reusing it, but it returns a formatted string
("5.3", or "None.None" when the read fails), so it can't feed the numeric
(major, minor) >= (5, 3) comparison without parsing it back — and a
lexicographic version compare would be unsafe ("5.10" < "5.3"). I kept the two
explicit int reads for that reason. Happy to switch to a small numeric helper
if you'd prefer that instead.

[prgeor]

@gs1571 may I suggest as follows?

    @read_only_cached_api_return
    def is_coherent_module(self):
        '''
        Returns True if the module follows the C-CMIS spec, False otherwise.

        Detection is the union of two independent signals, so the bit can
        only add coherent modules and never drops one that used to be
        detected:
          * the media interface name contains 'ZR' or 'FOIC' - works on
            every CMIS revision and matches the behavior from before this
            bit existed, and
          * CoherentPagesSupported (Page 01h byte 142 bit 4) is set. That
            bit is only defined from OIF-CMIS 5.3 on (Reserved before, where
            a real module may report it as a stray 1), so it is only honored
            when the module reports CMIS 5.3 or later.
        '''
        mintf = self.get_module_media_interface()
        if mintf is not None and any(kw in mintf for kw in ('ZR', 'FOIC')):
            return True
        try:
            cmis_rev = self.get_cmis_rev()
            if tuple(int(x) for x in cmis_rev.split('.')) >= (5, 3):
                return bool(self.xcvr_eeprom.read(consts.COHERENT_PAGES_SUPPORTED))
        except (ValueError, AttributeError):
            pass
        return False

[gs1571]

Thanks @prgeor — adopted in 0c78a71. is_coherent_module() now gates the
CoherentPagesSupported bit via get_cmis_rev(), parsing the revision into an int
tuple (so 5.10 >= 5.3 stays correct) with a try/except that falls back to
name-only detection if the revision read fails ("None.None"). Behaviour is
unchanged and the full cmis/ccmis unit test suite passes. PTAL.

[mssonicbld]

/azp run

[azure-pipelines]

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

[gs1571]

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.

[mssonicbld]

/azp run

[azure-pipelines]

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

[gs1571]

Hi @prgeor — this one is ready to merge. The two Copilot review comments were addressed in adc0dbf — a None-guard
in is_coherent_module(), and moving CoherentPagesSupported into its own RegBitField
so the neighbouring PAGE_SUPPORT_ADVT_FIELD decode stays unchanged. This is the last
PR in the same coherent-optics series as #718, which you merged on Jul 28. Could you
take a look when you have a moment? Thanks!

[mssonicbld]

/azp run

[azure-pipelines]

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

[mssonicbld]

/azp run

[azure-pipelines]

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