main_v2 - fby3.5: cl: new sensor reading structure and support MP5990 sensor reading by DelphineChiu · Pull Request #229 · facebook/OpenBIC

DelphineChiu · 2022-04-15T07:43:37Z

Summary:

common: Phase in new sensor reading structure with README file
fby3.5: cl: Support MP5990 sensor reading

facebook-github-bot · 2022-04-15T17:22:25Z

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

GoldenBug · 2022-04-15T17:41:18Z

+uint8_t adm1278_read(uint8_t sensor_num, int *reading)
+{
+	if ((reading == NULL) ||
+	    (sensor_config[sensor_config_index_map[sensor_num]].init_args == NULL)) {


Can we use a temporary variable for this in this function to improve readability?

Something like:
sensor_cfg config = sensor_config[sensor_config_index_map[sensor_num]];

and then use the temporary variable everywhere that ensor_config[sensor_config_index_map[sensor_num]] is used

The stack memory is limited to BIC.
The size of sensor_cfg is 60 bytes.
So, we propose not to define a temporary variable.

GoldenBug · 2022-04-15T17:44:06Z

 {
-	snr_cfg *cfg = &sensor_config[sensor_config_index_map[sensor_num]];
-	float val = 0;
+	if (!sensor_config[sensor_config_index_map[sensor_num]].init_args) {


Same thing here, can we use a temporary variable for sensor_config[sensor_config_index_map[sensor_num]] so we can make it more readable

The stack memory is limited to BIC.
The size of sensor_cfg is 60 bytes.
So, we propose not to define a temporary variable.

GoldenBug · 2022-04-15T17:45:55Z

+	return 0;
+}
+
+uint8_t adm1278_read(uint8_t sensor_num, int *reading)


We probably want to check to make sure sensor_num is within the allowed range

0 <= sensor_num < SENSOR_NUM_MAX ?

We have fixed this and do the forced update.
The sensor_num is unsigned type so only judge if it's less than MAX_NUM_SENSORS.
Please review it. Thanks.

GoldenBug · 2022-04-15T17:46:21Z

 {
-	snr_cfg *cfg = &sensor_config[sensor_config_index_map[sensor_num]];
-	float val = 0;
+	if (!sensor_config[sensor_config_index_map[sensor_num]].init_args) {


We probably want to check to make sure sensor_num is within the allowed range

0 <= sensor_num < SENSOR_NUM_MAX ?

We have fixed this and do the forced update.
The sensor_num is unsigned type so only judge if it's less than MAX_NUM_SENSORS.
Please review it. Thanks.

GoldenBug · 2022-04-15T17:49:23Z

+		return false;
+	}
+
+	int err, retval;


It looks like we don't need to use 2 different variables here. Can we just use 1 of them for adc_channel_setup and adc_read

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

GoldenBug · 2022-04-15T18:26:37Z


-	msg.bus = cfg->port;
-	msg.target_addr = cfg->target_addr;
+	msg.bus = sensor_config[sensor_config_index_map[sensor_num]].port;


make sensor_config[sensor_config_index_map[sensor_num]] a temp variable to make code more readable

The stack memory is limited to BIC.
The size of sensor_cfg is 60 bytes.
So, we propose not to define a temporary variable.

GoldenBug · 2022-04-15T18:27:00Z

+#include "pmbus.h"
+#include "util_pmbus.h"
+
+uint8_t tps53689_read(uint8_t sensor_num, int *reading)


Make sure sensor_num is in valid range

We have fixed this and do the forced update.
The sensor_num is unsigned type so only judge if it's less than MAX_NUM_SENSORS.
Please review it. Thanks.

GoldenBug · 2022-04-15T18:27:28Z

+	I2C_MSG msg;
+	memset(sval, 0, sizeof(sensor_val));
+
+	msg.bus = sensor_config[sensor_config_index_map[sensor_num]].port;


make sensor_config[sensor_config_index_map[sensor_num]] a temporary variable

The stack memory is limited to BIC.
The size of sensor_cfg is 60 bytes.
So, we propose not to define a temporary variable.

GoldenBug · 2022-04-15T18:27:51Z

+#include "util_pmbus.h"
+#include "pmbus.h"
+
+uint8_t xdpe15284_read(uint8_t sensor_num, int *reading)


Make sure sensor_num is in valid range

We have fixed this and do the forced update.
The sensor_num is unsigned type so only judge if it's less than MAX_NUM_SENSORS.
Please review it. Thanks.

GoldenBug · 2022-04-15T18:31:19Z

+	return ret;
+}
+
+bool get_exponent_from_vout_mode(uint8_t sensor_num, float *exponent)


Check exponent is not NULL and that sensor_num is within valid range

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

facebook-github-bot · 2022-04-18T03:22:41Z

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

facebook-github-bot · 2022-04-18T03:24:44Z

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

facebook-github-bot · 2022-04-18T07:59:46Z

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

Summary: - Support MP5990 sensor device Test Plan: 1. Build code: pass 2. [MPS] Check GPIOA7 and the MP5990 configure register(38h and 46h) 3. [MPS] Get MP5990 sensor reading: pass 4. [ADI] Get ADM1278 sensor reading: pass Log: 1. Class type: class-1, 1ou present status: false, 2ou present status: true, board revision: EVT3(EFUSE) root@bmc-oob:/root# bic-util slot1 --get_gpio|grep "HSC_SET_EN_R" 7 HSC_SET_EN_R: 1 root@bmc-oob:/root# bic-util slot1 0x18 0x52 0x5 0x16 0x2 0x38 BF 01 root@bmc-oob:/root# bic-util slot1 0x18 0x52 0x5 0x16 0x2 0x46 46 00 root@bmc-oob:/root# sensor-util slot1|grep HSC HSC Temp (0xE) : 25.00 C | (ok) HSC Input Vol (0x29) : 12.31 Volts | (ok) HSC Output Cur (0x30) : 0.25 Amps | (ok) HSC Input Pwr (0x39) : 0.00 Watts | (ok) 2. Class type: class-1, 1ou present status: false, 2ou present status: false, board revision: EVT3(EFUSE) root@bmc-oob:/root# bic-util slot1 --get_gpio|grep "HSC_SET_EN_R" 7 HSC_SET_EN_R: 0 root@bmc-oob:/root# bic-util slot1 0x18 0x52 0x5 0x16 0x2 0x46 28 00 root@bmc-oob:/root# bic-util slot1 0x18 0x52 0x5 0x16 0x2 0x38 04 01 root@bmc-oob:/root# sensor-util slot1|grep HSC HSC Temp (0xE) : 25.00 C | (ok) HSC Input Vol (0x29) : 12.44 Volts | (ok) HSC Output Cur (0x30) : 0.25 Amps | (ok) HSC Input Pwr (0x39) : 0.00 Watts | (ok) 3. Class type: class-1, 1ou present status: false, 2ou present status:false, board revision: POC root@bmc-oob:/root# sensor-util slot3|grep HSC HSC Temp (0xE) : 27.62 C | (ok) HSC Input Vol (0x29) : 12.00 Volts | (ok) HSC Output Cur (0x30) : 10.52 Amps | (ok) HSC Input Pwr (0x39) : 129.35 Watts | (ok)

facebook-github-bot · 2022-04-18T08:05:36Z

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

facebook-github-bot · 2022-04-18T22:14:21Z

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

… sensor reading (#229) Summary: - common: Phase in new sensor reading structure with README file - fby3.5: cl: Support MP5990 sensor reading Pull Request resolved: #229 Reviewed By: garnermic Differential Revision: D35679332 Pulled By: GoldenBug fbshipit-source-id: 6f645fa4fe561a217483f8b57d073c83ca861bf0

Summary: fby3.5: cl: Fixing warning messages - K_WORK_DELAYABLE_DEFINE needs a callback function with the function parameter type of "struct k_work *". - util_spi.c had a missing header that needed to be included as well as an improperly formatted print statement. - Mark card_type_1ou that is currently unused in Yv3.5 CL as unused to silence compiler warnings. - Initialized status in fall through case. Dependency: #229 Pull Request resolved: #230 Test Plan: - Build code: Pass Reviewed By: zhdaniel12 Differential Revision: D35679334 Pulled By: GoldenBug fbshipit-source-id: 02c91f11c6f6131f215be6cd43e5e47003bfcfe2

DelphineChiu · 2022-04-19T02:49:51Z

close the PR since the codes have been merged

… sensor reading (#229) Summary: - common: Phase in new sensor reading structure with README file - fby3.5: cl: Support MP5990 sensor reading Pull Request resolved: #229 Reviewed By: garnermic Differential Revision: D35941602 Pulled By: GoldenBug fbshipit-source-id: d8ecba9eaea9ba0a2aabac644ecd563f471cf73d

Summary: fby3.5: cl: Fixing warning messages - K_WORK_DELAYABLE_DEFINE needs a callback function with the function parameter type of "struct k_work *". - util_spi.c had a missing header that needed to be included as well as an improperly formatted print statement. - Mark card_type_1ou that is currently unused in Yv3.5 CL as unused to silence compiler warnings. - Initialized status in fall through case. Dependency: #229 Pull Request resolved: #230 Test Plan: - Build code: Pass Reviewed By: garnermic Differential Revision: D35941585 Pulled By: GoldenBug fbshipit-source-id: b6a4753658952a940f93df7cf319e656afedaaa4

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 15, 2022

DelphineChiu mentioned this pull request Apr 15, 2022

main_v2 - fby3.5: cl: Fixing warning messages #230

Closed

GoldenBug suggested changes Apr 15, 2022

View reviewed changes

RenChen-wiwynn force-pushed the Ren/MP5990 branch from a5fbe3d to 77c37cc Compare April 18, 2022 03:22

RenChen-wiwynn force-pushed the Ren/MP5990 branch from 77c37cc to 6324758 Compare April 18, 2022 03:24

RenChen-wiwynn force-pushed the Ren/MP5990 branch from 6324758 to 13a95bb Compare April 18, 2022 07:59

RenChen-wiwynn added 2 commits April 18, 2022 16:05

Support new sensor reading structure

6f8bfc4

RenChen-wiwynn force-pushed the Ren/MP5990 branch from 13a95bb to 92283dd Compare April 18, 2022 08:05

DelphineChiu closed this Apr 19, 2022

RenChen-wiwynn mentioned this pull request Apr 22, 2022

Move sensor monitoring changes from main to main_v2 #223

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

main_v2 - fby3.5: cl: new sensor reading structure and support MP5990 sensor reading#229

main_v2 - fby3.5: cl: new sensor reading structure and support MP5990 sensor reading#229
DelphineChiu wants to merge 2 commits into
facebook:main_v2from
Wiwynn:Ren/MP5990

DelphineChiu commented Apr 15, 2022

facebook-github-bot commented Apr 15, 2022

GoldenBug Apr 15, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

GoldenBug Apr 15, 2022

RenChen-wiwynn Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

DelphineChiu commented Apr 19, 2022

Labels

4 participants

Uh oh!

Conversation

DelphineChiu commented Apr 15, 2022

facebook-github-bot commented Apr 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

facebook-github-bot commented Apr 18, 2022

DelphineChiu commented Apr 19, 2022

Labels

4 participants