-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[platform/broadcom] Silverstone update platform i2c driver #3607
Conversation
retest vsimage please |
retest vs please |
Regression test with show interfaceNOTE: Due to limitation, QSFP-DD module only plug at first two ports, others are QSFP28
|
retest vs please |
1 similar comment
retest vs please |
@@ -0,0 +1,846 @@ | |||
// SPDX-License-Identifier: GPL-2.0 |
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.
https://elixir.bootlin.com/linux/v4.1/source/drivers/i2c/busses/i2c-ocores.c
why not use linux kernel source code?
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.
Hello,
The reason we keep it as a separate are:
- The original i2c-ocores in sonic kernel version 4.9.x is not support polling mode. We use the codes based on kernel 5.1 to support this feature.
- We add a workaround to fix read error on some SFP loopback module.
If only the first reason, we can backport the feature from kernel 5.1 to current sonic-linux-kernel, but combined with the 2nd reason, we decide to maintain the codes as a platform-specific.
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.
why do we need this workaround? We add a workaround to fix read error on some SFP loopback module.
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.
In some SFP module case.
When reading EEPROM, the i2c core detects the arbitration lost(AL) error and stop the i2c transfer. The AL randomly occurs on the device.
To make the device readable, the limited retry is added to the ocore FSM. When AL detected, it restarts the i2c transfer again until retry used up.
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.
this seems like a generic fix, can you patch the kernel in sonic-linux-kernel?
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.
Yes, I will create a PR to patch i2c-ocores.c in sonic-linux-kernel to support polling mode.
For the workaround, we found that it was a bug in the FPGA firmware and fixed with new firmware, so it will not include in this patch.
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.
thanks. I will review your sonic-linux-kernel patch and merge this pr after kernel patch has been fixed.
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.
The kernel patch has been introduced here sonic-net/sonic-linux-kernel#135.
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.
The driver is updated to use kernel's i2c-ocores module.
@@ -0,0 +1,22 @@ | |||
/* SPDX-License-Identifier: GPL-2.0 */ |
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.
kernel existing code.
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.
As we keep the own maintain version of i2c-ocores, this file is needed for module compile.
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.
This will be removed after the sonic-net/sonic-linux-kernel#135 is merged.
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.
The driver is updated to use kernel's i2c-ocores module.
retest broadcom please |
*To support FPGA firmware version 1.5 or later.
01190a6
to
3c0dcb6
Compare
84b4a98
to
0d3251e
Compare
I have rebased the changes onto the master and apply 2 changes
|
…atically (#20573) #### Why I did it src/sonic-utilities ``` * 32e6de7f - (HEAD -> master, origin/master, origin/HEAD) Update DB migrator script to next branch 202505 (#3664) (3 days ago) [Kumaresh Perumal] * 8d00aba7 - (origin/test_202411_20, origin/test_202411_100, origin/kperumal_202411, origin/202411) Fix save_file command in generate_dump (#3647) (4 days ago) [DavidZagury] * 038aaa5f - Revert "Speed up route_check script" (#3653) (4 days ago) [xwjiang-ms] * 032a0e03 - [config]Restrict YANG validation to Golden Config (#3656) (4 days ago) [jingwenxie] * 47b79bea - Update gcu_field_operation_validators.conf.json with Arista-7260CX3-D108C10 (#3650) (4 days ago) [Dev Ojha] * 845f46ef - Update PR checker pipeline to point to bookworm (#3651) (4 days ago) [bingwang-ms] * 28e645f4 - [show_tech] Filter out a list of commands in generate_dump script which are not applicable to Supervisor (#3646) (5 days ago) [Marty Y. Lok] * b767cb86 - [cisco|express-boot]: Add support for cisco express boot in sonic-utilities (#3056) (6 days ago) [jhli-cisco] * f6b9912c - Fix show command column change issue (#3645) (8 days ago) [Feng-msft] * 9cd6c8de - Debug dump utility dash objects update (#3387) (8 days ago) [Gagan Punathil Ellath] * f705c456 - Validate interface name length in CLI (#3580) (8 days ago) [Stepan Blyshchak] * 3d78cb16 - [Mellanox] Update the save_file command in generate_dump to handle folders (#3631) (13 days ago) [DavidZagury] * 5086a43b - Fix counterpoll show for ENI Counters (#3618) (13 days ago) [Vivek] * 70f81397 - Extra delay between running firmware and updating firmware version (#3610) (2 weeks ago) [Stephen Sun] * 636f7f1d - Add port FEC BER show changes (#3607) (2 weeks ago) [vincentpcng] * 5e60031b - Supports FRR-VRRP configuration (#2949) (3 weeks ago) [Philo] * 9bd37dc6 - [ARM64][Warm-Reboot]: Get boot arguments from uboot environment (#3613) (3 weeks ago) [Parthiv Shah] * 171b16c2 - platform/innovium renaming to platform/marvell-teralynx (#3474) (3 weeks ago) [krismarvell] * 2cbfcc91 - [Mellanox] Add Mellanox-SN5600-C224O8 to gcu_field.json (#3612) (4 weeks ago) [noaOrMlnx] * a8f7bde0 - PVST Feature commit (#3567) (4 weeks ago) [Divya Kumaran Chandralekha] * 0e188076 - Refresh dump file modify timestamp before cleanup call (#3595) (4 weeks ago) [anamehra] * 8f3c5ef5 - [warm-reboot] Add support to pass extra kernel cmdline parameters (#3192) (4 weeks ago) [Pavan Naregundi] * 96774477 - Run yang validation in db migrator (#3102) (4 weeks ago) [ganglv] * 9708f52b - [Mellanox] Update C256X1 SKU to be C256S1 in gcu_field.json (#3611) (4 weeks ago) [noaOrMlnx] * 093ed4aa - [SPM] Add logic to disable the feature before stopping it and enabling it before starting (#3344) (4 weeks ago) [Vadym Hlushko] * 7d013dff - Fix slash in path. (#3573) (5 weeks ago) [Xincun Li] * 0af4386e - Consolidate the get running config way. (#3585) (5 weeks ago) [Xincun Li] * 964b4895 - Fix for integer overflow of counter value if its too large (#3596) (5 weeks ago) [harjotsinghpawra] * 3354d08a - [config] Bypass standard input for reload (#3597) (5 weeks ago) [jingwenxie] * 7cbcfda5 - Speed up route_check script (#3544) (5 weeks ago) [Deepak Singhal] * 329fc223 - Add support of the pensando-dpu platform to generate-dump utility. (#3557) (6 weeks ago) [BHUKYA SIDDHU] * 0ae2ec1e - Add CLI for bmp configdb entity Enable/Disable (#3286) (6 weeks ago) [Feng-msft] * 5b37ee6c - Vnet_route_check TCP socket for DB connection. (#3578) (6 weeks ago) [siqbal1986] * d64a90a0 - Adds logic to get default disk and check disk type (#3399) (6 weeks ago) [Ashwin Srinivasan] * b2b97340 - [Banner] Added CLI commands to configure Banner and display current configuration (#3021) (6 weeks ago) [Sviatoslav Boichuk] * d0aa94a1 - Revert "Record and warn tables which not covered by YANG (#3583)" (#3588) (7 weeks ago) [jingwenxie] * aeda86a1 - Record and warn tables which not covered by YANG (#3583) (7 weeks ago) [jingwenxie] * dd34d7c4 - Revert "Skip default lanes dup check (#3489)" (#3572) (7 weeks ago) [Xincun Li] * 6c8007fa - [Mellanox]Remove deprecated sdk sniffer cli and collect sdk dump in show techsupport (#3491) (7 weeks ago) [Yuanzhe] * 89bb87ad - Add YANG validation for config reload if file is given (#3576) (7 weeks ago) [jingwenxie] ``` #### How I did it #### How to verify it #### Description for the changelog
- What I did
Update the Silverstone i2c driver to work with new FPGA i2c firmware version 1.5.
The new FPGA firmware improves i2c stability. This feature is mandatory for the current Silverstone machine which shipped with FPGA firmware version 1.5 by default.
- How I did it
- How to verify it
Test with Silverstone FPGA firmware version 1.5 and see the sfputil command works normally.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)