Read ID register for optoe1 to find pageable bit in optoe driver

[mihirpat1]

The current optoe driver looks at bit 2 for all optoe1
(dev_class as ONE_ADDR) transceivers to detect if it's pageable or not.
However, for 100G CMIS based transceivers, some platforms use it as optoe1
and not optoe3. With CMIS, the pageable bit has now changed to bit 7 for
the same register. This causes incorrect behavior when the driver checks
for pageability on 100G CMIS transceiver and hence, we need to
read the transceiver ID to see if the transceiver is CMIS based and then
find the relevant pageable bit.

Test result
Tested the changes on a switch with a 100G CMIS and non-CMIS transceiver

Signed-off-by: Mihir Patel [email protected]

[mihirpat1]

@prgeor @Staphylo - It will be great if you can help in reviewing this

[Staphylo]

Your change looks good to me.
The only comment I might have is that the comment doesn't easily convey that this change only applies to QSFP w/ CMIS and not SFP w/ CMIS.
A mention of the SFF-8024 spec could be useful to explain where the magic ID values come from.

[mihirpat1]

Your change looks good to me. The only comment I might have is that the comment doesn't easily convey that this change only applies to QSFP w/ CMIS and not SFP w/ CMIS. A mention of the SFF-8024 spec could be useful to explain where the magic ID values come from.

Addressed this now.

[prgeor]

@mihirpat1 can you update the test result for performance issue for reading additional bytes?

[paulmenzel]

@donboll, could you please review the series?

[mihirpat1]

@mihirpat1 can you update the test result for performance issue for reading additional bytes?

I have updated the test summary for the same now.

[prgeor]

@StormLiangMS please cherry pick to 202211
@yxieca please cherry pick to 202205

[donboll]

Sorry for the delay in replying. I am in the silicon valley, or power was knocked out all week. I’ve reviewed the change, it looks simple and effective, it should work. A couple of notes, regarding the performance impact… No significant time difference is seen related to EEPROM read after adding the current changes. Below stats were taken for a 100G CMIS based transceiver with making it as optoe3 v/s optoe1 Test stats (average time taken after 3 dumps) Time to dump first 4096B from EEPROM with transceiver as optoe3 - 914ms Time to dump first 4096B from EEPROM with transceiver as optoe1 - 911ms If this test was done as a single 4K read, then there was only one read of the ID byte, which would be small compared to the full 4K read. The impacted case is short reads of pages beyond page 0. For CMIS devices, page 11 might be read often to check the “Channel state, flags and monitors”. This is where we find all the alarms and warnings, per channel, for things like TxLOS, Tx CDR LOL, Tx Power High/Low, Tx Bias Current High/Low, etc. This page might be typically read regularly by health monitoring software, for every device in the switch. Try running 100 reads of page 11 (EEPROM bytes 1536-1663) to see if the overhead is higher. Note that there should also be a performance impact on QSFP (non-CMIS) devices. These will also fall into the case that reads the ID register. Keep in mind that with the patch, optoe3 devices do not suffer the performance impact, so it is appropriate to compare optoe3 vs optoe1 CMIS device performance. QSFP (non-CMIS) devices will always be impacted by this patch, so the only way to compare performance for true optoe1 devices is between the patched and unpatched drivers. However… For QSFP (non-CMIS) devices, there is not much interesting data beyond page 0. For reads between offset 0 and offset 255, the page checking does not happen. So there is no performance penalty (CMIS or non_CMIS) for data in the first 256 bytes. Bottom Line: In the real world, there should be VERY little accesses beyond byte 255 on QSFP (non-CMIS) devices, so the performance impact on those devices doesn’t really matter. Don’t bother measuring it. So… I would like to see the optoe3 vs optoe1 performance impact on a large number of small reads of page 11, rather than on a single read of a large block. Don From: Paul Menzel ***@***.*** Sent: Monday, March 13, 2023 2:10 PM To: sonic-net/sonic-linux-kernel ***@***.***> Cc: donboll ***@***.***>; Mention ***@***.***> Subject: Re: [sonic-net/sonic-linux-kernel] Read ID register for optoe1 to find pageable bit in optoe driver (PR #308) @donboll <https://github.com/donboll> , could you please review the series? — Reply to this email directly, view it on GitHub <#308 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD6VCJ4ZDUV2PAX3CQAFQETW36EKHANCNFSM6AAAAAAVW7OKEQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>