optoe: Add CMIS Bank support for transceivers with >8 lanes #473
+330
−0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,329 @@ | ||
| From aa39d1af8ae72a7a3f6f5337266d69f720a39086 Mon Sep 17 00:00:00 2001 | ||
| From: Wataru Ishida <[email protected]> | ||
| Date: Wed, 2 Apr 2025 07:10:10 +0000 | ||
| Subject: [PATCH] optoe: Add CMIS Bank support for transceivers with >8 lanes | ||
|
|
||
| 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 can be enabled only for the 'optoe3' dev class. | ||
| - '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. | ||
|
|
||
| Signed-off-by: Wataru Ishida <[email protected]> | ||
| --- | ||
| drivers/misc/eeprom/optoe.c | 165 +++++++++++++++++++++++++++++++----- | ||
| 1 file changed, 142 insertions(+), 23 deletions(-) | ||
|
|
||
| diff --git a/drivers/misc/eeprom/optoe.c b/drivers/misc/eeprom/optoe.c | ||
| index 22d2c0cd4..a4def4986 100644 | ||
| --- a/drivers/misc/eeprom/optoe.c | ||
| +++ b/drivers/misc/eeprom/optoe.c | ||
| @@ -101,9 +101,42 @@ | ||
| * considerably more pages (at least to page 0xAF), which this driver | ||
| * supports. | ||
| * | ||
| - * NOTE: This version of the driver ONLY SUPPORTS BANK 0 PAGES on CMIS | ||
| - * devices. | ||
| + * 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. | ||
|
There was a problem hiding this comment. @ishidawataru We probably don't need this. see my comments above. |
||
| * | ||
| + * The maximum number of banks supported in this version is 8. | ||
| + * | ||
| + * When access to the Banked Pages is enabled, they are mapped into a linear | ||
| + * address space. The mapping starts right after the Non-Banked Page area, | ||
| + * as shown below. | ||
| + * | ||
| + * +-------------------------------+ | ||
| + * | Lower Page | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 0, Page 0h) | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 0, Page 1h) | | ||
| + * +-------------------------------+ | ||
| + * | ... | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 0, Page FFh) | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 1, Page 10h) | | ||
| + * +-------------------------------+ | ||
| + * | ... | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 1, Page FFh) | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 2, Page 10h) | | ||
| + * +-------------------------------+ | ||
| + * | ... | | ||
| + * +-------------------------------+ | ||
| + * | Upper Page (Bank 2, Page FFh) | | ||
| + * +-------------------------------+ | ||
| + * | ... | | ||
| + * (continued for more banks) | ||
| **/ | ||
|
|
||
| /* #define DEBUG 1 */ | ||
| @@ -150,6 +183,16 @@ struct optoe_platform_data { | ||
|
|
||
| /* fundamental unit of addressing for EEPROM */ | ||
| #define OPTOE_PAGE_SIZE 128 | ||
| + | ||
| +/* | ||
| + * | ||
| + * | ||
| + */ | ||
| +#define OPTOE_DEFAULT_BANK_SIZE 0 | ||
|
There was a problem hiding this comment. @ishidawataru the default bank is 0, which means modules that support 8 lanes, the bank size = 1? There was a problem hiding this comment. @ishidawataru This probably can be removed if we implement full range of linear address for all bank pages. See my comments below |
||
| +#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 */ | ||
|
Comment on lines
+81
to
+83
There was a problem hiding this comment. @ishidawataru I am bit confused with respect to naming these by There was a problem hiding this comment. If these constants are still necessary after the discussion below, I’ll rename them as you suggested. |
||
| + | ||
| /* | ||
| * Single address devices (eg QSFP) have 256 pages, plus the unpaged | ||
| * low 128 bytes. If the device does not support paging, it is | ||
| @@ -168,6 +211,7 @@ struct optoe_platform_data { | ||
| #define TWO_ADDR_NO_0X51_SIZE (2 * OPTOE_PAGE_SIZE) | ||
|
|
||
| /* a few constants to find our way around the EEPROM */ | ||
| +#define OPTOE_BANK_SELECT_REG 0x7E | ||
| #define OPTOE_PAGE_SELECT_REG 0x7F | ||
| #define ONE_ADDR_PAGEABLE_REG 0x02 | ||
| #define QSFP_NOT_PAGEABLE (1<<2) | ||
| @@ -195,6 +239,7 @@ struct optoe_data { | ||
| u8 *writebuf; | ||
| unsigned int write_max; | ||
| unsigned int write_timeout; | ||
| + unsigned int bank_size; /* 0 means bank is not supported */ | ||
|
|
||
| unsigned int num_addresses; | ||
|
|
||
| @@ -245,13 +290,19 @@ static const struct i2c_device_id optoe_ids[] = { | ||
| }; | ||
| MODULE_DEVICE_TABLE(i2c, optoe_ids); | ||
|
|
||
| +static uint32_t one_addr_eeprom_size_with_bank(uint32_t bank_size) | ||
| +{ | ||
| + bank_size = bank_size == 0 ? 1 : bank_size; | ||
| + return (bank_size * OPTOE_BANKED_PAGE_SIZE + OPTOE_NON_BANKED_PAGE_SIZE + 1) * OPTOE_PAGE_SIZE; | ||
| +} | ||
| + | ||
| /*-------------------------------------------------------------------------*/ | ||
| /* | ||
| * This routine computes the addressing information to be used for | ||
| * a given r/w request. | ||
| * | ||
| * Task is to calculate the client (0 = i2c addr 50, 1 = i2c addr 51), | ||
| - * the page, and the offset. | ||
| + * the bank, the page, and the offset. | ||
| * | ||
| * Handles both single address (eg QSFP) and two address (eg SFP). | ||
| * For SFP, offset 0-255 are on client[0], >255 is on client[1] | ||
| @@ -274,7 +325,7 @@ MODULE_DEVICE_TABLE(i2c, optoe_ids); | ||
| */ | ||
|
|
||
| static uint8_t optoe_translate_offset(struct optoe_data *optoe, | ||
| - loff_t *offset, struct i2c_client **client) | ||
| + loff_t *offset, struct i2c_client **client, uint8_t *bank) | ||
| { | ||
| unsigned int page = 0; | ||
|
|
||
| @@ -301,8 +352,9 @@ static uint8_t optoe_translate_offset(struct optoe_data *optoe, | ||
| page = (*offset >> 7)-1; | ||
| /* 0x80 places the offset in the top half, offset is last 7 bits */ | ||
| *offset = OPTOE_PAGE_SIZE + (*offset & 0x7f); | ||
| - | ||
| - return page; /* note also returning client and offset */ | ||
|
There was a problem hiding this comment. @ishidawataru The computation does not consider full range of linear address for all banks. Its OK to use all page address for bank 0 i.e Assume that page 0x10, 0x11, 0x12... 0xFF exist for Bank 0 as well. So that the getaddr() can be implemented as follows:- In your case you are not wasting the address space for Here is the simple decoding of the above linear address that optoe needs to do:- Assumption here is the user application will provide only the valid pages and the module will return IO error if incorrect Page address is selected. There was a problem hiding this comment. @prgeor This is the linear address map of the current implementation. Are you suggesting to change it like below?
Returning I/O errors for the grayed-out area will prevent us from using cat on the EEPROM file as shown below. I suggest keeping the current implementation or returning the value from bank 0, rather than returning I/O errors. What do you think? |
||
| + *bank = page < OPTOE_PAGE_SIZE ? 0 : (page - OPTOE_NON_BANKED_PAGE_SIZE) / OPTOE_BANKED_PAGE_SIZE; | ||
| + page = page - *bank * OPTOE_BANKED_PAGE_SIZE; | ||
| + return page; /* note also returning client, bank and offset */ | ||
| } | ||
|
|
||
| static ssize_t optoe_eeprom_read(struct optoe_data *optoe, | ||
| @@ -511,21 +563,38 @@ static ssize_t optoe_eeprom_update_client(struct optoe_data *optoe, | ||
| { | ||
| struct i2c_client *client; | ||
| ssize_t retval = 0; | ||
| - uint8_t page = 0; | ||
| + uint8_t page = 0, bank = 0; | ||
| loff_t phy_offset = off; | ||
| int ret = 0; | ||
|
|
||
| - page = optoe_translate_offset(optoe, &phy_offset, &client); | ||
| + page = optoe_translate_offset(optoe, &phy_offset, &client, &bank); | ||
| dev_dbg(&client->dev, | ||
| - "%s off %lld page:%d phy_offset:%lld, count:%ld, opcode:%d\n", | ||
| - __func__, off, page, phy_offset, (long int) count, opcode); | ||
| + "%s off %lld bank:%d page:%d phy_offset:%lld, count:%ld, opcode:%d\n", | ||
| + __func__, off, bank, page, phy_offset, (long int) count, opcode); | ||
| if (page > 0) { | ||
| - ret = optoe_eeprom_write(optoe, client, &page, | ||
| - OPTOE_PAGE_SELECT_REG, 1); | ||
| + /* | ||
| + * CMIS 5.3 8.2.15 Page Mapping | ||
| + * | ||
| + * For an arbitrary Page Address change or for just a Bank Index change, | ||
| + * a host must write both BankSelect and PageSelect in one WRITE access, | ||
| + * even if the PageSelect value does not change. | ||
| + * The module does not begin processing the BankSelect value until after the PageSelect register has been written. | ||
| + * | ||
| + * 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) { | ||
| + char buf[2] = {bank, page}; | ||
| + ret = optoe_eeprom_write(optoe, client, buf, | ||
| + OPTOE_BANK_SELECT_REG, 2); | ||
| + } else { | ||
| + ret = optoe_eeprom_write(optoe, client, &page, | ||
| + OPTOE_PAGE_SELECT_REG, 1); | ||
| + } | ||
| if (ret < 0) { | ||
| dev_dbg(&client->dev, | ||
| - "Write page register for page %d failed ret:%d!\n", | ||
| - page, ret); | ||
| + "Write page register for bank %d, page %d failed ret:%d!\n", | ||
| + bank, page, ret); | ||
| return ret; | ||
| } | ||
| } | ||
| @@ -553,13 +622,18 @@ static ssize_t optoe_eeprom_update_client(struct optoe_data *optoe, | ||
|
|
||
|
|
||
| if (page > 0) { | ||
| - /* return the page register to page 0 (why?) */ | ||
| - page = 0; | ||
| - ret = optoe_eeprom_write(optoe, client, &page, | ||
| - OPTOE_PAGE_SELECT_REG, 1); | ||
| + if (bank > 0) { | ||
| + char buf[2] = {0, 0}; | ||
| + ret = optoe_eeprom_write(optoe, client, buf, | ||
| + OPTOE_BANK_SELECT_REG, 2); | ||
| + } else { | ||
| + page = 0; | ||
| + ret = optoe_eeprom_write(optoe, client, &page, | ||
| + OPTOE_PAGE_SELECT_REG, 1); | ||
| + } | ||
| if (ret < 0) { | ||
| dev_err(&client->dev, | ||
| - "Restore page register to 0 failed:%d!\n", ret); | ||
| + "Restore bank, page register to (0, 0) failed:%d!\n", ret); | ||
| /* error only if nothing has been transferred */ | ||
| if (retval == 0) | ||
| retval = ret; | ||
| @@ -622,8 +696,10 @@ static ssize_t optoe_page_legal(struct optoe_data *optoe, | ||
| /* if no pages needed, we're good */ | ||
| if ((off + len) <= ONE_ADDR_EEPROM_UNPAGED_SIZE) | ||
| return len; | ||
| + | ||
| + maxlen = one_addr_eeprom_size_with_bank(optoe->bank_size); | ||
| /* if offset exceeds possible pages, we're not good */ | ||
| - if (off >= ONE_ADDR_EEPROM_SIZE) | ||
| + if (off >= maxlen) | ||
| return OPTOE_EOF; | ||
| /* in between, are pages supported? */ | ||
| status = optoe_eeprom_read(optoe, client, ®val, | ||
| @@ -665,7 +741,7 @@ static ssize_t optoe_page_legal(struct optoe_data *optoe, | ||
| maxlen = ONE_ADDR_EEPROM_UNPAGED_SIZE - off; | ||
| } else { | ||
| /* Pages supported, trim len to the end of pages */ | ||
| - maxlen = ONE_ADDR_EEPROM_SIZE - off; | ||
| + maxlen = maxlen - off; | ||
| } | ||
| len = (len > maxlen) ? maxlen : len; | ||
| dev_dbg(&client->dev, | ||
| @@ -995,6 +1071,47 @@ static ssize_t set_port_name(struct device *dev, | ||
| return count; | ||
| } | ||
|
|
||
| +static ssize_t show_bank_size(struct device *dev, | ||
| + struct device_attribute *dattr, char *buf) | ||
| +{ | ||
| + struct i2c_client *client = to_i2c_client(dev); | ||
| + struct optoe_data *optoe = i2c_get_clientdata(client); | ||
| + ssize_t count; | ||
| + | ||
| + mutex_lock(&optoe->lock); | ||
| + count = sprintf(buf, "%u\n", optoe->bank_size); | ||
| + mutex_unlock(&optoe->lock); | ||
| + | ||
| + return count; | ||
| +} | ||
| + | ||
| +static ssize_t set_bank_size(struct device *dev, | ||
| + struct device_attribute *attr, | ||
| + const char *buf, size_t count) | ||
| +{ | ||
| + struct i2c_client *client = to_i2c_client(dev); | ||
| + struct optoe_data *optoe = i2c_get_clientdata(client); | ||
| + unsigned int bank_size; | ||
| + | ||
| + // setting bank size is only supported for the CMIS device | ||
| + if (optoe->dev_class != CMIS_ADDR) { | ||
| + return -EINVAL; | ||
| + } | ||
| + | ||
| + if (kstrtouint(buf, 0, &bank_size) != 0 || | ||
| + bank_size < 0 || bank_size > OPTOE_MAX_SUPPORTED_BANK_SIZE) | ||
| + return -EINVAL; | ||
| + | ||
| + mutex_lock(&optoe->lock); | ||
| + optoe->bank_size = bank_size; | ||
| + if (optoe->bank_size > 0 && optoe->write_max == 1) { | ||
| + optoe->write_max = 2; | ||
| + } | ||
| + mutex_unlock(&optoe->lock); | ||
| + | ||
| + return count; | ||
| +} | ||
| + | ||
| static DEVICE_ATTR(port_name, 0644, show_port_name, set_port_name); | ||
| #endif /* if NOT defined EEPROM_CLASS, the common case */ | ||
|
|
||
| @@ -1003,6 +1120,7 @@ static DEVICE_ATTR(write_timeout, 0644, show_dev_write_timeout_size, | ||
| static DEVICE_ATTR(write_max, 0644, show_dev_write_max_size, | ||
| set_dev_write_max_size); | ||
| static DEVICE_ATTR(dev_class, 0644, show_dev_class, set_dev_class); | ||
| +static DEVICE_ATTR(bank_size, 0644, show_bank_size, set_bank_size); | ||
|
|
||
| static struct attribute *optoe_attrs[] = { | ||
| #ifndef EEPROM_CLASS | ||
| @@ -1011,6 +1129,7 @@ static struct attribute *optoe_attrs[] = { | ||
| &dev_attr_write_timeout.attr, | ||
| &dev_attr_write_max.attr, | ||
| &dev_attr_dev_class.attr, | ||
| + &dev_attr_bank_size.attr, | ||
| NULL, | ||
| }; | ||
|
|
||
| @@ -1027,7 +1146,6 @@ static int optoe_probe(struct i2c_client *client, | ||
| struct optoe_data *optoe; | ||
| int num_addresses = 0; | ||
| char port_name[MAX_PORT_NAME_LEN]; | ||
| - | ||
| if (client->addr != 0x50) { | ||
| dev_dbg(&client->dev, "probe, bad i2c addr: 0x%x\n", | ||
| client->addr); | ||
| @@ -1108,7 +1226,7 @@ static int optoe_probe(struct i2c_client *client, | ||
| } else if (strcmp(client->name, "optoe3") == 0) { | ||
| /* CMIS spec */ | ||
| optoe->dev_class = CMIS_ADDR; | ||
| - chip.byte_len = ONE_ADDR_EEPROM_SIZE; | ||
| + chip.byte_len = one_addr_eeprom_size_with_bank(OPTOE_MAX_SUPPORTED_BANK_SIZE); | ||
| num_addresses = 1; | ||
| } else { /* those were the only choices */ | ||
| err = -EINVAL; | ||
| @@ -1120,6 +1238,7 @@ static int optoe_probe(struct i2c_client *client, | ||
| optoe->chip = chip; | ||
| optoe->num_addresses = num_addresses; | ||
| optoe->write_timeout = OPTOE_DEFAULT_WRITE_TIMEOUT; | ||
| + optoe->bank_size = OPTOE_DEFAULT_BANK_SIZE; | ||
| memcpy(optoe->port_name, port_name, MAX_PORT_NAME_LEN); | ||
|
|
||
| /* | ||
| -- | ||
| 2.25.1 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’d be great if you could add a paragraph how to verify/test this to the commit message too.