Darwin48V: Sync PM, led_manager, and weutil configs#357
Closed
aalamsi22 wants to merge 2 commits into
Closed
Conversation
Contributor
|
@joancaneus has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
Contributor
|
@joancaneus has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
@aalamsi22 has updated the pull request. You must reimport the pull request before landing. |
Contributor
|
@joancaneus has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
|
@joancaneus merged this pull request in 6ecc52f. |
joywu-coder
pushed a commit
to joywu-coder/fboss
that referenced
this pull request
May 26, 2025
Summary:
# Description
## Platform manager
### Port leds support
Added port leds support using scd-leds-darwin. Requires new bsp. See PR containing the new bsp changes for more details on testing.
### Ensure write-protect (WP) is set prior to initializing the PSUs.
This is required for the pmbus kernel driver to initialize telemtry data on unsupported commands by the hardware. Enabling WP causes the driver to not consider the status register, which reports unsupported command errors, therefore preventing the issue where certain telemetry data points are omitted (in3_input, fan1_input, fan2_input.).
The default value of WP is enabled, however this issue arises when transitioning from an OS that utilizes a driver that disables WP. Therefore, the problem with the missing endpoints occurs and is only recovered manually or by powering off/on the PSUs.
```
"regOffset": 16, -> 0x10 (WP register)
"ioBuf": [-128]. -> 0x80 (Write protect enabled)
```
## Led manager config
No logical changes, config clean up.
## Weutil config
Remove weutil support for fanspinner because it is not supported. This is because it lacks certain fields required by the Meta EEPROM format (namely, the Product Production State).
Pull Request resolved: facebook#357
Reviewed By: tao-ren
Differential Revision: D71938094
Pulled By: joancaneus
fbshipit-source-id: c61bea591c4eee956213f73569c99a814a62be90
anna-nexthop
added a commit
to nexthop-ai/fboss
that referenced
this pull request
Feb 3, 2026
…ecking (facebook#357) ## Summary Improve the benchmark test organization by splitting benchmarks into T1, T2, and additional categories and add binary existence checks since some (and sometimes all) benchmark binaries are not guaranteed to be installed on the machine under test. ### Added pytest for configuration validation Created `fboss/oss/hw_benchmark_tests/test_benchmark_conf_files.py` to validate that T1, T2, and additional benchmark configuration files don't have overlaps. The test uses itertools.combinations to automatically generate all pairwise comparisons and is integrated into CMake/ctest. ### Corrected benchmark organization Moved benchmarks that were previously in both T1 and T2 to be T2-only: - sai_ecmp_shrink_with_competing_route_updates_speed-sai_impl - sai_fsw_scale_route_add_speed-sai_impl - sai_init_and_exit_100Gx100G-sai_impl - sai_switch_reachability_change_speed-sai_impl This ensures each benchmark appears in exactly one configuration file, making the organization clearer and easier to maintain. ### Removed all_benchmarks.conf Deleted the all_benchmarks.conf file as it's no longer needed. The default behavior now dynamically combines T1, T2, and additional benchmarks, providing a single source of truth and eliminating duplication. ### Updated documentation - Updated `docs/docs/manuals/run_agent_benchmarks.md` to show the new additional tests option. ## Testing - All pre-commit hooks pass - Pytest validates no overlaps between configuration files **TODO** pytest module installed in Dockerfile, but still fails GHA??? - Test is integrated into CMake and will run with ctest - validated the documentation changes locally: <img width="777" height="586" alt="image" src="https://github.com/user-attachments/assets/09e6230b-10db-4f80-9e2b-395886e906eb" /> ## Related - Jira: NOS-4000
raghav-nexthop
pushed a commit
to nexthop-ai/fboss
that referenced
this pull request
Feb 6, 2026
…ecking (facebook#357) ## Summary Improve the benchmark test organization by splitting benchmarks into T1, T2, and additional categories and add binary existence checks since some (and sometimes all) benchmark binaries are not guaranteed to be installed on the machine under test. ### Added pytest for configuration validation Created `fboss/oss/hw_benchmark_tests/test_benchmark_conf_files.py` to validate that T1, T2, and additional benchmark configuration files don't have overlaps. The test uses itertools.combinations to automatically generate all pairwise comparisons and is integrated into CMake/ctest. ### Corrected benchmark organization Moved benchmarks that were previously in both T1 and T2 to be T2-only: - sai_ecmp_shrink_with_competing_route_updates_speed-sai_impl - sai_fsw_scale_route_add_speed-sai_impl - sai_init_and_exit_100Gx100G-sai_impl - sai_switch_reachability_change_speed-sai_impl This ensures each benchmark appears in exactly one configuration file, making the organization clearer and easier to maintain. ### Removed all_benchmarks.conf Deleted the all_benchmarks.conf file as it's no longer needed. The default behavior now dynamically combines T1, T2, and additional benchmarks, providing a single source of truth and eliminating duplication. ### Updated documentation - Updated `docs/docs/manuals/run_agent_benchmarks.md` to show the new additional tests option. ## Testing - All pre-commit hooks pass - Pytest validates no overlaps between configuration files **TODO** pytest module installed in Dockerfile, but still fails GHA??? - Test is integrated into CMake and will run with ctest - validated the documentation changes locally: <img width="777" height="586" alt="image" src="https://github.com/user-attachments/assets/09e6230b-10db-4f80-9e2b-395886e906eb" /> ## Related - Jira: NOS-4000
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Platform manager
Port leds support
Added port leds support using scd-leds-darwin. Requires new bsp. See PR containing the new bsp changes for more details on testing.
Ensure write-protect (WP) is set prior to initializing the PSUs.
This is required for the pmbus kernel driver to initialize telemtry data on unsupported commands by the hardware. Enabling WP causes the driver to not consider the status register, which reports unsupported command errors, therefore preventing the issue where certain telemetry data points are omitted (in3_input, fan1_input, fan2_input.).
The default value of WP is enabled, however this issue arises when transitioning from an OS that utilizes a driver that disables WP. Therefore, the problem with the missing endpoints occurs and is only recovered manually or by powering off/on the PSUs.
Led manager config
No logical changes, config clean up.
Weutil config
Remove weutil support for fanspinner because it is not supported. This is because it lacks certain fields required by the Meta EEPROM format (namely, the Product Production State).