optoe: Add CMIS Bank support for transceivers with >8 lanes by ishidawataru · Pull Request #473 · sonic-net/sonic-linux-kernel

ishidawataru · 2025-04-02T07:23:20Z

This patch adds CMIS Bank support to the 'optoe3' device class in order
to enable access to CMIS transceivers with more than 8 lanes (e.g., OSFP-XD, CPO OEs).

Bank support is enabled only for the 'optoe3' dev class.
~~The default bank size is set to 4, and can be modified via the 'bank_size' sysfs entry.~~
For 'optoe3', the 'write_max' value is updated to 2 to comply with CMIS requirements,
which mandate that both bank and page values be updated in a single WRITE operation.

Updated the behavior as below after discussing with @prgeor offline

'bank_size' sysfs entry is added to enable and configure the size of the bank.
By default, bank size is set to 0.
When enabling bank by setting a value greater than 1, the 'write_max' value is
automatically updated to 2 to comply with CMIS requirements,
which mandate that both bank and page values be updated in a single WRITE operation.
If the 'write_max' value is already greater than 2, this module keeps the value as is.

Only tested with the SONiC VM + i2c-stub.

admin@sonic:~$ cat /sys/class/i2c-dev/i2c-1/device/1-0050/bank_size
4
admin@sonic:~$ hexdump /sys/class/i2c-dev/i2c-1/device/1-0050/eeprom 
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
001e880
admin@sonic:~$ echo 1 | sudo tee /sys/class/i2c-dev/i2c-1/device/1-0050/bank_size
1
admin@sonic:~$ hexdump /sys/class/i2c-dev/i2c-1/device/1-0050/eeprom 
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0008080

Snip of dmesg. See bank:3 is accessed.

[140341.928384] optoe 1-0050: eeprom write 2@126 --> 2 (4329977748)
[140341.928386] optoe 1-0050: sff_r/w: off 122880, len 2176, chunk_start_offset 124928, chunk_offset 124928, chunk_len 128, pending_len 128
[140341.928388] optoe 1-0050: optoe_eeprom_update_client off 124928 bank:3 page:255 phy_offset:128, count:128, opcode:0
[140341.928391] optoe 1-0050: eeprom write 2@126 --> 2 (4329977748)
[140341.928393] optoe 1-0050: eeprom read 32@128 --> 32 (4329977748)
[140341.928395] optoe 1-0050: eeprom read 32@160 --> 32 (4329977748)
[140341.928397] optoe 1-0050: eeprom read 32@192 --> 32 (4329977748)
[140341.928399] optoe 1-0050: eeprom read 32@224 --> 32 (4329977748)
[140341.928401] optoe 1-0050: eeprom write 2@126 --> 2 (4329977748)
[140341.928413] optoe 1-0050: optoe_read_write: off 125056  len:4096, opcode:r

Not tested with real hardware yet.

mssonicbld · 2025-04-02T07:23:28Z

/azp run

azure-pipelines · 2025-04-02T07:23:37Z

Azure Pipelines successfully started running 1 pipeline(s).

ishidawataru · 2025-04-03T00:55:16Z

@prgeor PTAL

ishidawataru · 2025-04-03T00:59:58Z

The optoe linear address space is extended as below to support Bank without changing how the user space interact with optoe.

paulmenzel

Looks great overall.

paulmenzel · 2025-04-03T06:49:56Z

+	 * 2 byte writes are acceptable for PE and Vout changes per
+	 * Application Note AN-2071.
+	 *
+	 * use 2 bytes for CMIS devices since CMIS 5.3 requires write both BankSelect and PageSlect in one WRITE acess.


Wrap the line?

paulmenzel · 2025-04-03T06:50:57Z

+- The default bank size is set to 4, and can be modified via the 'bank_size' sysfs entry.
+- For 'optoe3', the 'write_max' value is updated to 2 to comply with CMIS requirements,
+  which mandate that both bank and page values be updated in a single WRITE operation.
+


