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

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

[Eddie James]

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,no-poll-on-init" boolean present, the driver will now do the
previous behavior of polling the OCC.

Changes since v1:
 - Fix bindings and rename ibm,inactive-on-init to ibm,no-poll-on-init
 - Fix incorrect MODULE_DEVICE_TABLE argument

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-hwmon.yaml         | 39 ++++++++++++++++++
 drivers/fsi/fsi-occ.c                         | 41 +++++++++++++++----
 drivers/hwmon/occ/common.c                    | 11 ++++-
 drivers/hwmon/occ/p9_sbe.c                    |  9 ++++
 4 files changed, 92 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml

-- 
2.31.1

[PATCH v2 1/3] dt-bindings: hwmon: Add IBM OCC bindings

[Eddie James]

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-hwmon.yaml         | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml
new file mode 100644
index 000000000000..3dbdc5af2804
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml#
+$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: |
+  The POWER processor On-Chip Controller (OCC) helps manage power and
+  thermals for the system. A service processor or baseboard management
+  controller can query the OCC for it's power and thermal data to report
+  through hwmon.
+
+properties:
+  compatible:
+    enum:
+      - ibm,p9-occ-hwmon
+      - ibm,p10-occ-hwmon
+
+  ibm,no-poll-on-init:
+    description: This property describes whether or not the OCC should
+      be polled during driver initialization.
+    type: boolean
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    hwmon {
+        compatible = "ibm,p10-occ-hwmon";
+        ibm,no-poll-on-init;
+    };
-- 
2.31.1

Re: [PATCH v2 1/3] dt-bindings: hwmon: Add IBM OCC bindings

[Rob Herring]

On Tue, Aug 9, 2022 at 2:07 PM Eddie James <eajames@linux.ibm.com> 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-hwmon.yaml         | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml

What about Documentation/devicetree/bindings/hwmon/ibm,p8-occ-hwmon.txt?

> diff --git a/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml
> new file mode 100644
> index 000000000000..3dbdc5af2804
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ibm,occ-hwmon.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/ibm,occ-hwmon.yaml#
> +$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: |

Don't need '|' if there is no formatting.

> +  The POWER processor On-Chip Controller (OCC) helps manage power and
> +  thermals for the system. A service processor or baseboard management
> +  controller can query the OCC for it's power and thermal data to report

s/it's/its/

> +  through hwmon.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ibm,p9-occ-hwmon
> +      - ibm,p10-occ-hwmon
> +
> +  ibm,no-poll-on-init:
> +    description: This property describes whether or not the OCC should
> +      be polled during driver initialization.
> +    type: boolean
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hwmon {
> +        compatible = "ibm,p10-occ-hwmon";
> +        ibm,no-poll-on-init;

How does one access this device? There is no address.

> +    };

> --
> 2.31.1
>

[PATCH v2 2/3] fsi: occ: Support probing the hwmon child device from dts node

[Eddie James]

There is now a need for reading devicetree properties in the OCC
hwmon driver, which isn't current supported as the FSI driver just
instantiates a basic platform device. Add support for this use case
by checking for an "occ-hwmon" node and if present, creating an
OF device from it.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/fsi/fsi-occ.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 8f7f602b909d..abdd37d5507f 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -44,6 +44,7 @@ struct occ {
 	struct device *sbefifo;
 	char name[32];
 	int idx;
+	bool platform_hwmon;
 	u8 sequence_number;
 	void *buffer;
 	void *client_buffer;
@@ -598,7 +599,7 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 }
 EXPORT_SYMBOL_GPL(fsi_occ_submit);
 
-static int occ_unregister_child(struct device *dev, void *data)
+static int occ_unregister_platform_child(struct device *dev, void *data)
 {
 	struct platform_device *hwmon_dev = to_platform_device(dev);
 
@@ -607,12 +608,25 @@ static int occ_unregister_child(struct device *dev, void *data)
 	return 0;
 }
 
