mbox series

[0/3] occ: Restore default behavior of polling OCC during init

Message ID 20220802194656.240564-1-eajames@linux.ibm.com
Headers show
Series occ: Restore default behavior of polling OCC during init | expand

Message

Eddie James Aug. 2, 2022, 7:46 p.m. UTC
A recent change to the OCC hwmon driver modified the default behavior to
no longer poll the OCC during initialization. This does change the
interface, meaning that old applications will not work with the more
recent driver. To resolve this issue, introduce a new dts property to
control the behavior of the driver during initialization, similar to the
FSI master property "no-scan-on-init". Without the new
"ibm,inactive-on-init" boolean present, the driver will now do the
previous behavior of polling the OCC.

Eddie James (3):
  dt-bindings: hwmon: Add IBM OCC bindings
  fsi: occ: Support probing the hwmon child device from dts node
  hwmon: (occ) Check for device property for setting OCC active during
    probe

 .../bindings/hwmon/ibm,occ-hmwon.yaml         | 40 ++++++++++++++++++
 drivers/fsi/fsi-occ.c                         | 41 +++++++++++++++----
 drivers/hwmon/occ/common.c                    | 11 ++++-
 drivers/hwmon/occ/p9_sbe.c                    |  9 ++++
 4 files changed, 93 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml

Comments

Eddie James Aug. 9, 2022, 7:42 p.m. UTC | #1
On 8/3/22 01:55, Krzysztof Kozlowski wrote:
> On 02/08/2022 21:46, Eddie James wrote:
>> These bindings describe the POWER processor On Chip Controller accessed
>> from a service processor or baseboard management controller (BMC).
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   .../bindings/hwmon/ibm,occ-hmwon.yaml         | 40 +++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml
>> new file mode 100644
>> index 000000000000..8f8c3b8d7129
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hmwon.yaml
>> @@ -0,0 +1,40 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml#
> typo here
>
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).


I actually did but it didn't catch that somehow. I had to use a somewhat 
hacked together python/pip on my system so perhaps that's to blame.


>
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: IBM On-Chip Controller (OCC) accessed from a service processor
>> +
>> +maintainers:
>> +  - Eddie James <eajames@linux.ibm.com>
>> +
>> +description: |
>> +  This binding describes a POWER processor On-Chip Controller (OCC)
> s/This binding describes a//
> But instead describe the hardware. What is the OCC?


OK I'll fix that. It's a management engine for system power and thermals.


>
>> +  accessed from a service processor or baseboard management controller
>> +  (BMC).
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ibm,p9-occ-hwmon
>> +      - ibm,p10-occ-hwmon
>> +
>> +  ibm,inactive-on-init:
>> +    description: This property describes whether or not the OCC should
>> +      be marked as active during device initialization. The alternative
>> +      is for user space to mark the device active based on higher level
>> +      communications between the BMC and the host processor.
> I find the combination property name with this description confusing. It
> sounds like init of OCC and somehow it should be inactive? I assume if
> you initialize device, it is active. Or maybe the "init" is of something
> else? What is more, non-negation is easier to understand, so rather
> "ibm,active-on-boot" (or something like that).


Well, the host processor initializes the OCC during it's boot, but this 
document is describing a binding to be used by a service processor 
talking to the OCC. So the OCC may be in any state. The init meant 
driver init, but I will simply the description and change the property 
to be more explicit: ibm,no-poll-on-init since that is what is actually 
happening. Similar to the FSI binding for no-scan-on-init.


>
>> +    type: boolean
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    occ-hmwon {
> just "hwmon"
>
>> +        compatible = "ibm,p9-occ-hwmon";
>> +        ibm,inactive-on-init;
>> +    };


Thanks for the review!

Eddie


>
> Best regards,
> Krzysztof