hwmon: (occ): Port fixes to 4.13
cover hwmon: (occ): Port fixes to 4.13
Two fixes that we're missing in 4.13.
Eddie James (1):
hwmon (occ): Fix integer overflow in multiplication
Edward A. James (1):
hwmon (occ): Remove temperature fault attribute and VRM temp alarm
drivers/hwmon/occ/common.c | 51 +++++++++++----------------------------------- 1 file changed, 12 insertions(+), 39 deletions(-)
Signature · 1 line
1.8.3.1
PATCH 1/2 hwmon (occ): Remove temperature fault attribute and VRM temp alarm
From: "Edward A. James" <[email protected]>
This removes the EAGAIN return for "sensor not ready" and the temperature fault and vrm temp alarms attributes.
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Quoted 28 lines · Brad Bishop
userspace retries for a configurable period on eio, etimedout, ebadmsg, eagain, and exnio. It also exits cleanly on enoent.
If any of these errnos don’t go away after the configured number of retry attempts, an error is logged. Any other errnos and an error is logged immediately. If this list needs improvement I’d love to hear about it.
ebadmsg and enxio are observed when i2c devices are unplugged with transfers in various stages of flight. They occur just before the driver is unbound after the presence gpio toggle on these devices is noticed and processed (killing the hwmon userspace daemon cleanly).
eio, etimedout seem to be bus type errors that appear somewhat infrequently, but occur nevertheless.
we all know what eagain means...
It isn’t so much that the hwmon userspace doesn’t support this change, its just the resulting behavior difference at the other end of the hwmon abi <-> dbus api translation is not optimal, right now.
1 - Without this change, the hwmon userspace reads a value of 0 out of the sysfs attribute and happily reports that as the temp at the DBus level.
With this change, the hwmon userspace would read the attribute, get an error and retry for a bit. After a number of retries, the error is logged and the sensor is marked faulted at a dbus level.
Quoted 8 lines
The issue is the consumer of the sensor at the dbus level. Today, the dbus consumer happens to translate the sensor value of zero into the desired behavior. If we submit this change the sensor will be put into faulted state and the consuming application doesn’t have support for that yet.
We have open issues to enhance the application to support faulted sensors, its just not implemented yet.
OpenBMC-Staging-Count: 1
--- drivers/hwmon/occ/common.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-)
drivers/hwmon/occ/common.c +3 -30
@@ -304,24 +304,11 @@ static ssize_t occ_show_temp_2(struct device *dev, val =get_unaligned_be32 (&temp->sensor_id);break ;case 1 : - val = temp->value; -if (val ==OCC_TEMP_SENSOR_FAULT ) -return -EREMOTEIO ; - -if (temp->fru_type !=OCC_FRU_TYPE_VRM ) { -/* sensor not ready */ -if (val ==0 ) -return -EAGAIN ; - - val *=1000 ;/* millidegrees */ - } + val = temp->value *1000 ;break ;case 2 : val = temp->fru_type;break ; -case 3 : - val = temp->value ==OCC_TEMP_SENSOR_FAULT ; -break ;default :return -EINVAL ; }
@@ -790,7 +777,7 @@ static int occ_setup_sensor_attrs(struct occ *occ) num_attrs += (sensors->temp.num_sensors *2 );break ;case 2 : - num_attrs += (sensors->temp.num_sensors *4 ); + num_attrs += (sensors->temp.num_sensors *3 ); show_temp = occ_show_temp_2;break ;default :
@@ -863,22 +850,13 @@ static int occ_setup_sensor_attrs(struct occ *occ)for (i =0 ; i < sensors->temp.num_sensors; ++i) { s = i +1 ; - temp = ((struct temp_sensor_2 *)sensors->temp.data) + i;snprintf (attr->name,sizeof (attr->name),"temp%d_label" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,0 , i); attr++; -if (sensors->temp.version >1 && - temp->fru_type ==OCC_FRU_TYPE_VRM ) { -snprintf (attr->name,sizeof (attr->name), -"temp%d_alarm" , s); - }else { -snprintf (attr->name,sizeof (attr->name), -"temp%d_input" , s); - } - +snprintf (attr->name,sizeof (attr->name),"temp%d_input" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,1 , i); attr++;
@@ -890,11 +868,6 @@ static int occ_setup_sensor_attrs(struct occ *occ) show_temp, NULL, 2, i); attr++; -snprintf (attr->name,sizeof (attr->name), -"temp%d_fault" , s); - attr->sensor =OCC_INIT_ATTR (attr->name,0444 , - show_temp,NULL ,3 , i); - attr++; } }
Signature · 1 line
1.8.3.1
Re: [PATCH linux dev-4.13 1/2] hwmon (occ): Remove temperature fault attribute and VRM temp alarm
Quoted 9 lines · Eddie James
From: "Edward A. James" <[email protected]>
This removes the EAGAIN return for "sensor not ready" and the temperature fault and vrm temp alarms attributes.
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Okay. Brad's email is from October 5th, 2017. That's 148 days ago.
Any ETA on when the userspace plans to get some love?
Cheers,
Joel
Quoted 124 lines
Quoted 28 lines · Brad Bishop
userspace retries for a configurable period on eio, etimedout, ebadmsg, eagain, and exnio. It also exits cleanly on enoent.
If any of these errnos don’t go away after the configured number of retry attempts, an error is logged. Any other errnos and an error is logged immediately. If this list needs improvement I’d love to hear about it.
ebadmsg and enxio are observed when i2c devices are unplugged with transfers in various stages of flight. They occur just before the driver is unbound after the presence gpio toggle on these devices is noticed and processed (killing the hwmon userspace daemon cleanly).
eio, etimedout seem to be bus type errors that appear somewhat infrequently, but occur nevertheless.
we all know what eagain means...
It isn’t so much that the hwmon userspace doesn’t support this change, its just the resulting behavior difference at the other end of the hwmon abi <-> dbus api translation is not optimal, right now.
1 - Without this change, the hwmon userspace reads a value of 0 out of the sysfs attribute and happily reports that as the temp at the DBus level.
With this change, the hwmon userspace would read the attribute, get an error and retry for a bit. After a number of retries, the error is logged and the sensor is marked faulted at a dbus level.
Quoted 8 lines
The issue is the consumer of the sensor at the dbus level. Today, the dbus consumer happens to translate the sensor value of zero into the desired behavior. If we submit this change the sensor will be put into faulted state and the consuming application doesn’t have support for that yet.
We have open issues to enhance the application to support faulted sensors, its just not implemented yet.
OpenBMC-Staging-Count: 1
Signed-off-by Eddie James <[email protected]>Signed-off-by Joel Stanley <[email protected]>--- drivers/hwmon/occ/common.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-)drivers/hwmon/occ/common.c +3
-31@@ -304,24 +304,11 @@ static ssize_t occ_show_temp_2(struct device *dev, val =get_unaligned_be32 (&temp->sensor_id);break ;case 1 : - val = temp->value; -if (val ==OCC_TEMP_SENSOR_FAULT ) -return -EREMOTEIO ; - -if (temp->fru_type !=OCC_FRU_TYPE_VRM ) { -/* sensor not ready */ -if (val ==0 ) -return -EAGAIN ; - - val *=1000 ;/* millidegrees */ - } + val = temp->value *1000 ;break ;case 2 : val = temp->fru_type;break ; -case 3 : - val = temp->value ==OCC_TEMP_SENSOR_FAULT ; -break ;default :return -EINVAL ; }@@ -790,7 +777,7 @@ static int occ_setup_sensor_attrs(struct occ *occ) num_attrs += (sensors->temp.num_sensors *2 );break ;case 2 : - num_attrs += (sensors->temp.num_sensors *4 ); + num_attrs += (sensors->temp.num_sensors *3 ); show_temp = occ_show_temp_2;break ;default :@@ -863,22 +850,13 @@ static int occ_setup_sensor_attrs(struct occ *occ)for (i =0 ; i < sensors->temp.num_sensors; ++i) { s = i +1 ; - temp = ((struct temp_sensor_2 *)sensors->temp.data) + i;snprintf (attr->name,sizeof (attr->name),"temp%d_label" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,0 , i); attr++; -if (sensors->temp.version >1 && - temp->fru_type ==OCC_FRU_TYPE_VRM ) { -snprintf (attr->name,sizeof (attr->name), -"temp%d_alarm" , s); - }else { -snprintf (attr->name,sizeof (attr->name), -"temp%d_input" , s); - } - +snprintf (attr->name,sizeof (attr->name),"temp%d_input" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,1 , i); attr++;@@ -890,11 +868,6 @@ static int occ_setup_sensor_attrs(struct occ *occ) show_temp, NULL, 2, i); attr++; -snprintf (attr->name,sizeof (attr->name), -"temp%d_fault" , s); - attr->sensor =OCC_INIT_ATTR (attr->name,0444 , - show_temp,NULL ,3 , i); - attr++; } } --1.8.3.1
Re: [PATCH linux dev-4.13 1/2] hwmon (occ): Remove temperature fault attribute and VRM temp alarm
Quoted 6 lines · Joel Stanley
Quoted 5 lines · Eddie James
From: "Edward A. James" <[email protected]>
This removes the EAGAIN return for "sensor not ready" and the temperature fault and vrm temp alarms attributes.
Quoted 9 lines
Quoted 4 lines
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Okay. Brad's email is from October 5th, 2017. That's 148 days ago.
Any ETA on when the userspace plans to get some love?
We have 4 issues in the backlog to address the OCC GPU sensors returning either a 0 or 0xFF:
https://github.com/openbmc/openbmc/issues/2327
https://github.com/openbmc/openbmc/issues/2329
https://github.com/openbmc/openbmc/issues/2222
https://github.com/openbmc/openbmc/issues/2223
As fan control is already doing the right thing when it reads the values as they are now, we deprioritized the work quite a bit so it still isn't scheduled and may not be until later systems.
Quoted 130 lines
Cheers,
Joel
Quoted 124 lines
Quoted 28 lines · Brad Bishop
userspace retries for a configurable period on eio, etimedout, ebadmsg, eagain, and exnio. It also exits cleanly on enoent.
If any of these errnos don’t go away after the configured number of retry attempts, an error is logged. Any other errnos and an error is logged immediately. If this list needs improvement I’d love to hear about it.
ebadmsg and enxio are observed when i2c devices are unplugged with transfers in various stages of flight. They occur just before the driver is unbound after the presence gpio toggle on these devices is noticed and processed (killing the hwmon userspace daemon cleanly).
eio, etimedout seem to be bus type errors that appear somewhat infrequently, but occur nevertheless.
we all know what eagain means...
It isn’t so much that the hwmon userspace doesn’t support this change, its just the resulting behavior difference at the other end of the hwmon abi <-> dbus api translation is not optimal, right now.
1 - Without this change, the hwmon userspace reads a value of 0 out of the sysfs attribute and happily reports that as the temp at the DBus level.
With this change, the hwmon userspace would read the attribute, get an error and retry for a bit. After a number of retries, the error is logged and the sensor is marked faulted at a dbus level.
Quoted 8 lines
The issue is the consumer of the sensor at the dbus level. Today, the dbus consumer happens to translate the sensor value of zero into the desired behavior. If we submit this change the sensor will be put into faulted state and the consuming application doesn’t have support for that yet.
We have open issues to enhance the application to support faulted sensors, its just not implemented yet.
OpenBMC-Staging-Count: 1
Signed-off-by Eddie James <[email protected]>Signed-off-by Joel Stanley <[email protected]>--- drivers/hwmon/occ/common.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-)drivers/hwmon/occ/common.c +3
-31@@ -304,24 +304,11 @@ static ssize_t occ_show_temp_2(struct device *dev, val =get_unaligned_be32 (&temp->sensor_id);break ;case 1 : - val = temp->value; -if (val ==OCC_TEMP_SENSOR_FAULT ) -return -EREMOTEIO ; - -if (temp->fru_type !=OCC_FRU_TYPE_VRM ) { -/* sensor not ready */ -if (val ==0 ) -return -EAGAIN ; - - val *=1000 ;/* millidegrees */ - } + val = temp->value *1000 ;break ;case 2 : val = temp->fru_type;break ; -case 3 : - val = temp->value ==OCC_TEMP_SENSOR_FAULT ; -break ;default :return -EINVAL ; }@@ -790,7 +777,7 @@ static int occ_setup_sensor_attrs(struct occ *occ) num_attrs += (sensors->temp.num_sensors *2 );break ;case 2 : - num_attrs += (sensors->temp.num_sensors *4 ); + num_attrs += (sensors->temp.num_sensors *3 ); show_temp = occ_show_temp_2;break ;default :@@ -863,22 +850,13 @@ static int occ_setup_sensor_attrs(struct occ *occ)for (i =0 ; i < sensors->temp.num_sensors; ++i) { s = i +1 ; - temp = ((struct temp_sensor_2 *)sensors->temp.data) + i;snprintf (attr->name,sizeof (attr->name),"temp%d_label" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,0 , i); attr++; -if (sensors->temp.version >1 && - temp->fru_type ==OCC_FRU_TYPE_VRM ) { -snprintf (attr->name,sizeof (attr->name), -"temp%d_alarm" , s); - }else { -snprintf (attr->name,sizeof (attr->name), -"temp%d_input" , s); - } - +snprintf (attr->name,sizeof (attr->name),"temp%d_input" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,1 , i); attr++;@@ -890,11 +868,6 @@ static int occ_setup_sensor_attrs(struct occ *occ) show_temp, NULL, 2, i); attr++; -snprintf (attr->name,sizeof (attr->name), -"temp%d_fault" , s); - attr->sensor =OCC_INIT_ATTR (attr->name,0444 , - show_temp,NULL ,3 , i); - attr++; } } --1.8.3.1
Re: [PATCH linux dev-4.13 1/2] hwmon (occ): Remove temperature fault attribute and VRM temp alarm
Quoted 30 lines · Matt Spinler
Quoted 15 lines · Joel Stanley
Quoted 10 lines · Eddie James
From: "Edward A. James" <[email protected]>
This removes the EAGAIN return for "sensor not ready" and the temperature fault and vrm temp alarms attributes.
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Quoted 4 lines
Okay. Brad's email is from October 5th, 2017. That's 148 days ago.
Any ETA on when the userspace plans to get some love?
We have 4 issues in the backlog to address the OCC GPU sensors returning either a 0 or
0xFF:
https://github.com/openbmc/openbmc/issues/2327
https://github.com/openbmc/openbmc/issues/2329
https://github.com/openbmc/openbmc/issues/2222
https://github.com/openbmc/openbmc/issues/2223
Quoted 3 lines
As fan control is already doing the right thing when it reads the values as they are now, we deprioritized the work quite a bit so it still isn't scheduled and may not be until later systems.
Okay. Lets get that work scheduled ASAP, as it's already been quite some time since we needed that fixed. I think it's now holding up the Witherspoon team from moving to the new kernel.
Do you have an ETA?
Cheers,
Joel
Re: [PATCH linux dev-4.13 1/2] hwmon (occ): Remove temperature fault attribute and VRM temp alarm
Quoted 36 lines · Joel Stanley
Quoted 30 lines · Matt Spinler
Quoted 15 lines · Joel Stanley
Quoted 10 lines · Eddie James
From: "Edward A. James" <[email protected]>
This removes the EAGAIN return for "sensor not ready" and the temperature fault and vrm temp alarms attributes.
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Quoted 4 lines
Okay. Brad's email is from October 5th, 2017. That's 148 days ago.
Any ETA on when the userspace plans to get some love?
We have 4 issues in the backlog to address the OCC GPU sensors returning either a 0 or
0xFF:
https://github.com/openbmc/openbmc/issues/2327
https://github.com/openbmc/openbmc/issues/2329
https://github.com/openbmc/openbmc/issues/2222
https://github.com/openbmc/openbmc/issues/2223Quoted 3 lines
As fan control is already doing the right thing when it reads the values as they are now, we deprioritized the work quite a bit so it still isn't scheduled and may not be until later systems.
Quoted 5 lines
Okay. Lets get that work scheduled ASAP, as it's already been quite some time since we needed that fixed. I think it's now holding up the Witherspoon team from moving to the new kernel.
Do you have an ETA?
We're going to try to start work on it next sprint, so done in about a month give or take.
Quoted 5 lines
Cheers,
Joel
PATCH 1/2 Revert "drivers/hwmon/occ: Add temperature fault attribute and VRM temp alarm"
In reply to Eddie JamesFrom: "Edward A. James" <[email protected]>
This reverts commit e55423ee10a5057338d24383c00e813436a126ea.
Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Quoted 28 lines · Brad Bishop
userspace retries for a configurable period on eio, etimedout, ebadmsg, eagain, and exnio. It also exits cleanly on enoent.
If any of these errnos don’t go away after the configured number of retry attempts, an error is logged. Any other errnos and an error is logged immediately. If this list needs improvement I’d love to hear about it.
ebadmsg and enxio are observed when i2c devices are unplugged with transfers in various stages of flight. They occur just before the driver is unbound after the presence gpio toggle on these devices is noticed and processed (killing the hwmon userspace daemon cleanly).
eio, etimedout seem to be bus type errors that appear somewhat infrequently, but occur nevertheless.
we all know what eagain means...
It isn’t so much that the hwmon userspace doesn’t support this change, its just the resulting behavior difference at the other end of the hwmon abi <-> dbus api translation is not optimal, right now.
1 - Without this change, the hwmon userspace reads a value of 0 out of the sysfs attribute and happily reports that as the temp at the DBus level.
With this change, the hwmon userspace would read the attribute, get an error and retry for a bit. After a number of retries, the error is logged and the sensor is marked faulted at a dbus level.
Quoted 8 lines
The issue is the consumer of the sensor at the dbus level. Today, the dbus consumer happens to translate the sensor value of zero into the desired behavior. If we submit this change the sensor will be put into faulted state and the consuming application doesn’t have support for that yet.
We have open issues to enhance the application to support faulted sensors, its just not implemented yet.
OpenBMC-Staging-Count: 1
--- drivers/hwmon/occ/common.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-)
drivers/hwmon/occ/common.c +3 -30
@@ -304,24 +304,11 @@ static ssize_t occ_show_temp_2(struct device *dev, val =get_unaligned_be32 (&temp->sensor_id);break ;case 1 : - val = temp->value; -if (val ==OCC_TEMP_SENSOR_FAULT ) -return -EREMOTEIO ; - -if (temp->fru_type !=OCC_FRU_TYPE_VRM ) { -/* sensor not ready */ -if (val ==0 ) -return -EAGAIN ; - - val *=1000 ;/* millidegrees */ - } + val = temp->value *1000 ;break ;case 2 : val = temp->fru_type;break ; -case 3 : - val = temp->value ==OCC_TEMP_SENSOR_FAULT ; -break ;default :return -EINVAL ; }
@@ -790,7 +777,7 @@ static int occ_setup_sensor_attrs(struct occ *occ) num_attrs += (sensors->temp.num_sensors *2 );break ;case 2 : - num_attrs += (sensors->temp.num_sensors *4 ); + num_attrs += (sensors->temp.num_sensors *3 ); show_temp = occ_show_temp_2;break ;default :
@@ -863,22 +850,13 @@ static int occ_setup_sensor_attrs(struct occ *occ)for (i =0 ; i < sensors->temp.num_sensors; ++i) { s = i +1 ; - temp = ((struct temp_sensor_2 *)sensors->temp.data) + i;snprintf (attr->name,sizeof (attr->name),"temp%d_label" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,0 , i); attr++; -if (sensors->temp.version >1 && - temp->fru_type ==OCC_FRU_TYPE_VRM ) { -snprintf (attr->name,sizeof (attr->name), -"temp%d_alarm" , s); - }else { -snprintf (attr->name,sizeof (attr->name), -"temp%d_input" , s); - } - +snprintf (attr->name,sizeof (attr->name),"temp%d_input" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,1 , i); attr++;
@@ -890,11 +868,6 @@ static int occ_setup_sensor_attrs(struct occ *occ) show_temp, NULL, 2, i); attr++; -snprintf (attr->name,sizeof (attr->name), -"temp%d_fault" , s); - attr->sensor =OCC_INIT_ATTR (attr->name,0444 , - show_temp,NULL ,3 , i); - attr++; } }
Signature · 1 line
1.8.3.1
Re: [PATCH linux dev-4.13 1/2] Revert "drivers/hwmon/occ: Add temperature fault attribute and VRM temp alarm"
Sorry, ignore this one. I renamed this patch to hwmon (occ): Remove temperature fault attribute and VRM temp alarm and forgot to delete this one.
Quoted 128 lines · Eddie James
From: "Edward A. James" <[email protected]>
This reverts commit
e55423ee10a5057338d24383c00e813436a126ea.Apologies for pushing this up so early... Userspace applications aren't ready for this change. The hwmon polling application cannot accept EGAIN yet, and we can't be returning apparent errors if the sensor is temporarily unavailable.
Quoted 28 lines · Brad Bishop
userspace retries for a configurable period on eio, etimedout, ebadmsg, eagain, and exnio. It also exits cleanly on enoent.
If any of these errnos don’t go away after the configured number of retry attempts, an error is logged. Any other errnos and an error is logged immediately. If this list needs improvement I’d love to hear about it.
ebadmsg and enxio are observed when i2c devices are unplugged with transfers in various stages of flight. They occur just before the driver is unbound after the presence gpio toggle on these devices is noticed and processed (killing the hwmon userspace daemon cleanly).
eio, etimedout seem to be bus type errors that appear somewhat infrequently, but occur nevertheless.
we all know what eagain means...
It isn’t so much that the hwmon userspace doesn’t support this change, its just the resulting behavior difference at the other end of the hwmon abi <-> dbus api translation is not optimal, right now.
1 - Without this change, the hwmon userspace reads a value of 0 out of the sysfs attribute and happily reports that as the temp at the DBus level.
With this change, the hwmon userspace would read the attribute, get an error and retry for a bit. After a number of retries, the error is logged and the sensor is marked faulted at a dbus level.
Quoted 8 lines
The issue is the consumer of the sensor at the dbus level. Today, the dbus consumer happens to translate the sensor value of zero into the desired behavior. If we submit this change the sensor will be put into faulted state and the consuming application doesn’t have support for that yet.
We have open issues to enhance the application to support faulted sensors, its just not implemented yet.
OpenBMC-Staging-Count: 1
Signed-off-by Eddie James <[email protected]>Signed-off-by Joel Stanley <[email protected]>--- drivers/hwmon/occ/common.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-)drivers/hwmon/occ/common.c +3
-30@@ -304,24 +304,11 @@ static ssize_t occ_show_temp_2(struct device *dev, val =get_unaligned_be32 (&temp->sensor_id);break ;case 1 : - val = temp->value; -if (val ==OCC_TEMP_SENSOR_FAULT ) -return -EREMOTEIO ; - -if (temp->fru_type !=OCC_FRU_TYPE_VRM ) { -/* sensor not ready */ -if (val ==0 ) -return -EAGAIN ; - - val *=1000 ;/* millidegrees */ - } + val = temp->value *1000 ;break ;case 2 : val = temp->fru_type;break ; -case 3 : - val = temp->value ==OCC_TEMP_SENSOR_FAULT ; -break ;default :return -EINVAL ; }@@ -790,7 +777,7 @@ static int occ_setup_sensor_attrs(struct occ *occ) num_attrs += (sensors->temp.num_sensors *2 );break ;case 2 : - num_attrs += (sensors->temp.num_sensors *4 ); + num_attrs += (sensors->temp.num_sensors *3 ); show_temp = occ_show_temp_2;break ;default :@@ -863,22 +850,13 @@ static int occ_setup_sensor_attrs(struct occ *occ)for (i =0 ; i < sensors->temp.num_sensors; ++i) { s = i +1 ; - temp = ((struct temp_sensor_2 *)sensors->temp.data) + i;snprintf (attr->name,sizeof (attr->name),"temp%d_label" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,0 , i); attr++; -if (sensors->temp.version >1 && - temp->fru_type ==OCC_FRU_TYPE_VRM ) { -snprintf (attr->name,sizeof (attr->name), -"temp%d_alarm" , s); - }else { -snprintf (attr->name,sizeof (attr->name), -"temp%d_input" , s); - } - +snprintf (attr->name,sizeof (attr->name),"temp%d_input" , s); attr->sensor =OCC_INIT_ATTR (attr->name,0444 , show_temp,NULL ,1 , i); attr++;@@ -890,11 +868,6 @@ static int occ_setup_sensor_attrs(struct occ *occ) show_temp, NULL, 2, i); attr++; -snprintf (attr->name,sizeof (attr->name), -"temp%d_fault" , s); - attr->sensor =OCC_INIT_ATTR (attr->name,0444 , - show_temp,NULL ,3 , i); - attr++; } }
PATCH 2/2 hwmon (occ): Fix integer overflow in multiplication
In reply to Eddie JamesPower values were overflowing INT_MAX when being converted to microwatts, even though the storage was sufficiently large (unsigned 64 bit). Change literals to unsigned long long. Also change tmep storage to u32 to avoid overflows at 65000 millidegrees.
OpenBMC-Staging-Count: 1
--- drivers/hwmon/occ/common.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/hwmon/occ/common.c +9 -9
@@ -256,7 +256,7 @@ static ssize_t occ_show_temp_1(struct device *dev, struct device_attribute *attr, char *buf) { int rc; -u16 val =0 ; +u32 val =0 ;struct temp_sensor_1 *temp;struct occ *occ =dev_get_drvdata (dev);struct occ_sensors *sensors = &occ->sensors;
@@ -380,7 +380,7 @@ static ssize_t occ_show_power_1(struct device *dev, struct device_attribute *attr, char *buf) { int rc; -u32 val =0 ; +u64 val =0 ;struct power_sensor_1 *power;struct occ *occ =dev_get_drvdata (dev);struct occ_sensors *sensors = &occ->sensors;
@@ -404,13 +404,13 @@ static ssize_t occ_show_power_1(struct device *dev,break ;case 3 :/* microwatts */ - val =get_unaligned_be16 (&power->value) *1000000 ; + val =get_unaligned_be16 (&power->value) *1000000ULL ;break ;default :return -EINVAL ; } -return snprintf (buf,PAGE_SIZE -1 ,"%u\n" , val); +return snprintf (buf,PAGE_SIZE -1 ,"%llu\n" , val); } static ssize_t occ_show_power_2(struct device *dev,
@@ -441,7 +441,7 @@ static ssize_t occ_show_power_2(struct device *dev,break ;case 3 :/* microwatts */ - val =get_unaligned_be16 (&power->value) *1000000 ; + val =get_unaligned_be16 (&power->value) *1000000ULL ;break ;case 4 : val = power->function_id;
@@ -483,7 +483,7 @@ static ssize_t occ_show_power_a0(struct device *dev,break ;case 3 :/* microwatts */ - val =get_unaligned_be16 (&power->system.value) *1000000 ; + val =get_unaligned_be16 (&power->system.value) *1000000ULL ;break ;case 4 : val =get_unaligned_be32 (&power->system.update_tag);
@@ -498,7 +498,7 @@ static ssize_t occ_show_power_a0(struct device *dev,break ;case 8 :/* microwatts */ - val =get_unaligned_be16 (&power->proc.value) *1000000 ; + val =get_unaligned_be16 (&power->proc.value) *1000000ULL ;break ;case 9 : val =get_unaligned_be32 (&power->proc.update_tag);
@@ -510,7 +510,7 @@ static ssize_t occ_show_power_a0(struct device *dev,return snprintf (buf,PAGE_SIZE -1 ,"vdd\n" );case 12 :/* microwatts */ - val =get_unaligned_be16 (&power->vdd.value) *1000000 ; + val =get_unaligned_be16 (&power->vdd.value) *1000000ULL ;break ;case 13 : val =get_unaligned_be32 (&power->vdd.update_tag);
@@ -522,7 +522,7 @@ static ssize_t occ_show_power_a0(struct device *dev,return snprintf (buf,PAGE_SIZE -1 ,"vdn\n" );case 16 :/* microwatts */ - val =get_unaligned_be16 (&power->vdn.value) *1000000 ; + val =get_unaligned_be16 (&power->vdn.value) *1000000ULL ;break ;case 17 : val =get_unaligned_be32 (&power->vdn.update_tag);
Signature · 1 line
1.8.3.1