+static int occ_unregister_of_child(struct device *dev, void *data)
+{
+	struct platform_device *hwmon_dev = to_platform_device(dev);
+
+	of_device_unregister(hwmon_dev);
+	if (dev->of_node)
+		of_node_clear_flag(dev->of_node, OF_POPULATED);
+
+	return 0;
+}
+
 static int occ_probe(struct platform_device *pdev)
 {
 	int rc;
 	u32 reg;
+	char child_name[32];
 	struct occ *occ;
-	struct platform_device *hwmon_dev;
+	struct platform_device *hwmon_dev = NULL;
+	struct device_node *hwmon_node;
 	struct device *dev = &pdev->dev;
 	struct platform_device_info hwmon_dev_info = {
 		.parent = dev,
@@ -671,10 +685,20 @@ static int occ_probe(struct platform_device *pdev)
 		return rc;
 	}
 
-	hwmon_dev_info.id = occ->idx;
-	hwmon_dev = platform_device_register_full(&hwmon_dev_info);
-	if (IS_ERR(hwmon_dev))
-		dev_warn(dev, "failed to create hwmon device\n");
+	hwmon_node = of_get_child_by_name(dev->of_node, hwmon_dev_info.name);
+	if (hwmon_node) {
+		snprintf(child_name, sizeof(child_name), "%s.%d", hwmon_dev_info.name, occ->idx);
+		hwmon_dev = of_platform_device_create(hwmon_node, child_name, dev);
+		of_node_put(hwmon_node);
+	}
+
+	if (!hwmon_dev) {
+		occ->platform_hwmon = true;
+		hwmon_dev_info.id = occ->idx;
+		hwmon_dev = platform_device_register_full(&hwmon_dev_info);
+		if (IS_ERR(hwmon_dev))
+			dev_warn(dev, "failed to create hwmon device\n");
+	}
 
 	return 0;
 }
@@ -690,7 +714,10 @@ static int occ_remove(struct platform_device *pdev)
 	occ->buffer = NULL;
 	mutex_unlock(&occ->occ_lock);
 
-	device_for_each_child(&pdev->dev, NULL, occ_unregister_child);
+	if (occ->platform_hwmon)
+		device_for_each_child(&pdev->dev, NULL, occ_unregister_platform_child);
+	else
+		device_for_each_child(&pdev->dev, NULL, occ_unregister_of_child);
 
 	ida_simple_remove(&occ_ida, occ->idx);
 
-- 
2.31.1

[PATCH v2 3/3] hwmon: (occ) Check for device property for setting OCC active during probe

[Eddie James]

A previous commit changed the existing behavior of the driver to skip
attempting to communicate with the OCC during probe. Return to the
previous default behavior of automatically communicating with the OCC
and make it optional with a new device-tree property.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/hwmon/occ/common.c | 11 ++++++++++-
 drivers/hwmon/occ/p9_sbe.c |  9 +++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index 45407b12db4b..dd690f700d49 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -10,6 +10,7 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/sysfs.h>
 #include <asm/unaligned.h>
 
@@ -1216,8 +1217,16 @@ int occ_setup(struct occ *occ)
 	occ->groups[0] = &occ->group;
 
 	rc = occ_setup_sysfs(occ);
-	if (rc)
+	if (rc) {
 		dev_err(occ->bus_dev, "failed to setup sysfs: %d\n", rc);
+		return rc;
+	}
+
+	if (!device_property_read_bool(occ->bus_dev, "ibm,no-poll-on-init")) {
+		rc = occ_active(occ, true);
+		if (rc)
+			occ_shutdown_sysfs(occ);
+	}
 
 	return rc;
 }
diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
index 4a1fe4ee8e2c..3adcf8d0b4a6 100644
--- a/drivers/hwmon/occ/p9_sbe.c
+++ b/drivers/hwmon/occ/p9_sbe.c
@@ -7,6 +7,7 @@
 #include <linux/fsi-occ.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