It’d be great if you could add a paragraph how to verify/test this to the commit message too.

mssonicbld · 2025-04-04T06:11:42Z

/azp run

azure-pipelines · 2025-04-04T06:11:51Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-04-04T06:18:14Z

/azp run

azure-pipelines · 2025-04-04T06:18:23Z

Azure Pipelines successfully started running 1 pipeline(s).

ishidawataru · 2025-04-04T06:20:14Z

@prgeor I updated the patch based on our discussion PTAL.

prgeor · 2025-04-21T17:46:25Z

@prgeor I updated the patch based on our discussion PTAL.

@ishidawataru Ack. reviewing.

prgeor · 2025-04-21T18:21:38Z

@ishidawataru can you define what do you mean by bank_size ?

mssonicbld · 2025-04-22T01:27:31Z

/azp run

azure-pipelines · 2025-04-22T01:27:40Z

Azure Pipelines successfully started running 1 pipeline(s).

ishidawataru · 2025-04-22T01:36:08Z

@prgeor

bank_size represents the number of CMIS transceiver banks that the optoe driver assumes.
This setting is necessary because optoe maps CMIS 3D addresses into a linear address space.
The maximum of the linear address space is determined based on the bank_size, and address validity is checked accordingly.

For example, if bank_size is 0 or 1, the linear address space ends at 0x8080.
If bank_size is 2, the address space extends up to 0xf880.

admin@sonic:~$ echo 1 | sudo tee /sys/class/i2c-dev/i2c-1/device/1-0050/bank_size                                                                                      
1
admin@sonic:~$ hexdump /sys/class/i2c-dev/i2c-1/device/1-0050/eeprom                                                                                       
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0008080
admin@sonic:~$ echo 2 | sudo tee /sys/class/i2c-dev/i2c-1/device/1-0050/bank_size                                                                                      
2
admin@sonic:~$ hexdump /sys/class/i2c-dev/i2c-1/device/1-0050/eeprom                                                                                       
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
000f880

I also added a commit that explains this in the comment.

1b2fb60

prgeor · 2025-04-21T23:04:58Z

+ /* fundamental unit of addressing for EEPROM */
+ #define OPTOE_PAGE_SIZE 128
+
+#define OPTOE_DEFAULT_BANK_SIZE       0


@ishidawataru the default bank is 0, which means modules that support 8 lanes, the bank size = 1?

@ishidawataru This probably can be removed if we implement full range of linear address for all bank pages. See my comments below

prgeor · 2025-04-21T23:11:16Z

+#define OPTOE_MAX_SUPPORTED_BANK_SIZE 8
+#define OPTOE_NON_BANKED_PAGE_SIZE    16  /* page 00h-0Fh are not banked */
+#define OPTOE_BANKED_PAGE_SIZE        240 /* page 10h-FFh are banked */


@ishidawataru I am bit confused with respect to naming these by _SIZE. From the code use, it looks like these should be _COUNT?

If these constants are still necessary after the discussion below, I’ll rename them as you suggested.

prgeor · 2025-04-22T07:15:43Z

+ * For CMIS transceivers that support Banked Pages, access to these pages
+ * is also supported. To access the banked pages, set the number of banks
+ * to access via the `bank_size` sysfs entry.
+ * By default, `bank_size` is set to 0, which disables this feature.


@ishidawataru We probably don't need this. see my comments above.

prgeor · 2026-02-04T21:44:10Z

