-
Notifications
You must be signed in to change notification settings - Fork 52
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
fby4: sd: Support PLDM host power control #1344
fby4: sd: Support PLDM host power control #1344
Conversation
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
059891a
to
11d3fce
Compare
@EliHuang-wiwynn has updated the pull request. You must reimport the pull request before landing. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
LOG_ERR("Unsupport host power control effecter state, (%d)", | ||
host_power_state->effecter_state); | ||
*completion_code_p = PLDM_ERROR_INVALID_DATA; | ||
break; |
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 break needed for default case.
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.
Removed, thank you for correction
} | ||
break; | ||
default: | ||
LOG_ERR("Unsupport host power control effecter state, (%d)", |
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.
Unsupported
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.
corrected
*resp_len = 1; | ||
|
||
if (req_p->composite_effecter_count != PLDM_PLATFORM_OEM_HOST_POWER_CTRL_EFFECTER_STATE_FIELD_COUNT) { | ||
LOG_ERR("Unsupport host power control effecter count, (%d)", req_p->composite_effecter_count); |
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.
Unsupported
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.
corrected
#define MAX_STATE_EFFECTER_IDX 168 | ||
#define MAX_STATE_EFFECTER_IDX 169 | ||
|
||
#define PLDM_PLATFORM_HOST_PWR_CTRL_DEFAULT 0xFF |
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.
Lets use consistent prefix in code, since other places its plat_pldm, so let use PLAT_PLDM_
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.
Revised, thank you for correction.
@@ -17,7 +17,11 @@ | |||
#ifndef PLAT_PLDM_MONITOR_H | |||
#define PLAT_PLDM_MONITOR_H | |||
|
|||
#define MAX_STATE_EFFECTER_IDX 168 | |||
#define MAX_STATE_EFFECTER_IDX 169 |
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.
Let's use PLAT_PLDM_ as prefix.
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.
Added, thank you for the suggestion.
#include "plat_sensor_table.h" | ||
#include "plat_pldm_monitor.h" | ||
|
||
LOG_MODULE_REGISTER(plat_pldm_monitor); | ||
|
||
#define POWER_ON_BUTTON_MSEC 1000 |
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.
Lets use PLAT_PLDM as prefix to keep it consistent through out 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.
Revised, thank you for the suggestion.
|
||
uint8_t plat_pldm_host_button_sequence(const uint8_t *power_sequence, uint16_t pressing_interval) | ||
{ | ||
uint8_t sts_cnt = 3; |
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.
Lets define a macro and use that here and in power sequence and reset sequence definition rather than using arbitrary numbers at multiple places.
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.
Revised, thank you for the suggestion.
11d3fce
to
b09b26f
Compare
@EliHuang-wiwynn has updated the pull request. You must reimport the pull request before landing. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
b09b26f
to
4da266b
Compare
@EliHuang-wiwynn has updated the pull request. You must reimport the pull request before landing. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
4da266b
to
0f22dfb
Compare
@EliHuang-wiwynn has updated the pull request. You must reimport the pull request before landing. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Description Implement pldm host power control handler Motivation In Yosemite4, BMC will communicate with BIC by PLDM command. This change PLDM Support host power control by setting cpld through i2c Test Plan: - host power on: pass - host power off: pass - host power cycle: pass - host power reset: pass - host graceful shutdown: pass Test Log: - host power on pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x1 pldmtool: Tx: 81 02 39 00 00 01 00 01 pldmtool: Rx: 01 02 39 00 - host power off pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x2 pldmtool: Tx: 81 02 39 00 00 01 00 02 pldmtool: Rx: 01 02 39 00 - host power cycle pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x3 pldmtool: Tx: 81 02 39 00 00 01 00 03 pldmtool: Rx: 01 02 39 00 - host power reset pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x4 pldmtool: Tx: 81 02 39 00 00 01 00 04 pldmtool: Rx: 01 02 39 00 - host power reset pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x4 pldmtool: Tx: 81 02 39 00 00 01 00 04 pldmtool: Rx: 01 02 39 00 - host graceful shutdown pldmtool raw -d 0x80 0x02 0x39 0x0 0x0 0x1 0x0 0x5 pldmtool: Tx: 81 02 39 00 00 01 00 05 pldmtool: Rx: 01 02 39 00
0f22dfb
to
67e74f6
Compare
@EliHuang-wiwynn has updated the pull request. You must reimport the pull request before landing. |
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This pull request has been merged in 36196a2. |
Summary:
Description
Implement pldm host power control handler
Motivation
In Yosemite4, BMC will communicate with BIC by PLDM command. This change PLDM Support host power control by setting cpld through i2c
Test Plan:
Test Log:
pldmtool: Rx: 01 02 39 00
pldmtool: Rx: 01 02 39 00
pldmtool: Rx: 01 02 39 00
pldmtool: Rx: 01 02 39 00
pldmtool: Rx: 01 02 39 00
pldmtool: Rx: 01 02 39 00