@@ -179,9 +180,17 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id p9_sbe_occ_of_match[] = {
+	{ .compatible = "ibm,p9-occ-hwmon" },
+	{ .compatible = "ibm,p10-occ-hwmon" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, p9_sbe_occ_of_match);
+
 static struct platform_driver p9_sbe_occ_driver = {
 	.driver = {
 		.name = "occ-hwmon",
+		.of_match_table = p9_sbe_occ_of_match,
 	},
 	.probe	= p9_sbe_occ_probe,
 	.remove = p9_sbe_occ_remove,
-- 
2.31.1

Re: [PATCH v2 3/3] hwmon: (occ) Check for device property for setting OCC active during probe

[Guenter Roeck]

On Tue, Aug 09, 2022 at 03:07:01PM -0500, Eddie James wrote:
> A previous commit changed the existing behavior of the driver to skip
> attempting to communicate with the OCC during probe. Return to the
> previous default behavior of automatically communicating with the OCC
> and make it optional with a new device-tree property.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Assuming this patch is applied as part of the series outside
the hwmon subsystem.

Acked-by: Guenter Roeck <linux@roeck-us.net>

Guenter

> ---
>  drivers/hwmon/occ/common.c | 11 ++++++++++-
>  drivers/hwmon/occ/p9_sbe.c |  9 +++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index 45407b12db4b..dd690f700d49 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -10,6 +10,7 @@
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/property.h>
>  #include <linux/sysfs.h>
>  #include <asm/unaligned.h>
>  
> @@ -1216,8 +1217,16 @@ int occ_setup(struct occ *occ)
>  	occ->groups[0] = &occ->group;
>  
>  	rc = occ_setup_sysfs(occ);
> -	if (rc)
> +	if (rc) {
>  		dev_err(occ->bus_dev, "failed to setup sysfs: %d\n", rc);
> +		return rc;
> +	}
> +
> +	if (!device_property_read_bool(occ->bus_dev, "ibm,no-poll-on-init")) {
> +		rc = occ_active(occ, true);
> +		if (rc)
> +			occ_shutdown_sysfs(occ);
> +	}
>  
>  	return rc;
>  }
> diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> index 4a1fe4ee8e2c..3adcf8d0b4a6 100644
> --- a/drivers/hwmon/occ/p9_sbe.c
> +++ b/drivers/hwmon/occ/p9_sbe.c
> @@ -7,6 +7,7 @@
>  #include <linux/fsi-occ.h>
>  #include <linux/mm.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/mutex.h>
>  #include <linux/platform_device.h>
>  #include <linux/string.h>
> @@ -179,9 +180,17 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id p9_sbe_occ_of_match[] = {
> +	{ .compatible = "ibm,p9-occ-hwmon" },
> +	{ .compatible = "ibm,p10-occ-hwmon" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, p9_sbe_occ_of_match);
> +
>  static struct platform_driver p9_sbe_occ_driver = {
>  	.driver = {
>  		.name = "occ-hwmon",
> +		.of_match_table = p9_sbe_occ_of_match,
>  	},
>  	.probe	= p9_sbe_occ_probe,
>  	.remove = p9_sbe_occ_remove,

Re: [PATCH v2 3/3] hwmon: (occ) Check for device property for setting OCC active during probe

[Joel Stanley]

On Sun, 25 Sept 2022 at 18:06, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Aug 09, 2022 at 03:07:01PM -0500, Eddie James wrote:
> > A previous commit changed the existing behavior of the driver to skip
> > attempting to communicate with the OCC during probe. Return to the
> > previous default behavior of automatically communicating with the OCC
> > and make it optional with a new device-tree property.
> >
> > Signed-off-by: Eddie James <eajames@linux.ibm.com>
>
> Assuming this patch is applied as part of the series outside
> the hwmon subsystem.
>
> Acked-by: Guenter Roeck <linux@roeck-us.net>

Thanks Guenter. I have applied it to the FSI tree.


>
> Guenter
>
> > ---
> >  drivers/hwmon/occ/common.c | 11 ++++++++++-
> >  drivers/hwmon/occ/p9_sbe.c |  9 +++++++++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> > index 45407b12db4b..dd690f700d49 100644
> > --- a/drivers/hwmon/occ/common.c
> > +++ b/drivers/hwmon/occ/common.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/math64.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/property.h>
> >  #include <linux/sysfs.h>
> >  #include <asm/unaligned.h>
> >
> > @@ -1216,8 +1217,16 @@ int occ_setup(struct occ *occ)
> >       occ->groups[0] = &occ->group;
> >
> >       rc = occ_setup_sysfs(occ);
> > -     if (rc)
> > +     if (rc) {
> >               dev_err(occ->bus_dev, "failed to setup sysfs: %d\n", rc);
> > +             return rc;
> > +     }
> > +
> > +     if (!device_property_read_bool(occ->bus_dev, "ibm,no-poll-on-init")) {
> > +             rc = occ_active(occ, true);
> > +             if (rc)
> > +                     occ_shutdown_sysfs(occ);
> > +     }
> >
> >       return rc;
> >  }
> > diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> > index 4a1fe4ee8e2c..3adcf8d0b4a6 100644
> > --- a/drivers/hwmon/occ/p9_sbe.c
> > +++ b/drivers/hwmon/occ/p9_sbe.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/fsi-occ.h>
> >  #include <linux/mm.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/mutex.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/string.h>
> > @@ -179,9 +180,17 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
> >       return 0;
> >  }
> >
> > +static const struct of_device_id p9_sbe_occ_of_match[] = {
> > +     { .compatible = "ibm,p9-occ-hwmon" },
> > +     { .compatible = "ibm,p10-occ-hwmon" },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, p9_sbe_occ_of_match);
> > +
> >  static struct platform_driver p9_sbe_occ_driver = {
> >       .driver = {
> >               .name = "occ-hwmon",
> > +             .of_match_table = p9_sbe_occ_of_match,
> >       },
> >       .probe  = p9_sbe_occ_probe,
> >       .remove = p9_sbe_occ_remove,