[RFC PATCH linux 0/9] On-Chip Controller (OCC) hwmon driver

[Andrew Jeffery]

Hi Eddie,

> This patchset provides a new OCC hwmon driver. There are a number
> of issues with the existing driver. Firstly, i2c access was embedded
> throughout the driver. Secondly, there is no way to easily differentiate
> versions of the OCC.
> 
> This patchset addresses the first issue by abstracting the bus transfer
> protocol into a modular structure. In this way, any low level transfer
> method may be easily implemented.
> 
> The second issue is addressed by separating the "version specific" code
> for the OCC and the common hwmon code. This task is not yet complete, but
> the general structure is in place. Ultimately, different OCC versions
> could be probed up using the device tree.

I agree with all of this. However, I thought some of the layering could still
be improved, so rather than provide a review through prose I figured it might
be better for both of us if I mangled the patches directly. 

I've attacked two things. The first is I found the series hard to review due to
the way the patches were split, as the patches went from trivial to huge (and
complex) in a fairly short step:

> 
> Edward A. James (5):
>   Revert "hwmon: Add Power8 OCC hwmon driver"
>   drivers: Add hwmon occ driver framework
>   drivers: hwmon OCC scom bus operations
>   drivers: Add hwmon OCC version specific functions
>   drivers: OCC hwmon driver and sysfs

So I have reworked them into what is now 8 patches (see below) and tried to
tell a bit more of a story with them (hopefully I succeeded!) Second, I tried
my hand at reworking the layering as mentioned above, such that hardware
interaction is split from sysfs, and that the SCOM transport and sensor
configuration is decoupled from the data parsing. You largely had all of these
in-place already, I just cut and pasted code around a bit.

Please review the patches and provide feedback! I'd like to hear arguments for
or against both of our approaches. Note though that I've only tested that these
patches compile, I absolutely have not done any further testing.

Joel: Any chance you could chime in with an architectural review?

When the dust has settled on the bigger issues we can start to look at the
finer details of the APIs (there are some bugs, oddities and complexities that
I would like to address before they are applied to openbmc/linux).

Cheers,

Andrew

Andrew Jeffery (8):
  hwmon: Add core On-Chip Controller support for POWER CPUs
  hwmon: occ: Add sysfs interface
  hwmon: occ: Add I2C SCOM transport implementation
  hwmon: occ: Add callbacks for parsing P8 OCC datastructures
  hwmon: occ: Add hwmon implementation for the P8 OCC
  arm: aspeed: dt: Fix OCC compatible strings
  hwmon: occ: Add callbacks for parsing P9 OCC datastructures
  wip: hwmon: occ: Add sketch of the hwmon implementation for the P9 OCC

Edward A. James (1):
  Revert "hwmon: Add Power8 OCC hwmon driver"

 .../devicetree/bindings/i2c/i2c-ibm-occ.txt        |   13 -
 arch/arm/boot/dts/aspeed-bmc-opp-barreleye.dts     |    4 +-
 arch/arm/boot/dts/aspeed-bmc-opp-firestone.dts     |    4 +-
 arch/arm/boot/dts/aspeed-bmc-opp-garrison.dts      |    4 +-
 arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts      |    2 +-
 drivers/hwmon/Kconfig                              |   13 +-
 drivers/hwmon/Makefile                             |    2 +-
 drivers/hwmon/occ/Kconfig                          |   38 +
 drivers/hwmon/occ/Makefile                         |    3 +
 drivers/hwmon/occ/occ.c                            |  494 ++++++++
 drivers/hwmon/occ/occ.h                            |   79 ++
 drivers/hwmon/occ/occ_p8.c                         |  219 ++++
 drivers/hwmon/occ/occ_p8.h                         |   30 +
 drivers/hwmon/occ/occ_p8_i2c.c                     |  133 +++
 drivers/hwmon/occ/occ_p9.c                         |  261 ++++
 drivers/hwmon/occ/occ_p9.h                         |   30 +
 drivers/hwmon/occ/occ_p9_fsi.c                     |   93 ++
 drivers/hwmon/occ/occ_scom_i2c.c                   |   68 ++
 drivers/hwmon/occ/occ_scom_i2c.h                   |   20 +
 drivers/hwmon/occ/occ_sysfs.c                      |  498 ++++++++
 drivers/hwmon/occ/occ_sysfs.h                      |   51 +
 drivers/hwmon/occ/scom.h                           |   50 +
 drivers/hwmon/power8_occ_i2c.c                     | 1254 --------------------
 23 files changed, 2076 insertions(+), 1287 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
 create mode 100644 drivers/hwmon/occ/Kconfig
 create mode 100644 drivers/hwmon/occ/Makefile
 create mode 100644 drivers/hwmon/occ/occ.c
 create mode 100644 drivers/hwmon/occ/occ.h
 create mode 100644 drivers/hwmon/occ/occ_p8.c
 create mode 100644 drivers/hwmon/occ/occ_p8.h
 create mode 100644 drivers/hwmon/occ/occ_p8_i2c.c
 create mode 100644 drivers/hwmon/occ/occ_p9.c
 create mode 100644 drivers/hwmon/occ/occ_p9.h
 create mode 100644 drivers/hwmon/occ/occ_p9_fsi.c
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
 create mode 100644 drivers/hwmon/occ/occ_sysfs.c
 create mode 100644 drivers/hwmon/occ/occ_sysfs.h
 create mode 100644 drivers/hwmon/occ/scom.h
 delete mode 100644 drivers/hwmon/power8_occ_i2c.c

-- 
2.9.3