[PATCH v2 0/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[PATCH v2 0/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[Lakshmi Yadlapati]

Adding new acbel,crps driver and documentation updates.

Changes since V1:
- New patch added for documentation.
- Fix acbel,crps drives as per review comments.

Lakshmi Yadlapati (5):
  dt-bindings: vendor-prefixes: Add prefix for acbel
  dt-bindings: trivial-devices: Add acbel,crps
  hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver
  docs: hwmon: Add documenttaion for acbel-crps PSU
  ARM: dts: aspeed: p10bmc: Change power supply info

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 Documentation/hwmon/acbel-crps.rst            |  81 ++++++++++++++
 arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts  |  12 +--
 drivers/hwmon/pmbus/Kconfig                   |  10 ++
 drivers/hwmon/pmbus/Makefile                  |   1 +
 drivers/hwmon/pmbus/acbel-crps.c              | 102 ++++++++++++++++++
 7 files changed, 204 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/hwmon/acbel-crps.rst
 create mode 100644 drivers/hwmon/pmbus/acbel-crps.c

-- 
2.37.2

[PATCH v2 1/5] dt-bindings: vendor-prefixes: Add prefix for acbel

[Lakshmi Yadlapati]

Add a vendor prefix entry for acbel (https://www.acbel.com)

Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ed64e06ecca4..9dbb8f69be65 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -37,6 +37,8 @@ patternProperties:
     description: Abracon Corporation
   "^abt,.*":
     description: ShenZhen Asia Better Technology Ltd.
+  "^acbel,.*":
+    description: Acbel Polytech Inc.
   "^acer,.*":
     description: Acer Inc.
   "^acme,.*":
-- 
2.37.2

[PATCH v2 2/5] dt-bindings: trivial-devices: Add acbel,crps

[Lakshmi Yadlapati]

Add Acbel CRPS Series power supply to trivial devices.

Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 6f482a254a1d..ae2cf4411b39 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -30,6 +30,8 @@ properties:
     items:
       - enum:
             # SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert and Over Temperature Pin
+          - acbel,crps
+            # Acbel CRPS Series power supply
           - ad,ad7414
             # ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems
           - ad,adm9240
-- 
2.37.2

Re: [PATCH v2 2/5] dt-bindings: trivial-devices: Add acbel,crps

[Krzysztof Kozlowski]

On 20/03/2023 16:40, Lakshmi Yadlapati wrote:
> Add Acbel CRPS Series power supply to trivial devices.
> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6f482a254a1d..ae2cf4411b39 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -30,6 +30,8 @@ properties:
>      items:
>        - enum:
>              # SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert and Over Temperature Pin
> +          - acbel,crps
> +            # Acbel CRPS Series power supply

Wrong placements of comments. This is AD, not Acbel.

>            - ad,ad7414
>              # ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems
>            - ad,adm9240

Best regards,
Krzysztof

RE: [PATCH v2 2/5] dt-bindings: trivial-devices: Add acbel,crps

[Lakshmi Yadlapati]

On 3/20/23, 11:29 AM, "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org <mailto:krzysztof.kozlowski@linaro.org>> wrote:


On 20/03/2023 16:40, Lakshmi Yadlapati wrote:
> Add Acbel CRPS Series power supply to trivial devices.
> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com <mailto:lakshmiy@us.ibm.com>>
> ---
> Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6f482a254a1d..ae2cf4411b39 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -30,6 +30,8 @@ properties:
> items:
> - enum:
> # SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert and Over Temperature Pin
> + - acbel,crps
> + # Acbel CRPS Series power supply


Wrong placements of comments. This is AD, not Acbel.

I will fix it. Thx


> - ad,ad7414
> # ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems
> - ad,adm9240


Best regards,
Krzysztof

Re: [PATCH v2 2/5] dt-bindings: trivial-devices: Add acbel,crps

[Guenter Roeck]

On Mon, Mar 20, 2023 at 10:40:16AM -0500, Lakshmi Yadlapati wrote:
> Add Acbel CRPS Series power supply to trivial devices.
> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6f482a254a1d..ae2cf4411b39 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -30,6 +30,8 @@ properties:
>      items:
>        - enum:
>              # SMBus/I2C Digital Temperature Sensor in 6-Pin SOT with SMBus Alert and Over Temperature Pin
> +          - acbel,crps
> +            # Acbel CRPS Series power supply

This is way too generic. There is not just one Acbel CRPS power supply,
there is a whole lot of them. It is very unlikely that they can all
be described with a single devicetree "compatible" property.

Guenter

>            - ad,ad7414
>              # ADM9240: Complete System Hardware Monitor for uProcessor-Based Systems
>            - ad,adm9240
> -- 
> 2.37.2
> 

[PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[Lakshmi Yadlapati]

Add the driver to support Acbel CRPS power supply.

Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
---
Changes since V1
- Removed debugfs stuff.
- Removed acbel_crps_read_word_data and acbel_crps_read_byte_data.
- Removed PMBUS_MFR_IIN_MAX.
- Added validation for the supported power supply.
- Fix the formatting.

 drivers/hwmon/pmbus/Kconfig      |  10 +++
 drivers/hwmon/pmbus/Makefile     |   1 +
 drivers/hwmon/pmbus/acbel-crps.c | 102 +++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100644 drivers/hwmon/pmbus/acbel-crps.c

diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 59d9a7430499..0215709c3dd2 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -27,6 +27,16 @@ config SENSORS_PMBUS
 	  This driver can also be built as a module. If so, the module will
 	  be called pmbus.
 
+config SENSORS_ACBEL_CRPS
+	tristate "ACBEL CRPS Power Supply"
+	help
+	  If you say yes here you get hardware monitoring support for the ACBEL
+	  Common Redundant Power Supply.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called acbel-crps.
+	  Supported models: FSG032-00xG
+
 config SENSORS_ADM1266
 	tristate "Analog Devices ADM1266 Sequencer"
 	select CRC8
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 3ae019916267..39aef0cb9934 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -5,6 +5,7 @@
 
 obj-$(CONFIG_PMBUS)		+= pmbus_core.o
 obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
+obj-$(CONFIG_SENSORS_ACBEL_CRPS) += acbel-crps.o
 obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
 obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
 obj-$(CONFIG_SENSORS_BEL_PFE)	+= bel-pfe.o
diff --git a/drivers/hwmon/pmbus/acbel-crps.c b/drivers/hwmon/pmbus/acbel-crps.c
new file mode 100644
index 000000000000..ac281699709f
--- /dev/null
+++ b/drivers/hwmon/pmbus/acbel-crps.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2023 IBM Corp.
+ */
+
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pmbus.h>
+#include <linux/hwmon-sysfs.h>
+#include "pmbus.h"
+
+struct acbel_crps {
+	struct i2c_client *client;
+};
+
+static const struct i2c_device_id acbel_crps_id[] = {
+	{ "acbel_crps" },
+	{}
+};
+#define to_psu(x, y) container_of((x), struct acbel_crps, debugfs_entries[(y)])
+
+static const struct file_operations acbel_crps_fops = {
+	.llseek = noop_llseek,
+	.open = simple_open,
+};
+
+static struct pmbus_driver_info acbel_crps_info = {
+	.pages = 1,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
+		   PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | PMBUS_HAVE_POUT |
+		   PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 |
+		   PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP |
+		   PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_FAN12,
+};
+
+static int acbel_crps_probe(struct i2c_client *client)
+{
+	struct acbel_crps *psu;
+	u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
+	struct device *dev = &client->dev;
+	int rc;
+
+	rc = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
+	if (rc < 0) {
+		dev_err(dev, "Failed to read PMBUS_MFR_ID\n");
+		return rc;
+	}
+	if (strncmp(buf, "ACBEL", 5)) {
+		buf[rc] = '\0';
+		dev_err(dev, "Manufacturer '%s' not supported\n", buf);
+		return -ENODEV;
+	}
+
+	rc = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
+	if (rc < 0) {
+		dev_err(dev, "Failed to read PMBUS_MFR_MODEL\n");
+		return rc;
+	}
+
+	if (strncmp(buf, "FSG032", 6)) {
+		buf[rc] = '\0';
+		dev_err(dev, "Model '%s' not supported\n", buf);
+		return -ENODEV;
+	}
+
+	rc = pmbus_do_probe(client, &acbel_crps_info);
+	if (rc)
+		return rc;
+	/*
+         * Don't fail the probe if there isn't enough memory for debugfs.
+         */
+	psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL);
+	if (!psu)
+		return 0;
+
+	return 0;
+}
+
+static const struct of_device_id acbel_crps_of_match[] = {
+	{ .compatible = "acbel,crps" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, acbel_crps_of_match);
+
+static struct i2c_driver acbel_crps_driver = {
+	.driver = {
+		.name = "acbel-crps",
+		.of_match_table = acbel_crps_of_match,
+	},
+	.probe_new = acbel_crps_probe,
+	.id_table = acbel_crps_id,
+};
+
+module_i2c_driver(acbel_crps_driver);
+
+MODULE_AUTHOR("Lakshmi Yadlapati");
+MODULE_DESCRIPTION("PMBus driver for AcBel Power System power supplies");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(PMBUS);
-- 
2.37.2

Re: [PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[kernel test robot]

Hi Lakshmi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230320154019.1943770-4-lakshmiy%40us.ibm.com
patch subject: [PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230321/202303210322.ZZ2VlGIE-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/d873cbdc9f171b066c3f6f6c2a39736e168ad19f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
        git checkout d873cbdc9f171b066c3f6f6c2a39736e168ad19f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/pmbus/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303210322.ZZ2VlGIE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/acbel-crps.c:24:37: warning: 'acbel_crps_fops' defined but not used [-Wunused-const-variable=]
      24 | static const struct file_operations acbel_crps_fops = {
         |                                     ^~~~~~~~~~~~~~~


vim +/acbel_crps_fops +24 drivers/hwmon/pmbus/acbel-crps.c

    23	
  > 24	static const struct file_operations acbel_crps_fops = {
    25		.llseek = noop_llseek,
    26		.open = simple_open,
    27	};
    28	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[Guenter Roeck]

On Mon, Mar 20, 2023 at 10:40:17AM -0500, Lakshmi Yadlapati wrote:
> Add the driver to support Acbel CRPS power supply.
> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> ---
> Changes since V1
> - Removed debugfs stuff.
> - Removed acbel_crps_read_word_data and acbel_crps_read_byte_data.
> - Removed PMBUS_MFR_IIN_MAX.
> - Added validation for the supported power supply.
> - Fix the formatting.
> 
>  drivers/hwmon/pmbus/Kconfig      |  10 +++
>  drivers/hwmon/pmbus/Makefile     |   1 +
>  drivers/hwmon/pmbus/acbel-crps.c | 102 +++++++++++++++++++++++++++++++
>  3 files changed, 113 insertions(+)
>  create mode 100644 drivers/hwmon/pmbus/acbel-crps.c
> 
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 59d9a7430499..0215709c3dd2 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -27,6 +27,16 @@ config SENSORS_PMBUS
>  	  This driver can also be built as a module. If so, the module will
>  	  be called pmbus.
>  
> +config SENSORS_ACBEL_CRPS
> +	tristate "ACBEL CRPS Power Supply"
> +	help
> +	  If you say yes here you get hardware monitoring support for the ACBEL
> +	  Common Redundant Power Supply.
> +

This sounds like there is only one, but ...

> +	  This driver can also be built as a module. If so, the module will
> +	  be called acbel-crps.
> +	  Supported models: FSG032-00xG
> +
... here it says that only one model is (currently) supported.

This should just say "Support for Acbel FSG032-00xG CRPS Power Supply"
and not claim that it supports any others.

I am also not convinced that the Kconfig option driver name should simply
be "crps" There is no guarantee that all crps power supplies from this
vendor will always be supported (supportable) by this driver.


>  config SENSORS_ADM1266
>  	tristate "Analog Devices ADM1266 Sequencer"
>  	select CRC8
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 3ae019916267..39aef0cb9934 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -5,6 +5,7 @@
>  
>  obj-$(CONFIG_PMBUS)		+= pmbus_core.o
>  obj-$(CONFIG_SENSORS_PMBUS)	+= pmbus.o
> +obj-$(CONFIG_SENSORS_ACBEL_CRPS) += acbel-crps.o
>  obj-$(CONFIG_SENSORS_ADM1266)	+= adm1266.o
>  obj-$(CONFIG_SENSORS_ADM1275)	+= adm1275.o
>  obj-$(CONFIG_SENSORS_BEL_PFE)	+= bel-pfe.o
> diff --git a/drivers/hwmon/pmbus/acbel-crps.c b/drivers/hwmon/pmbus/acbel-crps.c
> new file mode 100644
> index 000000000000..ac281699709f
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/acbel-crps.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2023 IBM Corp.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/pmbus.h>
> +#include <linux/hwmon-sysfs.h>

Unused include

> +#include "pmbus.h"
> +
> +struct acbel_crps {
> +	struct i2c_client *client;
> +};
> +
> +static const struct i2c_device_id acbel_crps_id[] = {
> +	{ "acbel_crps" },
> +	{}
> +};
> +#define to_psu(x, y) container_of((x), struct acbel_crps, debugfs_entries[(y)])
> +
> +static const struct file_operations acbel_crps_fops = {
> +	.llseek = noop_llseek,
> +	.open = simple_open,
> +};

The above code is unused.

> +
> +static struct pmbus_driver_info acbel_crps_info = {
> +	.pages = 1,
> +	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_IIN | PMBUS_HAVE_PIN |
> +		   PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | PMBUS_HAVE_POUT |
> +		   PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 |
> +		   PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_VOUT |
> +		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP |
> +		   PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_FAN12,
> +};
> +
> +static int acbel_crps_probe(struct i2c_client *client)
> +{
> +	struct acbel_crps *psu;
> +	u8 buf[I2C_SMBUS_BLOCK_MAX + 1];
> +	struct device *dev = &client->dev;
> +	int rc;
> +
> +	rc = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> +	if (rc < 0) {
> +		dev_err(dev, "Failed to read PMBUS_MFR_ID\n");
> +		return rc;
> +	}
> +	if (strncmp(buf, "ACBEL", 5)) {

this also needs to check for rc
	if (rc < 5 || ...)

> +		buf[rc] = '\0';
> +		dev_err(dev, "Manufacturer '%s' not supported\n", buf);
> +		return -ENODEV;
> +	}
> +
> +	rc = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, buf);
> +	if (rc < 0) {
> +		dev_err(dev, "Failed to read PMBUS_MFR_MODEL\n");
> +		return rc;
> +	}
> +
> +	if (strncmp(buf, "FSG032", 6)) {

	if (rc < 6 || ...)

> +		buf[rc] = '\0';
> +		dev_err(dev, "Model '%s' not supported\n", buf);
> +		return -ENODEV;
> +	}
> +
> +	rc = pmbus_do_probe(client, &acbel_crps_info);
> +	if (rc)
> +		return rc;
> +	/*
> +         * Don't fail the probe if there isn't enough memory for debugfs.
> +         */

Formatting

> +	psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL);
> +	if (!psu)
> +		return 0;

This code doesn't make any sense.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id acbel_crps_of_match[] = {
> +	{ .compatible = "acbel,crps" },

This is way too generic. What if there is some other Acbel power supply
which needs some other options or supports other attributes ?
This needs to be something like "acbel,fsg032" or similar.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, acbel_crps_of_match);
> +
> +static struct i2c_driver acbel_crps_driver = {
> +	.driver = {
> +		.name = "acbel-crps",
> +		.of_match_table = acbel_crps_of_match,
> +	},
> +	.probe_new = acbel_crps_probe,

I think probe_new may be gone.

> +	.id_table = acbel_crps_id,
> +};
> +
> +module_i2c_driver(acbel_crps_driver);
> +
> +MODULE_AUTHOR("Lakshmi Yadlapati");
> +MODULE_DESCRIPTION("PMBus driver for AcBel Power System power supplies");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);
> -- 
> 2.37.2
> 

Re: [PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver

[kernel test robot]

Hi Lakshmi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.3-rc3 next-20230321]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230320154019.1943770-4-lakshmiy%40us.ibm.com
patch subject: [PATCH v2 3/5] hwmon: (pmbus/acbel-crps) Add Acbel CRPS power supply driver
config: riscv-randconfig-c006-20230319 (https://download.01.org/0day-ci/archive/20230321/202303211303.U8Ip4xDC-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/d873cbdc9f171b066c3f6f6c2a39736e168ad19f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
        git checkout d873cbdc9f171b066c3f6f6c2a39736e168ad19f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/hwmon/pmbus/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303211303.U8Ip4xDC-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/acbel-crps.c:24:37: warning: unused variable 'acbel_crps_fops' [-Wunused-const-variable]
   static const struct file_operations acbel_crps_fops = {
                                       ^
   1 warning generated.


vim +/acbel_crps_fops +24 drivers/hwmon/pmbus/acbel-crps.c

    23	
  > 24	static const struct file_operations acbel_crps_fops = {
    25		.llseek = noop_llseek,
    26		.open = simple_open,
    27	};
    28	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v2 4/5] docs: hwmon: Add documenttaion for acbel-crps PSU

[Lakshmi Yadlapati]

Add documentation changes for acbel-crps psu

Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
---
 Documentation/hwmon/acbel-crps.rst | 81 ++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/hwmon/acbel-crps.rst

diff --git a/Documentation/hwmon/acbel-crps.rst b/Documentation/hwmon/acbel-crps.rst
new file mode 100644
index 000000000000..89a43fb88c8c
--- /dev/null
+++ b/Documentation/hwmon/acbel-crps.rst
@@ -0,0 +1,81 @@
+Kernel driver acbel-crps
+=======================
+
+Supported chips:
+
+  * ACBEL Common Redundant Power Supply.
+  Supported models: FSG032-00xG
+
+Author: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
+
+Description
+-----------
+
+This driver supports ACBEL Common Redundant Power Supply. This driver
+is a client to the core PMBus driver.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate the
+devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
+details.
+
+Sysfs entries
+-------------
+
+The following attributes are supported:
+
+======================= ======================================================
+curr1_crit          Critical maximum current.
+curr1_crit_alarm    Input current critical alarm.
+curr1_input         Measured output current.
+curr1_label         "iin"
+curr1_max           Maximum input current.
+curr1_max_alarm     Maximum input current high alarm.
+curr1_rated_max     Maximum rated input current.
+curr2_crit          Critical maximum current.
+curr2_crit_alarm    Output current critical alarm.
+curr2_input         Measured output current.
+curr2_label         "iout1"
+curr2_max           Maximum output current.
+curr2_max_alarm     Output current high alarm.
+curr2_rated_max     Maximum rated output current.
+
+
+fan1_alarm          Fan 1 warning.
+fan1_fault	    Fan 1 fault.
+fan1_input	    Fan 1 speed in RPM.
+fan1_target         Set fan speed reference.
+
+in1_alarm           Input voltage under-voltage alarm.
+in1_input           Measured input voltage.
+in1_label           "vin"
+in1_rated_max       Maximum rated input voltage.
+in1_rated_min       Minimum rated input voltage.
+in2_crit            Critical maximum output voltage.
+in2_crit_alarm      Output voltage critical high alarm.
+in2_input           Measured output voltage.
+in2_label           "vout1"
+in2_lcrit           Critical minimum output voltage.
+in2_lcrit_alarm     Output voltage critical low alarm.
+in2_rated_max       Maximum rated output voltage.
+in2_rated_min       Minimum rated output voltage.
+
+power1_alarm        Input fault or alarm.
+power1_input        Measured input power.
+power1_label        "pin"
+power1_max          Input power limit.
+power1_rated_max    Maximum rated input power.
+power2_crit         Critical output power limit.
+power2_crit_alarm   Output power crit alarm limit exceeded.
+power2_input        Measured output power.
+power2_label        "pout"
+power2_max          Output power limit.
+power2_max_alarm    Output power high alarm.
+power2_rated_max    Maximum rated output power.
+
+temp[1-3]_input     Measured temperature.
+temp[1-2]_max       Maximum temperature.
+temp[1-3]_rated_max Temperature high alarm.
+======================= ======================================================
-- 
2.37.2

Re: [PATCH v2 4/5] docs: hwmon: Add documenttaion for acbel-crps PSU

[Guenter Roeck]

On Mon, Mar 20, 2023 at 10:40:18AM -0500, Lakshmi Yadlapati wrote:
> Add documentation changes for acbel-crps psu
> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> ---
>  Documentation/hwmon/acbel-crps.rst | 81 ++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/hwmon/acbel-crps.rst
> 
> diff --git a/Documentation/hwmon/acbel-crps.rst b/Documentation/hwmon/acbel-crps.rst
> new file mode 100644
> index 000000000000..89a43fb88c8c
> --- /dev/null
> +++ b/Documentation/hwmon/acbel-crps.rst
> @@ -0,0 +1,81 @@
> +Kernel driver acbel-crps
> +=======================
> +
> +Supported chips:
> +
> +  * ACBEL Common Redundant Power Supply.
> +  Supported models: FSG032-00xG
> +
> +Author: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> +
> +Description
> +-----------
> +
> +This driver supports ACBEL Common Redundant Power Supply. This driver
> +is a client to the core PMBus driver.

No, it supports the FSG032-00xG power supply. Neither the driver nor its
documentation should claim otherwise.

> +
> +Usage Notes
> +-----------
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
> +details.
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported:
> +
> +======================= ======================================================
> +curr1_crit          Critical maximum current.
> +curr1_crit_alarm    Input current critical alarm.
> +curr1_input         Measured output current.
> +curr1_label         "iin"
> +curr1_max           Maximum input current.
> +curr1_max_alarm     Maximum input current high alarm.
> +curr1_rated_max     Maximum rated input current.
> +curr2_crit          Critical maximum current.
> +curr2_crit_alarm    Output current critical alarm.
> +curr2_input         Measured output current.
> +curr2_label         "iout1"
> +curr2_max           Maximum output current.
> +curr2_max_alarm     Output current high alarm.
> +curr2_rated_max     Maximum rated output current.
> +
> +
> +fan1_alarm          Fan 1 warning.
> +fan1_fault	    Fan 1 fault.
> +fan1_input	    Fan 1 speed in RPM.
> +fan1_target         Set fan speed reference.
> +
> +in1_alarm           Input voltage under-voltage alarm.
> +in1_input           Measured input voltage.
> +in1_label           "vin"
> +in1_rated_max       Maximum rated input voltage.
> +in1_rated_min       Minimum rated input voltage.
> +in2_crit            Critical maximum output voltage.
> +in2_crit_alarm      Output voltage critical high alarm.
> +in2_input           Measured output voltage.
> +in2_label           "vout1"
> +in2_lcrit           Critical minimum output voltage.
> +in2_lcrit_alarm     Output voltage critical low alarm.
> +in2_rated_max       Maximum rated output voltage.
> +in2_rated_min       Minimum rated output voltage.
> +
> +power1_alarm        Input fault or alarm.
> +power1_input        Measured input power.
> +power1_label        "pin"
> +power1_max          Input power limit.
> +power1_rated_max    Maximum rated input power.
> +power2_crit         Critical output power limit.
> +power2_crit_alarm   Output power crit alarm limit exceeded.
> +power2_input        Measured output power.
> +power2_label        "pout"
> +power2_max          Output power limit.
> +power2_max_alarm    Output power high alarm.
> +power2_rated_max    Maximum rated output power.
> +
> +temp[1-3]_input     Measured temperature.
> +temp[1-2]_max       Maximum temperature.
> +temp[1-3]_rated_max Temperature high alarm.
> +======================= ======================================================
> -- 
> 2.37.2
> 

Re: [PATCH v2 4/5] docs: hwmon: Add documenttaion for acbel-crps PSU

[kernel test robot]

Hi Lakshmi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230320154019.1943770-5-lakshmiy%40us.ibm.com
patch subject: [PATCH v2 4/5] docs: hwmon: Add documenttaion for acbel-crps PSU
reproduce:
        # https://github.com/intel-lab-lkp/linux/commit/e44825c6557c3020131598f7a5e842cbaf29cb1f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Lakshmi-Yadlapati/dt-bindings-vendor-prefixes-Add-prefix-for-acbel/20230320-235222
        git checkout e44825c6557c3020131598f7a5e842cbaf29cb1f
        make menuconfig
        # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
        make htmldocs

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303211050.D9YSILr1-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Documentation/hwmon/acbel-crps.rst:2: WARNING: Title underline too short.
>> Documentation/hwmon/acbel-crps.rst:7: WARNING: Bullet list ends without a blank line; unexpected unindent.
>> Documentation/hwmon/acbel-crps.rst:30: WARNING: Malformed table.
>> Documentation/hwmon/acbel-crps.rst: WARNING: document isn't included in any toctree

vim +2 Documentation/hwmon/acbel-crps.rst

   > 2	=======================
     3	
     4	Supported chips:
     5	
     6	  * ACBEL Common Redundant Power Supply.
   > 7	  Supported models: FSG032-00xG
     8	
     9	Author: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
    10	
    11	Description
    12	-----------
    13	
    14	This driver supports ACBEL Common Redundant Power Supply. This driver
    15	is a client to the core PMBus driver.
    16	
    17	Usage Notes
    18	-----------
    19	
    20	This driver does not auto-detect devices. You will have to instantiate the
    21	devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
    22	details.
    23	
    24	Sysfs entries
    25	-------------
    26	
    27	The following attributes are supported:
    28	
    29	======================= ======================================================
  > 30	curr1_crit          Critical maximum current.
    31	curr1_crit_alarm    Input current critical alarm.
    32	curr1_input         Measured output current.
    33	curr1_label         "iin"
    34	curr1_max           Maximum input current.
    35	curr1_max_alarm     Maximum input current high alarm.
    36	curr1_rated_max     Maximum rated input current.
    37	curr2_crit          Critical maximum current.
    38	curr2_crit_alarm    Output current critical alarm.
    39	curr2_input         Measured output current.
    40	curr2_label         "iout1"
    41	curr2_max           Maximum output current.
    42	curr2_max_alarm     Output current high alarm.
    43	curr2_rated_max     Maximum rated output current.
    44	
    45	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v2 5/5] ARM: dts: aspeed: p10bmc: Change power supply info

[Lakshmi Yadlapati]

Change power supply driver and device address.

Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
index a5be0ee048ec..414191b5aeba 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
@@ -552,14 +552,14 @@ ucd90160@64 {
 &i2c3 {
 	status = "okay";
 
-	power-supply@58 {
-		compatible = "ibm,cffps";
-		reg = <0x58>;
+	power-supply@5a {
+		compatible = "acbel,crps";
+		reg = <0x5a>;
 	};
 
-	power-supply@59 {
-		compatible = "ibm,cffps";
-		reg = <0x59>;
+	power-supply@5b {
+		compatible = "acbel,crps";
+		reg = <0x5b>;
 	};
 };
 
-- 
2.37.2

Re: [PATCH v2 5/5] ARM: dts: aspeed: p10bmc: Change power supply info

[Krzysztof Kozlowski]

On 20/03/2023 16:40, Lakshmi Yadlapati wrote:
> Change power supply driver and device address.

Why? We see what you are doing from the diff. It's unclear why you do it
and that's what for you have commit message.

> 
> Signed-off-by: Lakshmi Yadlapati <lakshmiy@us.ibm.com>
> ---

Best regards,
Krzysztof