+		 * For just a Page Index change (mapping another Page in the current Bank),
+		 * or for mapping an arbitrary unbanked Page to Upper Memory, a host may WRITE only the PageSelect Byte.
+		 */
+		if (bank > 0) {


@ishidawataru would it be good to have the sanity check to validate the bank value to what the module supports? Page 01h, Byte 142

prgeor · 2026-02-05T02:29:14Z

+	return count;
+}
+
+static ssize_t set_bank_size(struct device *dev,


@ishidawataru do we need this? What is the usecase? I thought the number of banks is already advertised so the optoe driver can read the advertisement and cache the value? Are you considering a case where the module is swapped?

mssonicbld · 2026-02-15T14:15:49Z

/azp run

azure-pipelines · 2026-02-15T14:15:58Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

This pull request adds CMIS Bank support to the optoe3 driver to enable access to CMIS transceivers with more than 8 lanes (e.g., OSFP-XD, CPO OEs). The implementation adds a configurable bank_size sysfs interface that allows extending the addressable EEPROM space beyond the default single bank (256 pages) to support up to 8 banks, mapping them into a linear address space.

Changes:

Adds bank register handling logic to the optoe driver for CMIS transceivers (optoe3 device class)
Implements bank_size sysfs attribute to enable/configure bank support (defaults to 0 = disabled)
Updates address translation logic to compute bank, page, and offset from linear address space
Modifies page/bank register restoration logic to handle bank select register writes

Reviewed changes

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

File	Description
patches-sonic/series	Adds the new bank support patch to the patch series, positioned correctly after existing optoe patches
patches-sonic/driver-support-optoe-bank-support.patch	Complete implementation of CMIS bank support including address translation, register handling, sysfs interface, and memory size calculations

Copilot · 2026-02-15T14:21:25Z

+	 * Memory layout: multiple banks, each containing 256 pages of 128 bytes.
+	 */
+	loff_t offset_in_paged_area = *offset - OPTOE_PAGE_SIZE;
+	const size_t bytes_per_bank = OPTOE_ARCH_PAGES * OPTOE_PAGE_SIZE;  // 256 * 128 = 32KB


Use C-style comment syntax /* / instead of C++ style //. Kernel code should use C-style comments for inline code comments. The comment should be: / 256 * 128 = 32KB */

Suggested change

+ const size_t bytes_per_bank = OPTOE_ARCH_PAGES * OPTOE_PAGE_SIZE; // 256 * 128 = 32KB

+ const size_t bytes_per_bank = OPTOE_ARCH_PAGES * OPTOE_PAGE_SIZE; /* 256 * 128 = 32KB */

mssonicbld · 2026-02-15T14:53:45Z

/azp run

azure-pipelines · 2026-02-15T14:53:55Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2026-02-15T15:02:26Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2026-03-23T04:27:07Z

@bobby-nexthop can you review?

bobby-nexthop

Thank you!

mssonicbld · 2026-04-23T03:52:20Z

/azp run

azure-pipelines · 2026-04-23T03:52:28Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-04-23T04:00:35Z

/azp run

azure-pipelines · 2026-04-23T04:00:45Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

mssonicbld · 2026-04-23T15:28:57Z

/azp run

azure-pipelines · 2026-04-23T15:29:07Z

Azure Pipelines successfully started running 1 pipeline(s).

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

mssonicbld · 2026-04-23T16:01:14Z

/azp run

azure-pipelines · 2026-04-23T16:01:25Z

Azure Pipelines successfully started running 1 pipeline(s).

ishidawataru · 2026-04-23T23:33:20Z

@prgeor all CI checks have passed. PTAL

prgeor · 2026-04-28T00:58:22Z

+ * The maximum supported bank size in this version is 8.
+ */
+#define OPTOE_DEFAULT_BANK_SIZE       0
+#define OPTOE_MAX_SUPPORTED_BANK_SIZE 8


@ishidawataru Why the value is 8? As per CMIS spec, the max number of lanes is 32 which means MAX bank size is 4

ishidawataru requested a review from a team as a code owner April 2, 2025 07:23

paulmenzel reviewed Apr 3, 2025

View reviewed changes

ishidawataru force-pushed the optoe-bank-support branch from b380e95 to 28fa045 Compare April 4, 2025 06:11

ishidawataru force-pushed the optoe-bank-support branch from 28fa045 to 4f10614 Compare April 4, 2025 06:18

prgeor reviewed Apr 22, 2025

View reviewed changes

prgeor reviewed Feb 4, 2026

View reviewed changes

bobby-nexthop reviewed Feb 5, 2026

View reviewed changes

Comment thread patches-sonic/driver-support-optoe-bank-support.patch

prgeor reviewed Feb 5, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 15, 2026 14:15

ishidawataru force-pushed the optoe-bank-support branch from 1b2fb60 to 053afe8 Compare February 15, 2026 14:15

Copilot started reviewing on behalf of ishidawataru February 15, 2026 14:16 View session

Copilot AI reviewed Feb 15, 2026

View reviewed changes

ishidawataru requested review from bobby-nexthop and prgeor February 15, 2026 17:15

mihirpat1 previously approved these changes Mar 5, 2026

View reviewed changes

bobby-nexthop approved these changes Mar 24, 2026

View reviewed changes

prgeor reviewed Apr 23, 2026

View reviewed changes

Comment thread patches-sonic/driver-support-optoe-bank-support.patch Outdated

prgeor dismissed mihirpat1’s stale review via 9458609 April 23, 2026 03:52

ishidawataru added 8 commits April 24, 2026 00:23

optoe: Add CMIS Bank support for transceivers with >8 lanes

ac8340c

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): add explanation for bank page support

