On Tue, Sep 05, 2017 at 03:07:10PM -0500, Eddie James wrote: > Hi Guenter, > > I have some additional features to add to this driver to handle some special temperature sensor values. Wanted to run it by you before submitting a new version to the list, since I’m not sure if there is a standard way of addressing this or not. > > Temp sensor may return 0 for “data unavailable; may be available later” and 0xFF for error. It makes sense to map 0xFF to a tempX_fault attribute. However, for the “unavailable” case, should I use tempX_alarm for this? I think with “alarm” the sensor data is usually available in a normal hwmon device? We would either have to return a cached value for the tempX_input or return an error, since the data is not there. > mapping 0xff to fault makes sense. Mapping 0x00 to alarm, however, doesn't really make sense, since there is not really an alarm condition (over/undertemperature/voltage). I can see two options: Either return a cached value, or return something like -EAGAIN. Question though is if the condition is or can be permanent. If yes, -EAGAIN would not be a good idea since it most likely results in an immediate retry by userspace. If the condition is or can be permanent, all you can probably do would be to report a cached value for some period of time and then switch to reporting a fault condition. [ sorry for not being more proactive with the review ] Guenter > Thanks, > Eddie > > > On Jul 27, 2017, at 9:30 AM, Eddie James <eajames@xxxxxxxxxxxxxxxxxx> wrote: > > > > From: "Edward A. James" <eajames@xxxxxxxxxx> > > > > This series adds a hwmon driver to support the OCC on POWER8 and POWER9 > > processors. The OCC is an embedded processor that provides realtime power and > > thermal monitoring and management. > > > > This driver has two different platform drivers as a "base" for the > > hwmon interface, as the means of communicating with the OCC on P8 and P9 is > > completely different. For P8, the driver is an I2C client driver. For P9 the > > driver is an FSI-based OCC client driver, and uses the OCC driver in-kernel > > API. The OCC driver is on the LKML (latest https://lkml.org/lkml/2017/7/5/633). > > > > Changes since v1: > > * Remove wait loop in P9 code, as that is now handled by FSI OCC driver. > > * Removed dt binding documentation for P9, FSI OCC driver will probe OCC hwmon > > driver automatically. > > * Moved OCC response code definitions to the OCC include file. > > * Fixed includes. > > * Changed some structure fields to __beXX as that is what they are. > > * Changed some errnos. > > * Removed some dev_err(). > > * Refactored P8 code a bit to use #defined addresses and magic values, and > > changed "goto retry" to a loop. > > * Refactored error handling a bit. > > > > Edward A. James (10): > > drivers/hwmon: Add On-Chip Controller (OCC) hwmon driver > > drivers/hwmon/occ: Add command transport method for P8 and P9 > > drivers/hwmon/occ: Parse OCC poll response > > drivers/hwmon/occ: Add sensor types and versions > > drivers/hwmon/occ: Add sensor attributes and register hwmon device > > drivers/hwmon/occ: Add non-hwmon attributes > > drivers/hwmon/occ: Add error handling > > Documentation: hwmon: Add OCC documentation > > Documentation: ABI: Add occ-hwmon driver sysfs documentation > > dt-bindings: i2c: Add P8 OCC hwmon driver documentation > > > > Documentation/ABI/testing/sysfs-driver-occ-hwmon | 77 ++ > > .../devicetree/bindings/i2c/ibm,p8-occ-hwmon.txt | 25 + > > Documentation/hwmon/occ | 74 ++ > > drivers/hwmon/Kconfig | 2 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/occ/Kconfig | 28 + > > drivers/hwmon/occ/Makefile | 11 + > > drivers/hwmon/occ/common.c | 1289 ++++++++++++++++++++ > > drivers/hwmon/occ/common.h | 119 ++ > > drivers/hwmon/occ/p8_i2c.c | 261 ++++ > > drivers/hwmon/occ/p9_sbe.c | 124 ++ > > 11 files changed, 2011 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-driver-occ-hwmon > > create mode 100644 Documentation/devicetree/bindings/i2c/ibm,p8-occ-hwmon.txt > > create mode 100644 Documentation/hwmon/occ > > create mode 100644 drivers/hwmon/occ/Kconfig > > create mode 100644 drivers/hwmon/occ/Makefile > > create mode 100644 drivers/hwmon/occ/common.c > > create mode 100644 drivers/hwmon/occ/common.h > > create mode 100644 drivers/hwmon/occ/p8_i2c.c > > create mode 100644 drivers/hwmon/occ/p9_sbe.c > > > > -- > > 1.8.3.1 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html