[PATCH linux dev-4.10 0/5] pmbus: Add MAX31785 driver

[Andrew Jeffery]

Some follow-up notes:

On Sat, 2017-07-22 at 03:21 +0930, Andrew Jeffery wrote:
> Hello,
> 
> This is a backport to dev-4.10 of the series I intend to send upstream,
> which implements the MAX31785 driver using the PMBus subsystem.
> 
> I'd like to see it cook for a bit in the OpenBMC tree to get some confidence
> in the implementation (after recent indiscretions).
> 
> Some caveats:
> 
> 1. I've only updated the Witherspoon devicetree. Romulus and Firestone also need
>    to be updated, but I'm not sure what configuration they require (it may not
>    be too different to Witherspoon). If someone could provide feedback here I
>    can send more patches.

Given Romulus and Firestone are IBM designs my expectation is the
max37185 configuration should be the same as Witherspoon, and we can
copy the nodes from Witherspoon into both devicetrees. Further, I've
taken a look at the Romulus schematic and it requires all 6 fans to be
configured. I haven't hunted down the relevant Firestone schematics,
but there's no harm in specifying nodes for all 6 fans (the driver will
simply ignore any fans that the hardware claims are not installed).
There is harm in *not* specifying the nodes, as the driver will mark
fans the hardware reports present as not installed if there is not a
corresponding devicetree node, as the devicetree should reflect the
system design.

> 2. There may be some userspace breakage around what fans are exposed: Only
>    fans configured in the devicetree will have attributes in sysfs, and the
>    first and second tach input attributes are always contiguous. That is, if
>    only one fan is specified in the devicetree and it is configured as dual
>    tach, then the input attributes will be fan1_input and fan2_input.
> 
> Anyway please review and test! I've given it a spin on a Witherspoon system and
> it looks to be functional. I haven't done much testing with the OpenBMC
> userspace to shore up the second issue mentioned above, so that needs further
> investigation.
> 

As a note, I have been tracking down an issue where I occasionally see
IO errors when writing e.g. fan1_target. It turns out that the max31785
is NACK'ing a transfer, but doesn't appear to set any bits in
STATUS_BYTE to indicate a problem. Issuing the same command immediately
after succeeds. I was concerned I may have some undefined behaviour in
the driver but it appears that's not the case in this instance.

It turns out that I can hit the NACK issue with the current hwmon
implementation, and I suspect Timothy's original driver was susceptible
to it as well. As such the new driver is issue-compatible with what we
have currently, and applying it is not a regression. Some more
investigation needs to be done to resolve whether it's there are issues
in PMBus core, the max31785 and/or the I2C bus driver.

Cheers,

Andrew

> Cheers,
> 
> Andrew
> 
> Andrew Jeffery (5):
>   dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
>   ARM: aspeed: Update max31785 node in Witherspoon devicetree
>   hwmon: pmbus: Add fan control support
>   hwmon: Remove MAX31785 implementation
>   pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller
> 
>  .../devicetree/bindings/hwmon/pmbus/max31785.txt   | 113 +++
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts   |  55 +-
>  drivers/hwmon/Kconfig                              |  10 -
>  drivers/hwmon/Makefile                             |   1 -
>  drivers/hwmon/max31785.c                           | 893 ---------------------
>  drivers/hwmon/pmbus/Kconfig                        |  10 +
>  drivers/hwmon/pmbus/Makefile                       |   1 +
>  drivers/hwmon/pmbus/max31785.c                     | 655 +++++++++++++++
>  drivers/hwmon/pmbus/pmbus.h                        |  29 +
>  drivers/hwmon/pmbus/pmbus_core.c                   | 222 ++++-
>  10 files changed, 1068 insertions(+), 921 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
>  delete mode 100644 drivers/hwmon/max31785.c
>  create mode 100644 drivers/hwmon/pmbus/max31785.c
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170726/5f357598/attachment-0001.sig>