Skip to content
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

main_v2 - Optimize Crater Lake - Fix Warnings #224

Closed
wants to merge 1 commit into from

Conversation

DelphineChiu
Copy link

Summary:

  • Fix all warnings in CL layer and common layer.
  • Refactor ME portion APIs

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 1, 2022
@facebook-github-bot
Copy link
Contributor

@GoldenBug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

common/dev/adc.c Outdated
@@ -74,19 +74,30 @@ bool adc_init()
return true;
}

int bat_3v_set_gpio(uint8_t sensor_num, void *arg)
{
SET_GPIO_VALUE_CFG *cfg = arg;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check to make sure arg != NULL

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

common/dev/adc.c Outdated
Comment on lines 121 to 132
*reading = (float)val * cfg->arg0 / cfg->arg1 / 1000;
cfg->cache = *reading;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check reading != NULL before use

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

printf("Sensor number %x read fail\n", sensor_num);
return false;
}
*reading = val;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check reading != NULL before use.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

return GUID_OUT_OF_RANGE;
}

memcpy(&entry->config, &guid_config[entry->config.dev_id], sizeof(EEPROM_CFG));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check that entry != NULL when passed in

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @GoldenBug
The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .


struct isl69260_page_data isl69260_page_data_args[] = { [0] = { 0x0, 0x0 }, [1] = { 0x0, 0x1 } };

int isl69260_switch_page(uint8_t sensor_num, void *args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check args != NULL

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

} else {
// energy / unit time
*reading = ((float)diff_energy / (float)diff_time * 0.06103515625);
free(read_buf);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SAFE_FREE

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

}
}

bool peci_sensor_read(uint8_t sensor_num, float *reading)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check reading != NULL

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

*reading = (float)val;
cfg->cache = *reading;
cfg->cache_status = SENSOR_READ_SUCCESS;
free(read_buf);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SAFE_FREE

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .


#define RETRY 5

bool tmp75_read(uint8_t sensor_num, float *reading)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check reading != NULL

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optimization code is based on v2022.07.01. After that, the main branch has been phased in some
commits related to sensor monitoring. Those changes will be phased in main_v2 in the other PRs. This
problem will be solved in those PRs.
Please check #223 .

ipmi_message.InF_source = source_inft;
ipmi_message.InF_target = target_inft;
ipmi_message.data_len = data_len;
if (data_len != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& data != NULL ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and do the forced update.
Please review it. Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and do the forced update.
Please review it. Thanks.

@facebook-github-bot
Copy link
Contributor

@DelphineChiu has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@DelphineChiu has updated the pull request. You must reimport the pull request before landing.

@DelphineChiu
Copy link
Author

Dependency: #214

@facebook-github-bot
Copy link
Contributor

@GoldenBug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

return;
}
gpio_num = (group * GPIO_GROUP_SIZE) + pins;

if (gpio_cfg[gpio_num].int_cb == NULL) {
printk("CB func pointer NULL for gpio num %d\n", gpio_num);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CB -> Callback

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @GoldenBug
We have fixed this and done the forced update.
Please review it. Thanks.

if (me_msg == NULL) {
printf("[%s] Failed to allocate memory\n", __func__);
if ((me_fw_mode != ME_FW_RECOVERY) && (me_fw_mode != ME_FW_RESTORE)) {
printf("Not support the ME frimware mode 0x%x setting", me_fw_mode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Unsupported ME firmware mode 0x%x setting"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and done the forced update.
Please review it. Thanks.

data_len, data);
ret = ipmb_read(me_msg, IPMB_inf_index_map[me_msg->InF_target]);
if (ret != IPMB_ERROR_SUCCESS) {
printf("Failed to set ME frimware mode to 0x%x, ret: 0x%x\n", me_fw_mode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ME firmware

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and done the forced update.
Please review it. Thanks.

}
}
if (i == retry) {
printf("Failed to set ME frimware mode to 0x%x, retry time: %d\n", me_fw_mode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ME firmware

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and done the forced update.
Please review it. Thanks.

@@ -42,8 +48,7 @@ void set_sys_status()

void main(void)
{
uint8_t proj_stage = (FIRMWARE_REVISION_1 & 0xf0) >> 4;
printk("Hello, wellcome to yv35 craterlake %x%x.%x.%x\n", BIC_FW_YEAR_MSB, BIC_FW_YEAR_LSB,
printf("Hello, wellcome to yv35 craterlake %x%x.%x.%x\n", BIC_FW_YEAR_MSB, BIC_FW_YEAR_LSB,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

welcome

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have fixed this and done the forced update.
Please review it. Thanks.

Summary:
    - Fix all warnings in CL layer and common layer.
    - Refactor ME portion APIs
@facebook-github-bot
Copy link
Contributor

@DelphineChiu has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@GoldenBug has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Apr 12, 2022
Summary:
- Fix all warnings in CL layer and common layer.
- Refactor ME portion APIs

Pull Request resolved: #224

Reviewed By: garnermic

Differential Revision: D35319956

Pulled By: GoldenBug

fbshipit-source-id: c5ec0e0806bd40c5a92ac1c9c8792446dd4cd88d
@GoldenBug GoldenBug closed this Apr 13, 2022
facebook-github-bot pushed a commit that referenced this pull request Apr 26, 2022
Summary:
- Fix all warnings in CL layer and common layer.
- Refactor ME portion APIs

Pull Request resolved: #224

Reviewed By: garnermic

Differential Revision: D35941608

Pulled By: GoldenBug

fbshipit-source-id: 5537e193e18801062cd0acc728bd1c6466aeebbb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants