linux-kernel - Re: [PATCH linux v7 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Feb 2017 16:01:51 +1030
From:   Joel Stanley <joel@....id.au>
To:     eajames <eajames@...ux.vnet.ibm.com>
Cc:     Guenter Roeck <linux@...ck-us.net>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
        linux-doc@...r.kernel.org, jdelvare@...e.com, corbet@....net,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Wolfram Sang <wsa@...-dreams.de>,
        Andrew Jeffery <andrew@...id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        "Edward A. James" <eajames@...ibm.com>
Subject: Re: [PATCH linux v7 5/6] hwmon: occ: Add hwmon implementation for the
 P8 OCC

On Wed, Feb 8, 2017 at 9:40 AM,  <eajames@...ux.vnet.ibm.com> wrote:
> From: "Edward A. James" <eajames@...ibm.com>
>
> Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as
> well as probe the entire driver from the I2C bus. I2C is the communication
> method between the BMC and the P8 OCC.
>
> Signed-off-by: Edward A. James <eajames@...ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@...id.au>
> Acked-by: Rob Herring <robh@...nel.org>
> ---
>  Documentation/devicetree/bindings/hwmon/occ.txt |  13 +++
>  drivers/hwmon/occ/Kconfig                       |  14 ++++
>  drivers/hwmon/occ/Makefile                      |   1 +
>  drivers/hwmon/occ/p8_occ_i2c.c                  | 104 ++++++++++++++++++++++++

For consistency, how about we call this occ_p8_i2c.c? The other files
have the machine name second.


>  4 files changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/occ.txt
>  create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c
>

> diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> index cdb64a7..3a5188f 100644
> --- a/drivers/hwmon/occ/Kconfig
> +++ b/drivers/hwmon/occ/Kconfig
> @@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC
>
>           This driver can also be built as a module. If so, the module
>           will be called occ.
> +
> +if SENSORS_PPC_OCC
> +
> +config SENSORS_PPC_OCC_P8_I2C
> +       tristate "POWER8 OCC hwmon support"
> +       depends on I2C
> +       help
> +        Provide a hwmon sysfs interface for the POWER8 On-Chip Controller,
> +        exposing temperature, frequency and power measurements.
> +
> +        This driver can also be built as a module. If so, the module will be
> +        called p8-occ-i2c.

Mention that this driver is for the BMC (or just "service processor"),
and is not useful in the Power8 kernel.

I'm trying to think of a better prefix than PPC. PPC means much more
than Power8 and Power9, which is what we mean. It's also confusing as
we don't run this on any PowerPC machine - it's for an ARM chip.

> +
> +int p8_i2c_getscom(void *bus, u32 address, u64 *data)
> +{
> +       /* P8 i2c slave requires address to be shifted by 1 */
> +       address = address << 1;

I think these are scom addresses? Please indicate that so we don't get
them confused with i2c.

Or are they scom operations?

> +
> +       return occ_i2c_getscom(bus, address, data);
> +}
> +
> +int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)

The functions can be static.

> +{
> +       /* P8 i2c slave requires address to be shifted by 1 */
> +       address = address << 1;
> +
> +       return occ_i2c_putscom(bus, address, data0, data1);
> +}