2023-03-20 15:50:20

by Lakshmi Yadlapati

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

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



2023-03-20 15:50:27

by Lakshmi Yadlapati

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

Add Acbel CRPS Series power supply to trivial devices.

Signed-off-by: Lakshmi Yadlapati <[email protected]>
---
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


2023-03-20 15:50:32

by Lakshmi Yadlapati

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

Add documentation changes for acbel-crps psu

Signed-off-by: Lakshmi Yadlapati <[email protected]>
---
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 <[email protected]>
+
+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


2023-03-20 15:50:37

by Lakshmi Yadlapati

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

Add the driver to support Acbel CRPS power supply.

Signed-off-by: Lakshmi Yadlapati <[email protected]>
---
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


2023-03-20 15:50:44

by Lakshmi Yadlapati

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

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

Signed-off-by: Lakshmi Yadlapati <[email protected]>
---
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


2023-03-20 15:50:49

by Lakshmi Yadlapati

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

Change power supply driver and device address.

Signed-off-by: Lakshmi Yadlapati <[email protected]>
---
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


2023-03-20 16:36:43

by Krzysztof Kozlowski

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

On 20/03/2023 16:40, Lakshmi Yadlapati wrote:
> Add Acbel CRPS Series power supply to trivial devices.
>
> Signed-off-by: Lakshmi Yadlapati <[email protected]>
> ---
> 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


2023-03-20 16:37:18

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---

Best regards,
Krzysztof


2023-03-20 17:12:27

by Lakshmi Yadlapati

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



On 3/20/23, 11:29 AM, "Krzysztof Kozlowski" <[email protected] <mailto:[email protected]>> wrote:


On 20/03/2023 16:40, Lakshmi Yadlapati wrote:
> Add Acbel CRPS Series power supply to trivial devices.
>
> Signed-off-by: Lakshmi Yadlapati <[email protected] <mailto:[email protected]>>
> ---
> 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





2023-03-20 19:46:51

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2023-03-20 20:31:44

by Guenter Roeck

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

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 <[email protected]>
> ---
> 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
>

2023-03-20 20:32:55

by Guenter Roeck

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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
>

2023-03-20 20:41:36

by Guenter Roeck

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

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 <[email protected]>
> ---
> 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
>

2023-03-21 02:18:18

by kernel test robot

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

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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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 <[email protected]>
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

2023-03-21 05:32:25

by kernel test robot

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

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/[email protected]/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 <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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