e43e49d

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): rebased onto master

a6eb49e

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): update the mapping of banked pages as suggested

c8d1fc0

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): don't skip page selection when page restore is failed

4f9d28d

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): ensure page and bank select is written in one WRITE access

0dafa0b

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): remove unused constant and redundant value check

49e67a9

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

fix(optoe): add missing comments to bank related constants

85d8fe0

Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

ishidawataru force-pushed the optoe-bank-support branch from 37e4508 to a2d8ba6 Compare April 23, 2026 15:28

fix(optoe): apply suggestion from @Copilot

6af9b8e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Wataru Ishida <wataru.ishid@gmail.com>

ishidawataru force-pushed the optoe-bank-support branch from a2d8ba6 to 6af9b8e Compare April 23, 2026 16:01

prgeor approved these changes Apr 27, 2026

View reviewed changes

prgeor reviewed Apr 28, 2026

View reviewed changes

	+ const size_t bytes_per_bank = OPTOE_ARCH_PAGES * OPTOE_PAGE_SIZE; // 256 * 128 = 32KB
	+ const size_t bytes_per_bank = OPTOE_ARCH_PAGES * OPTOE_PAGE_SIZE; /* 256 * 128 = 32KB */

Conversation

ishidawataru commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Apr 2, 2025

Uh oh!

azure-pipelines Bot commented Apr 2, 2025

Uh oh!

ishidawataru commented Apr 3, 2025

Uh oh!

ishidawataru commented Apr 3, 2025

Uh oh!

paulmenzel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 4, 2025

Uh oh!

azure-pipelines Bot commented Apr 4, 2025

Uh oh!

mssonicbld commented Apr 4, 2025

Uh oh!

azure-pipelines Bot commented Apr 4, 2025

Uh oh!

ishidawataru commented Apr 4, 2025

Uh oh!

prgeor commented Apr 21, 2025

Uh oh!

prgeor commented Apr 21, 2025

Uh oh!

mssonicbld commented Apr 22, 2025

Uh oh!

azure-pipelines Bot commented Apr 22, 2025

Uh oh!

ishidawataru commented Apr 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

prgeor Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 15, 2026

Uh oh!

azure-pipelines Bot commented Feb 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented Feb 15, 2026

Uh oh!

azure-pipelines Bot commented Feb 15, 2026

ishidawataru commented Apr 2, 2025 •

edited

Loading

prgeor Apr 22, 2025 •

edited

Loading