-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Generalizing config.bcm support for BRCM silicons #699
Conversation
@gechiang , can you take a look? |
``` | ||
Nov 3 09:19:25.373964 2020 sonic INFO syncd#supervisord: syncd Merging | ||
/usr/share/sonic/hwsku/td3-ix8a-bwde-48x25G+8x100G.config.bcm with | ||
/usr/share/sonic/device/x86_64-broadcom_common/x86_64-broadcom_b77 |
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.
What is the difference between the b77 and b87 version of the common td3 config.bcm? from chip_id you can only tell if it is TD3/TH/TD2/... etc but how can you use that to select which one of the TD3 to use? Can you elaborate on this?
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 b77 and b87 is different TD3 model which is the upper 3 nibbles of the switching silicon's device ID.
-
We use the CHIP ID to select the common config folder. FYI, see the mention in HLD :
In the config_syncd_bcm() function, it will load the common silicon config.bcm file by looking for a *.bcm file in the /usr/share/sonic/device/x86_64-broadcom_common/x86_64-broadcom_{$chip_id} folder where the $chip_id is the upper 3 nibbles of the switching silicon's device ID as listed in SDK/include/soc/devids.h. -
I will fix the the typo as the following.
Then, merge common broadcom-sonic-{$chip_name}.config.bcm file with the existing platform specific config.bcm file in which the duplicate configuration entries in the platform specific file will override entries in the common broadcom-sonic-{$chip_name}.config.bcm file.
- Check the syslog to make sure the common config merged to | ||
config.bcm successfully | ||
- Check the final merged config.bcm dump from show tech dump | ||
|
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.
Do you have plans to extract those "common" soc properties from each of the platform specific config.bcm file into the common config.bcm file?
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.
No, we don't have the plans to merge anything common from platform specific config.bcm to common config.bcm, since It will break our rule. In the design, when we find the same common config property existing on both platform specific and common specific. We will keep the original config in platform specific without merging it.
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.
if a platform have multiple brcm asics, is it handled?
|-- x86_64-broadcom_b87-- broadcom-sonic-td3.config.bcm | ||
|-- x86_64-broadcom_b96-- broadcom-sonic-th.config.bcm | ||
|-- x86_64-broadcom_b97-- broadcom-sonic-th2.config.bcm | ||
|-- x86_64-broadcom_b98-- broadcom-sonic-th3.config.bcm |
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.
How about the dnx family of chips, e.g. Jericho2?
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 common config of JR2 is not created so far. The framework will not handle this.
At the community review session we raised a few questions which require you to update with clarifications. Here are the set of items I noted down:
|
My Reply inline:
[GR] We will update the new section on the enhancement PR after this base line PR merged.
[GS] We will update the Documents on the enhancement PR after this base line PR merged.
[GS] By default the common config feature is disabled, and ODM can enable this feature by touch the |
HLD for per-switching silicon Common config for Broadcom Supported Platforms design