Support for BMC cards based on Aspeed 2720 - Phase 2#26002
Support for BMC cards based on Aspeed 2720 - Phase 2#26002chander-nexthop wants to merge 3 commits intosonic-net:masterfrom
Conversation
files - We dont intent to use obmc-console, so remove the same from platform packages - Removed the remnant setup.py Signed-off-by: Chandrasekaran Swaminathan <[email protected]>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Chandrasekaran Swaminathan <[email protected]>
systemd services. This way platform specific changes for the same made to build_debian script cn be removed. 2. Fix the watchdog logic. Add a new systemd service that will pet the watchdog every 60 seconds (after arming it for 180 seconds) 3. Differentiate between user issued reboot and watchdog reset 4. Create a script called platform/aspeed/install-sonic-to-emmc.sh which can be used to burn the image to emmc from OpenBMC Signed-off-by: Chandrasekaran Swaminathan <[email protected]>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
yxieca
left a comment
yxieca
left a comment
There was a problem hiding this comment.
Code Review
This PR is a cleanup/refactor for the Aspeed BMC platform in SONiC. Here's a breakdown:
Changes Summary
1. Platform services → proper Debian package ✅ Good
- Moves scripts/services from ad-hoc
build_debian.shinstallation into a properaspeed-platform-servicesDebian package (.mk,debian/scaffolding,rules.mk,one-image.mk) - This is the right pattern — proper packaging vs. raw file copies in build scripts
2. New watchdog keepalive daemon (watchdog-keepalive.sh)
- 180s timeout, 60s keepalive interval
- Custom log rotation (5 files max)
- Uses
watchdogutil arm/disarmfor setup/teardown
3. Watchdog platform API refactor (both AST2700 EVB and NextHop)
- Removes
self.armedtracking in favor of sysfs state (is_armed()reads sysfsstate) - Removes
_close_watchdog()(magic 'V' close) — replaces with_disablewatchdog()using ioctlWDIOS_DISABLECARD - Adds proper ioctl methods:
_enablewatchdog,_keepalive,_settimeout,_gettimeout,_gettimeleft arm()now caps at 300s max — is this intentional? Some use cases may need longer timeouts
4. Reboot cause logic improvement (both chassis.py files)
WDIOF_CARDRESETnow checks for software reboot cause file before defaulting to watchdog- Previously always returned
NON_HARDWARE, now returnsWATCHDOGwhen no software cause found — good fix
5. Removed BMC component from platform.json (both EVB and NextHop)
- Removes "BMC" from components list — why? Is BMC FW update not supported via
fwutil?
6. Removed obmc-console configs (both EVB and NextHop)
- Deleted
server.tty.conffiles and related install logic — are these now managed elsewhere (OpenBMC side)?
7. USB network init fix (NextHop)
- Hardcodes UDC name to
12021000.usb-vhub:p1instead of auto-detecting first available — could this break on different AST2700 boards?
8. New eMMC installer script (install-sonic-to-emmc.sh)
- Installs SONiC from OpenBMC to eMMC with U-Boot env setup
- Has commented-out alternatives and hardcoded
BOOTCONF="nexthop-b27-r0"— should these be configurable rather than requiring source edits?
Issues
1. Duplicate code across EVB and NextHop
chassis.pyreboot cause changes are identical in bothast2700/andnexthop/common/watchdog.pychanges are identical in both- Should this be a shared base class?
2. Commented-out code left in
build-emmc-image-installer.sh:#EMMC_SIZE_MB=7168install-sonic-to-emmc.sh:#BOOTCONF="ast2700-evb"- These should be removed or made configurable
3. watchdog-keepalive.sh logs every 60s
echo "Watchdog keepalive sent"on every iteration generates ~1440 log lines/day — consider removing or making it periodic (every Nth keepalive)
4. Deleted ast2700/setup.py — is the platform wheel now built differently, or was this unused?
5. PR title is vague — "Address pending review comments and some cleanup" doesn't convey the scope. This has significant changes (new Debian package, watchdog refactor, reboot cause fix, eMMC installer).
This platform itself is a BMC and makes little sense to have a BMC component within it was the point Judy made in the code review. That made sense and hence removed it.
We no longer use obmc-console, instead we have moved to use SONiC's native consutil service and hence removed this.
This is under the Nexthop platform module and is specific to our card. Will not affect the rest.
Fair point. As these scripts are helper scripts (not part of the image build) I didn't make them configurable. If we really need that I can make it as such. Do let me know.
If we make this a shared base class, then we will need a new intermediate python wheel package. Also, chassis.py will change for nexthop card to accommodate eeprom and other features. As of today there are same, but expected to diverge.
Eval board ships with a 8G eMMC whereas Nexthop card will ship with a 32G (but 10G due to pSLC). To catrer
Thanks for pointing out, Will fix.
yes it was unused.
Will fix the title. |
chander-nexthop
changed the title
|
|
||
| [Service] | ||
| Type=simple | ||
| ExecStart=/usr/bin/watchdog-keepalive.sh |
There was a problem hiding this comment.
do we need infra change to copy the watchdog script from platform/aspeed/aspeed-platform-services/scripts/watchdog-keepalive.sh to /usr/bin -- as I see some line relating to this removed in build_debian.sh ?
4 participants
files
Why I did it
Minor review comments were pending even as the base pull request to add sonic support to aspeed ast2720 based boards merged. Plus more refactoring and bug fixing were needed.
Work item tracking
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)