-
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
[Arista] Add tuning values for phys on 7280cr3mk #10084
Conversation
Do we have any dependency between the two prs? |
Yes, the values added here can take effect only after the sonic-swss PR (sonic-net/sonic-swss#2158) is merged. |
"line_lanes": [16,17,18,19], | ||
"system_tx_fir_pre2": [1,1], | ||
"system_tx_fir_pre1": [-5,-5], | ||
"system_tx_fir_main": [14,14], | ||
"system_tx_fir_post1": [0,0], | ||
"system_tx_fir_post2": [0,0], | ||
"line_tx_fir_pre2": [0,0,0,0], | ||
"line_tx_fir_pre1": [-1,-1,-1,-1], | ||
"line_tx_fir_main": [13,13,13,13], | ||
"line_tx_fir_post1": [-5,-5,-5,-5], | ||
"line_tx_fir_post2": [0,0,0,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.
The line side SI settings may depend upon the type of optics for some platforms. In that case, Xcvrd should read these values based upon the media (see Ref) and publish to gearbox syncd to program the line side on the gearbox. Can we discuss this scheme in the community?
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.
@prgeor , this gearbox_config.json here is just to provide some initial value like setting tuning values for brcm ASIC in brcm config file. For the dynamic tuning support (based on xcvr type), an idea is to add the support in the future with some extension to the usage of media_settings.json.
Tagging @arlakshm for awareness |
@arlakshm gentle reminder to merge this change when you get a chance |
Hi @prgeor, can you help approve and merge this PR |
Another gentle reminder to consider merging. All checks have passed. This PR has been open for nearly 6 months. |
What I did This change adds support for setting tx tap or tuning values on gearbox ports. It uses the SAI attributes such as SAI_PORT_SERDES_ATTR_TX_FIR_PRE1 to communicate with SAI-based gearbox drivers. For the values, they are provided in the format like "system_tx_fir_pre2": [1,1] for an interface from gearbox_config.json. Why I did it How I verified it We verified that values provided in sonic-net/sonic-buildimage#10084 are set to the chip with this change. Added test to tests/test_gearbox.py. The added test will not pass until the following two changes (which should be merged first) are merged: Support SAI_PORT_ATTR_PORT_SERDES_ID on vs gearbox: sonic-net/sonic-sairedis#1082 Add gearbox taps to vs gearbox_config.json: sonic-net/sonic-buildimage#11480
What I did This change adds support for setting tx tap or tuning values on gearbox ports. It uses the SAI attributes such as SAI_PORT_SERDES_ATTR_TX_FIR_PRE1 to communicate with SAI-based gearbox drivers. For the values, they are provided in the format like "system_tx_fir_pre2": [1,1] for an interface from gearbox_config.json. Why I did it How I verified it We verified that values provided in sonic-net/sonic-buildimage#10084 are set to the chip with this change. Added test to tests/test_gearbox.py. The added test will not pass until the following two changes (which should be merged first) are merged: Support SAI_PORT_ATTR_PORT_SERDES_ID on vs gearbox: sonic-net/sonic-sairedis#1082 Add gearbox taps to vs gearbox_config.json: sonic-net/sonic-buildimage#11480 Updated handling of VRF_VNI mapping and VLAN_VNI mapping for same VNI ID fixed compile issues Updated code for the flow where VRF VNI mapping is processed first followed by VLAN VNI mapping
@prgeor @sujinmkang The sonic-swss change (sonic-net/sonic-swss#2158) that this change depends on has been merged. Can you please help to review this change again? Thanks. |
@lguohan please help merge |
@prgeor , is the swss submodule updated, i see it as a dependency. |
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
Why I did it
This change specifies the tuning values for each lane of the B52 phy chips. These values can be different for different ports. The values being set are under the assumption of optical transceivers. This change depends on the change to sonic-swss: sonic-net/sonic-swss#2158.
How I did it
How to verify it
We verified the values are correctly set on the B52 chips of Arista 7280cr3, by reading them from the debug cli of the B52 driver.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)