17.04.2019 16:03, Guenter Roeck wrote: > On 4/17/19 4:26 AM, Alexander Amelkin wrote: >> Inspecting the OCC sources for P8 reveals that OCC may send >> a special value 0xFFFF to indicate that a sensor read timeout >> has occured, see >> > occurred Yup. A typo. Will fix. > >> https://github.com/open-power/occ/blob/master_p8/src/occ/cmdh/cmdh_fsp_cmds.c#L395 >> >> That situation wasn't handled in the driver. This patch adds invalid >> temp value check for the sensor data format 1 and handles it the same >> way as it is done for the format 2, where EREMOTEIO is reported for >> this case. >> > ETIMEDOUT ? Though that is really a corner case, so I guess both are fine. We just reused the error code used for the same case for format 2 in common.c:309 (inside occ_show_temp_2() function). We thought it would be strange to report different codes for the same case in different format versions. Besides, it's quite a remote I/O error indeed. > >> Signed-off-by: Alexander Soldatov <a.soldatov@xxxxxxxxx> >> Signed-off-by: Alexander Amelkin <a.amelkin@xxxxxxxxx> >> Reviewed-by: Alexander Amelkin <a.amelkin@xxxxxxxxx> >> Cc: Edward A. James <eajames@xxxxxxxxxx> >> Cc: Joel Stanley <joel@xxxxxxxxx> >> --- >> drivers/hwmon/occ/common.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c >> index 4679acb..825631c 100644 >> --- a/drivers/hwmon/occ/common.c >> +++ b/drivers/hwmon/occ/common.c >> @@ -235,6 +235,10 @@ static ssize_t occ_show_temp_1(struct device *dev, >> val = get_unaligned_be16(&temp->sensor_id); >> break; >> case 1: >> + /* If a sensor timed out long enough, > > "timed out" is sufficient. "timed out long enough" is difficult to understand. Agreed. That's a weird wording, but I double-checked before submitting that it was just a copy-paste of the wording from the OCC sources for this case. You're probably right though that it's better to fix it. > >> + OCC returns 0xFFFF for that sensor.*/ > > /* > * This is how multi-line comments look like > */ > > Please run checkpatch on your patches. Mea culpa. Didn't check. Will fix tomorrow. > > Thanks, > Guenter > Thanks. With best regards, Alexander Amelkin, Leading BMC Software Engineer, YADRO https://yadro.com
Attachment:
signature.asc
Description: OpenPGP digital signature
