* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 11:21 ` [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support Radu Sabau
@ 2024-03-18 15:06 ` Guenter Roeck
2024-03-19 10:46 ` Sabau, Radu bogdan
2024-03-18 16:12 ` Krzysztof Kozlowski
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Guenter Roeck @ 2024-03-18 15:06 UTC (permalink / raw)
To: Radu Sabau, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Delphine CC Chiu, linux-hwmon,
devicetree, linux-kernel, linux-doc, linux-i2c
On 3/18/24 04:21, Radu Sabau wrote:
> Add support for ADP1050 Digital Controller for Isolated Power Supplies
> with PMBus interface Voltage, Current and Temperature Monitor.
>
> The ADP1050 implements several features to enable a robust
> system of parallel and redundant operation for customers who
> require high availability. The device can measure voltage,
> current and temperature that can be used in different
> techniques to identify and safely shut down an erroneous
> power supply in parallel operation mode.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> Documentation/hwmon/adp1050.rst | 69 ++++++++++++++++++++
> Documentation/hwmon/index.rst | 1 +
> drivers/hwmon/pmbus/Kconfig | 10 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/adp1050.c | 111 ++++++++++++++++++++++++++++++++
> 5 files changed, 192 insertions(+)
> create mode 100644 Documentation/hwmon/adp1050.rst
> create mode 100644 drivers/hwmon/pmbus/adp1050.c
>
> diff --git a/Documentation/hwmon/adp1050.rst b/Documentation/hwmon/adp1050.rst
> new file mode 100644
> index 000000000000..e3e5bb650a51
> --- /dev/null
> +++ b/Documentation/hwmon/adp1050.rst
> @@ -0,0 +1,69 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver adp1050
> +=====================
> +
> +Supported chips:
> +
> + * Analog Devices ADP1050
> +
> + Prefix: 'adp1050'
> +
> + Addresses scanned: I2C 0x70 - 0x77
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-
> +sheets/ADP1050.pdf
> +
> +Authors:
> +
> + - Radu Sabau <radu.sabau@analog.com>
> +
> +
> +Description
> +-----------
> +
> +This driver supprts hardware monitoring for Analog Devices ADP1050 Digital
> +Controller for Isolated Power Supply with PMBus interface.
> +
> +The ADP1050 is an advanced digital controller with a PMBus™
> +interface targeting high density, high efficiency dc-to-dc power
> +conversion used to monitor system temperatures, voltages and currents.
> +Through the PMBus interface, the device can monitor input/output voltages,
> +input current and temperature.
> +
> +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.
> +
> +The vin scale monitor value and iin scale monitor value can be
> +configured by a device tree property;see
> +Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml for details
> +
> +Platform data support
> +---------------------
> +
> +The driver supports standard PMBus driver platform data.
> +
> +Sysfs Attributes
> +----------------
> +
> +================= ========================================
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_alarm Input voltage alarm
> +in2_label "vout1"
> +in2_input Measured output voltage
> +in2_crit Critical maximum output voltage
> +in2_crit_alarm Output voltage high alarm
> +in2_lcrit Critical minimum output voltage
> +in2_lcrit_alarm Output voltage critical low alarm
> +curr1_label "iin"
> +curr1_input Measured input current.
> +curr1_alarm Input current alarm
> +temp1_input Measured temperature
> +temp1_crit Critical high temperature
> +temp1_crit_alarm Chip temperature critical high alarm
> +================= ========================================
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 1ca7a4fe1f8f..9a4fd576e6f6 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -33,6 +33,7 @@ Hardware Monitoring Kernel Drivers
> adm1266
> adm1275
> adm9240
> + adp1050
> ads7828
> adt7410
> adt7411
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 557ae0c414b0..38e794d83cc3 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -57,6 +57,16 @@ config SENSORS_ADM1275
> This driver can also be built as a module. If so, the module will
> be called adm1275.
>
> +config SENSORS_ADP1050
> + tristate "Analog Devices ADP1050 digital controller for Power Supplies"
> + help
> + If you say yes here you get hardware monitoring support for Analog
> + Devices ADP1050 digital controller for isolated power supply with
> + PMBus interface.
> +
> + This driver can also be built as a module. If so, the module will
> + be called adp1050.
> +
> config SENSORS_BEL_PFE
> tristate "Bel PFE Compatible Power Supplies"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index f14ecf03ad77..95a8dea5e5ed 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
> obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o
> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> +obj-$(CONFIG_SENSORS_ADP1050) += adp1050.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> diff --git a/drivers/hwmon/pmbus/adp1050.c b/drivers/hwmon/pmbus/adp1050.c
> new file mode 100644
> index 000000000000..53198d858156
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/adp1050.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for Analog Devices ADP1050
> + *
> + * Copyright (C) 2024 Analog Devices, Inc.
> + */
> +#include <linux/bits.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include "pmbus.h"
> +
> +#define ADP1050_CHIP_PASSWORD 0xD7
> +
> +#define ADP1050_VIN_SCALE_MONITOR 0xD8
> +#define ADP1050_IIN_SCALE_MONITOR 0xD9
> +
> +static struct pmbus_driver_info adp1050_info = {
> + .pages = 1,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_VOLTAGE_OUT] = linear,
> + .format[PSC_CURRENT_IN] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_TEMP,
> +};
> +
> +static int adp1050_probe(struct i2c_client *client)
> +{
> + u32 vin_scale_monitor, iin_scale_monitor;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_WORD_DATA))
> + return -ENODEV;
> +
> + /* Unlock CHIP's password in order to be able to read/write to it's
> + * VIN_SCALE and IIN_SCALE registers.
> + */
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device can't be unlocked.\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
> + return ret;
> + }
> +
The datasheet says that the factory default password is 0xffff. What if it isn't ?
Refusing to instantiate the chip because it can't be unlocked seems a bit extreme.
After all, the password _can_ be changed.
> + /* If adi,vin-scale-monitor isn't set or is set to 0 means that the
> + * VIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
> + if (device_property_read_u32(&client->dev, "adi,vin-scale-monitor",
> + &vin_scale_monitor))
> + vin_scale_monitor = 0;
> +
> + /* If adi,iin-scale-monitor isn't set or is set to 0 means that the
> + * IIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
> + if (device_property_read_u32(&client->dev, "adi,iin-scale-monitor",
> + &iin_scale_monitor))
> + iin_scale_monitor = 0;
> +
I don't think the "-monitor" extensions in those property names
add any value.
> + ret = i2c_smbus_write_word_data(client, ADP1050_VIN_SCALE_MONITOR,
> + vin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_IIN_SCALE_MONITOR,
> + iin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
If vin and iin monitoring is disabled on purpose, why still set
PMBUS_HAVE_VIN and PMBUS_HAVE_IIN above ? Reporting input values as 0
if the values are explicitly not monitored does not make sense to me.
I am puzzled about this to start with. Unless I am missing something,
the data sheet has no indication that VIN monitoring and/or IIN monitoring
is actually turned off if iin_scale/vin_scale are set to 0. Why support
the option to disable them ? That doesn't make sense to me. Please explain
how/when this makes sense. Please also explain why chip support is made
to depend on devicetree support. Register values may have been be set by
rommon/bios or (much more likely) by manufacturing, or users could just
want to keep the chip default. Why force-override it or even force-disable
it ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 15:06 ` Guenter Roeck
@ 2024-03-19 10:46 ` Sabau, Radu bogdan
0 siblings, 0 replies; 13+ messages in thread
From: Sabau, Radu bogdan @ 2024-03-19 10:46 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Delphine CC Chiu, linux-hwmon,
devicetree, linux-kernel, linux-doc, linux-i2c
-----Original Message-----
From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
Sent: Monday, March 18, 2024 5:07 PM
To: Sabau, Radu bogdan <Radu.Sabau@analog.com>; Jean Delvare <jdelvare@suse.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com>; linux-hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org; linux-i2c@vger.kernel.org
Subject: Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
[External]
On 3/18/24 04:21, Radu Sabau wrote:
> Add support for ADP1050 Digital Controller for Isolated Power Supplies
> with PMBus interface Voltage, Current and Temperature Monitor.
>
> The ADP1050 implements several features to enable a robust
> system of parallel and redundant operation for customers who
> require high availability. The device can measure voltage,
> current and temperature that can be used in different
> techniques to identify and safely shut down an erroneous
> power supply in parallel operation mode.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> Documentation/hwmon/adp1050.rst | 69 ++++++++++++++++++++
> Documentation/hwmon/index.rst | 1 +
> drivers/hwmon/pmbus/Kconfig | 10 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/adp1050.c | 111 ++++++++++++++++++++++++++++++++
> 5 files changed, 192 insertions(+)
> create mode 100644 Documentation/hwmon/adp1050.rst
> create mode 100644 drivers/hwmon/pmbus/adp1050.c
>
> diff --git a/Documentation/hwmon/adp1050.rst b/Documentation/hwmon/adp1050.rst
> new file mode 100644
> index 000000000000..e3e5bb650a51
> --- /dev/null
> +++ b/Documentation/hwmon/adp1050.rst
> @@ -0,0 +1,69 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver adp1050
> +=====================
> +
> +Supported chips:
> +
> + * Analog Devices ADP1050
> +
> + Prefix: 'adp1050'
> +
> + Addresses scanned: I2C 0x70 - 0x77
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-
> +sheets/ADP1050.pdf
> +
> +Authors:
> +
> + - Radu Sabau <radu.sabau@analog.com>
> +
> +
> +Description
> +-----------
> +
> +This driver supprts hardware monitoring for Analog Devices ADP1050 Digital
> +Controller for Isolated Power Supply with PMBus interface.
> +
> +The ADP1050 is an advanced digital controller with a PMBus™
> +interface targeting high density, high efficiency dc-to-dc power
> +conversion used to monitor system temperatures, voltages and currents.
> +Through the PMBus interface, the device can monitor input/output voltages,
> +input current and temperature.
> +
> +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.
> +
> +The vin scale monitor value and iin scale monitor value can be
> +configured by a device tree property;see
> +Documentation/devicetree/bindings/hwmon/pmbus/adi,adp1050.yaml for details
> +
> +Platform data support
> +---------------------
> +
> +The driver supports standard PMBus driver platform data.
> +
> +Sysfs Attributes
> +----------------
> +
> +================= ========================================
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_alarm Input voltage alarm
> +in2_label "vout1"
> +in2_input Measured output voltage
> +in2_crit Critical maximum output voltage
> +in2_crit_alarm Output voltage high alarm
> +in2_lcrit Critical minimum output voltage
> +in2_lcrit_alarm Output voltage critical low alarm
> +curr1_label "iin"
> +curr1_input Measured input current.
> +curr1_alarm Input current alarm
> +temp1_input Measured temperature
> +temp1_crit Critical high temperature
> +temp1_crit_alarm Chip temperature critical high alarm
> +================= ========================================
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 1ca7a4fe1f8f..9a4fd576e6f6 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -33,6 +33,7 @@ Hardware Monitoring Kernel Drivers
> adm1266
> adm1275
> adm9240
> + adp1050
> ads7828
> adt7410
> adt7411
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 557ae0c414b0..38e794d83cc3 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -57,6 +57,16 @@ config SENSORS_ADM1275
> This driver can also be built as a module. If so, the module will
> be called adm1275.
>
> +config SENSORS_ADP1050
> + tristate "Analog Devices ADP1050 digital controller for Power Supplies"
> + help
> + If you say yes here you get hardware monitoring support for Analog
> + Devices ADP1050 digital controller for isolated power supply with
> + PMBus interface.
> +
> + This driver can also be built as a module. If so, the module will
> + be called adp1050.
> +
> config SENSORS_BEL_PFE
> tristate "Bel PFE Compatible Power Supplies"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index f14ecf03ad77..95a8dea5e5ed 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> obj-$(CONFIG_SENSORS_ACBEL_FSG032) += acbel-fsg032.o
> obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o
> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> +obj-$(CONFIG_SENSORS_ADP1050) += adp1050.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o
> obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o
> diff --git a/drivers/hwmon/pmbus/adp1050.c b/drivers/hwmon/pmbus/adp1050.c
> new file mode 100644
> index 000000000000..53198d858156
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/adp1050.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for Analog Devices ADP1050
> + *
> + * Copyright (C) 2024 Analog Devices, Inc.
> + */
> +#include <linux/bits.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include "pmbus.h"
> +
> +#define ADP1050_CHIP_PASSWORD 0xD7
> +
> +#define ADP1050_VIN_SCALE_MONITOR 0xD8
> +#define ADP1050_IIN_SCALE_MONITOR 0xD9
> +
> +static struct pmbus_driver_info adp1050_info = {
> + .pages = 1,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_VOLTAGE_OUT] = linear,
> + .format[PSC_CURRENT_IN] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> + | PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
> + | PMBUS_HAVE_IIN | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_TEMP,
> +};
> +
> +static int adp1050_probe(struct i2c_client *client)
> +{
> + u32 vin_scale_monitor, iin_scale_monitor;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_WORD_DATA))
> + return -ENODEV;
> +
> + /* Unlock CHIP's password in order to be able to read/write to it's
> + * VIN_SCALE and IIN_SCALE registers.
> + */
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device can't be unlocked.\n");
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
> + return ret;
> + }
> +
The datasheet says that the factory default password is 0xffff. What if it isn't ?
Refusing to instantiate the chip because it can't be unlocked seems a bit extreme.
After all, the password _can_ be changed.
This is indeed a bit extreme, will remove it.
> + /* If adi,vin-scale-monitor isn't set or is set to 0 means that the
> + * VIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
> + if (device_property_read_u32(&client->dev, "adi,vin-scale-monitor",
> + &vin_scale_monitor))
> + vin_scale_monitor = 0;
> +
> + /* If adi,iin-scale-monitor isn't set or is set to 0 means that the
> + * IIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
> + if (device_property_read_u32(&client->dev, "adi,iin-scale-monitor",
> + &iin_scale_monitor))
> + iin_scale_monitor = 0;
> +
I don't think the "-monitor" extensions in those property names
add any value.
> + ret = i2c_smbus_write_word_data(client, ADP1050_VIN_SCALE_MONITOR,
> + vin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_IIN_SCALE_MONITOR,
> + iin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
If vin and iin monitoring is disabled on purpose, why still set
PMBUS_HAVE_VIN and PMBUS_HAVE_IIN above ? Reporting input values as 0
if the values are explicitly not monitored does not make sense to me.
I am puzzled about this to start with. Unless I am missing something,
the data sheet has no indication that VIN monitoring and/or IIN monitoring
is actually turned off if iin_scale/vin_scale are set to 0. Why support
the option to disable them ? That doesn't make sense to me. Please explain
how/when this makes sense. Please also explain why chip support is made
to depend on devicetree support. Register values may have been be set by
rommon/bios or (much more likely) by manufacturing, or users could just
want to keep the chip default. Why force-override it or even force-disable
it ?
At first I made the chip support depend on devicetree support because iin_scale
and vin_scale are values that should be calculated depending on external
hardware components. I tested with with ADP1050DC1-EVALZ board and there is
a resistor divider on the VF (VIN) pin. Although now that you said it, chip support
shouldn't depend on devicetree support, I tested the driver in this way and works the
same, in my case I have a resistor divider and have to modify the scale accordingly
in order for correct readings, and from a user point of view I actually found this way
more suitable.
Thanks,
Guenter
Thanks,
Radu
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 11:21 ` [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support Radu Sabau
2024-03-18 15:06 ` Guenter Roeck
@ 2024-03-18 16:12 ` Krzysztof Kozlowski
2024-03-18 16:48 ` Guenter Roeck
2024-03-18 21:28 ` kernel test robot
2024-03-19 0:25 ` kernel test robot
3 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-18 16:12 UTC (permalink / raw)
To: Radu Sabau, Jean Delvare, Guenter Roeck, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Delphine CC Chiu, linux-hwmon, devicetree, linux-kernel,
linux-doc, linux-i2c
On 18/03/2024 12:21, Radu Sabau wrote:
> Add support for ADP1050 Digital Controller for Isolated Power Supplies
> with PMBus interface Voltage, Current and Temperature Monitor.
>
...
> +static int adp1050_probe(struct i2c_client *client)
> +{
> + u32 vin_scale_monitor, iin_scale_monitor;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_WORD_DATA))
> + return -ENODEV;
> +
> + /* Unlock CHIP's password in order to be able to read/write to it's
> + * VIN_SCALE and IIN_SCALE registers.
> + */
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device can't be unlocked.\n");
Syntax is: return dev_err_probe(). Same in other places.
> + return ret;
> + }
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
> + if (ret < 0) {
> + dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
> + return ret;
> + }
> +
> + /* If adi,vin-scale-monitor isn't set or is set to 0 means that the
> + * VIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
Please use Linux coding style comments. /* and aligned */.
> + if (device_property_read_u32(&client->dev, "adi,vin-scale-monitor",
> + &vin_scale_monitor))
> + vin_scale_monitor = 0;
> +
> + /* If adi,iin-scale-monitor isn't set or is set to 0 means that the
> + * IIN monitor isn't used, therefore 0 is used as scale in order
> + * for the readings to return 0.
> + */
> + if (device_property_read_u32(&client->dev, "adi,iin-scale-monitor",
> + &iin_scale_monitor))
> + iin_scale_monitor = 0;
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_VIN_SCALE_MONITOR,
> + vin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_smbus_write_word_data(client, ADP1050_IIN_SCALE_MONITOR,
> + iin_scale_monitor);
> + if (ret < 0)
> + return ret;
> +
> + return pmbus_do_probe(client, &adp1050_info);
> +}
> +
> +static const struct i2c_device_id adp1050_id[] = {
> + {"adp1050", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, adp1050_id);
> +
> +static const struct of_device_id adp1050_of_match[] = {
> + { .compatible = "adi,adp1050"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, adp1050_of_match);
> +
> +static struct i2c_driver adp1050_driver = {
> + .driver = {
> + .name = "adp1050",
> + .of_match_table = of_match_ptr(adp1050_of_match),
Drop of_match_ptr, you will have here warnings.
> + },
> + .probe = adp1050_probe,
> + .id_table = adp1050_id,
> +};
> +module_i2c_driver(adp1050_driver);
> +
> +MODULE_AUTHOR("Radu Sabau <radu.sabau@analog.com>");
> +MODULE_DESCRIPTION("Analog Devices ADP1050 HWMON PMBus Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 16:12 ` Krzysztof Kozlowski
@ 2024-03-18 16:48 ` Guenter Roeck
2024-03-18 18:00 ` Krzysztof Kozlowski
0 siblings, 1 reply; 13+ messages in thread
From: Guenter Roeck @ 2024-03-18 16:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Radu Sabau, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Delphine CC Chiu, linux-hwmon, devicetree, linux-kernel,
linux-doc, linux-i2c
On 3/18/24 09:12, Krzysztof Kozlowski wrote:
> On 18/03/2024 12:21, Radu Sabau wrote:
>> Add support for ADP1050 Digital Controller for Isolated Power Supplies
>> with PMBus interface Voltage, Current and Temperature Monitor.
>>
>
> ...
>
>> +static int adp1050_probe(struct i2c_client *client)
>> +{
>> + u32 vin_scale_monitor, iin_scale_monitor;
>> + int ret;
>> +
>> + if (!i2c_check_functionality(client->adapter,
>> + I2C_FUNC_SMBUS_WRITE_WORD_DATA))
>> + return -ENODEV;
>> +
>> + /* Unlock CHIP's password in order to be able to read/write to it's
>> + * VIN_SCALE and IIN_SCALE registers.
>> + */
>> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
>> + if (ret < 0) {
>> + dev_err_probe(&client->dev, "Device can't be unlocked.\n");
>
> Syntax is: return dev_err_probe(). Same in other places.
>
dev_err_probe() expects the error number as second parameter, so I don't
really understand how the above even compiles.
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 16:48 ` Guenter Roeck
@ 2024-03-18 18:00 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-18 18:00 UTC (permalink / raw)
To: Guenter Roeck, Radu Sabau, Jean Delvare, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Delphine CC Chiu, linux-hwmon, devicetree, linux-kernel,
linux-doc, linux-i2c
On 18/03/2024 17:48, Guenter Roeck wrote:
> On 3/18/24 09:12, Krzysztof Kozlowski wrote:
>> On 18/03/2024 12:21, Radu Sabau wrote:
>>> Add support for ADP1050 Digital Controller for Isolated Power Supplies
>>> with PMBus interface Voltage, Current and Temperature Monitor.
>>>
>>
>> ...
>>
>>> +static int adp1050_probe(struct i2c_client *client)
>>> +{
>>> + u32 vin_scale_monitor, iin_scale_monitor;
>>> + int ret;
>>> +
>>> + if (!i2c_check_functionality(client->adapter,
>>> + I2C_FUNC_SMBUS_WRITE_WORD_DATA))
>>> + return -ENODEV;
>>> +
>>> + /* Unlock CHIP's password in order to be able to read/write to it's
>>> + * VIN_SCALE and IIN_SCALE registers.
>>> + */
>>> + ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
>>> + if (ret < 0) {
>>> + dev_err_probe(&client->dev, "Device can't be unlocked.\n");
>>
>> Syntax is: return dev_err_probe(). Same in other places.
>>
>
> dev_err_probe() expects the error number as second parameter, so I don't
> really understand how the above even compiles.
I did not explain the arguments, because they are obvious... but if you
need so:
return dev_err_probe(&client->dev, ret, "Device can't be unlocked.\n");
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 11:21 ` [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support Radu Sabau
2024-03-18 15:06 ` Guenter Roeck
2024-03-18 16:12 ` Krzysztof Kozlowski
@ 2024-03-18 21:28 ` kernel test robot
2024-03-19 0:25 ` kernel test robot
3 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2024-03-18 21:28 UTC (permalink / raw)
To: Radu Sabau, Jean Delvare, Guenter Roeck, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Delphine CC Chiu, linux-hwmon, devicetree, linux-kernel,
linux-doc, linux-i2c
Cc: oe-kbuild-all
Hi Radu,
kernel test robot noticed the following build errors:
[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on robh/for-next linus/master v6.8 next-20240318]
[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/Radu-Sabau/dt-bindings-hwmon-pmbus-adp1050-add-bindings/20240318-202619
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/20240318112140.385244-3-radu.sabau%40analog.com
patch subject: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240319/202403190552.U4RHYvqc-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240319/202403190552.U4RHYvqc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403190552.U4RHYvqc-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/hwmon/pmbus/adp1050.c: In function 'adp1050_probe':
>> drivers/hwmon/pmbus/adp1050.c:47:45: warning: passing argument 2 of 'dev_err_probe' makes integer from pointer without a cast [-Wint-conversion]
47 | dev_err_probe(&client->dev, "Device can't be unlocked.\n");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| char *
In file included from include/linux/device.h:15,
from include/linux/acpi.h:14,
from include/linux/i2c.h:13,
from drivers/hwmon/pmbus/adp1050.c:9:
include/linux/dev_printk.h:277:64: note: expected 'int' but argument is of type 'char *'
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ~~~~^~~
>> drivers/hwmon/pmbus/adp1050.c:47:17: error: too few arguments to function 'dev_err_probe'
47 | dev_err_probe(&client->dev, "Device can't be unlocked.\n");
| ^~~~~~~~~~~~~
include/linux/dev_printk.h:277:20: note: declared here
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ^~~~~~~~~~~~~
drivers/hwmon/pmbus/adp1050.c:53:45: warning: passing argument 2 of 'dev_err_probe' makes integer from pointer without a cast [-Wint-conversion]
53 | dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| char *
include/linux/dev_printk.h:277:64: note: expected 'int' but argument is of type 'char *'
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ~~~~^~~
drivers/hwmon/pmbus/adp1050.c:53:17: error: too few arguments to function 'dev_err_probe'
53 | dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
| ^~~~~~~~~~~~~
include/linux/dev_printk.h:277:20: note: declared here
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ^~~~~~~~~~~~~
vim +/dev_err_probe +47 drivers/hwmon/pmbus/adp1050.c
> 9 #include <linux/i2c.h>
10 #include <linux/init.h>
11 #include <linux/kernel.h>
12 #include <linux/module.h>
13 #include <linux/of.h>
14 #include "pmbus.h"
15
16 #define ADP1050_CHIP_PASSWORD 0xD7
17
18 #define ADP1050_VIN_SCALE_MONITOR 0xD8
19 #define ADP1050_IIN_SCALE_MONITOR 0xD9
20
21 static struct pmbus_driver_info adp1050_info = {
22 .pages = 1,
23 .format[PSC_VOLTAGE_IN] = linear,
24 .format[PSC_VOLTAGE_OUT] = linear,
25 .format[PSC_CURRENT_IN] = linear,
26 .format[PSC_TEMPERATURE] = linear,
27 .func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
28 | PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT
29 | PMBUS_HAVE_IIN | PMBUS_HAVE_TEMP
30 | PMBUS_HAVE_STATUS_TEMP,
31 };
32
33 static int adp1050_probe(struct i2c_client *client)
34 {
35 u32 vin_scale_monitor, iin_scale_monitor;
36 int ret;
37
38 if (!i2c_check_functionality(client->adapter,
39 I2C_FUNC_SMBUS_WRITE_WORD_DATA))
40 return -ENODEV;
41
42 /* Unlock CHIP's password in order to be able to read/write to it's
43 * VIN_SCALE and IIN_SCALE registers.
44 */
45 ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
46 if (ret < 0) {
> 47 dev_err_probe(&client->dev, "Device can't be unlocked.\n");
48 return ret;
49 }
50
51 ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
52 if (ret < 0) {
53 dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
54 return ret;
55 }
56
57 /* If adi,vin-scale-monitor isn't set or is set to 0 means that the
58 * VIN monitor isn't used, therefore 0 is used as scale in order
59 * for the readings to return 0.
60 */
61 if (device_property_read_u32(&client->dev, "adi,vin-scale-monitor",
62 &vin_scale_monitor))
63 vin_scale_monitor = 0;
64
65 /* If adi,iin-scale-monitor isn't set or is set to 0 means that the
66 * IIN monitor isn't used, therefore 0 is used as scale in order
67 * for the readings to return 0.
68 */
69 if (device_property_read_u32(&client->dev, "adi,iin-scale-monitor",
70 &iin_scale_monitor))
71 iin_scale_monitor = 0;
72
73 ret = i2c_smbus_write_word_data(client, ADP1050_VIN_SCALE_MONITOR,
74 vin_scale_monitor);
75 if (ret < 0)
76 return ret;
77
78 ret = i2c_smbus_write_word_data(client, ADP1050_IIN_SCALE_MONITOR,
79 iin_scale_monitor);
80 if (ret < 0)
81 return ret;
82
83 return pmbus_do_probe(client, &adp1050_info);
84 }
85
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
2024-03-18 11:21 ` [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support Radu Sabau
` (2 preceding siblings ...)
2024-03-18 21:28 ` kernel test robot
@ 2024-03-19 0:25 ` kernel test robot
3 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2024-03-19 0:25 UTC (permalink / raw)
To: Radu Sabau, Jean Delvare, Guenter Roeck, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
Delphine CC Chiu, linux-hwmon, devicetree, linux-kernel,
linux-doc, linux-i2c
Cc: llvm, oe-kbuild-all
Hi Radu,
kernel test robot noticed the following build errors:
[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on robh/for-next linus/master v6.8 next-20240318]
[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/Radu-Sabau/dt-bindings-hwmon-pmbus-adp1050-add-bindings/20240318-202619
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/20240318112140.385244-3-radu.sabau%40analog.com
patch subject: [PATCH 2/2] hwmon: pmbus: adp1050 : Add driver support
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240319/202403190800.h8cSGROp-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8f68022f8e6e54d1aeae4ed301f5a015963089b7)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240319/202403190800.h8cSGROp-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403190800.h8cSGROp-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/hwmon/pmbus/adp1050.c:9:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/hwmon/pmbus/adp1050.c:9:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/hwmon/pmbus/adp1050.c:9:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/hwmon/pmbus/adp1050.c:9:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:20:
In file included from include/linux/mm.h:2188:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/hwmon/pmbus/adp1050.c:47:60: error: too few arguments to function call, expected at least 3, have 2
47 | dev_err_probe(&client->dev, "Device can't be unlocked.\n");
| ~~~~~~~~~~~~~ ^
include/linux/dev_printk.h:277:20: note: 'dev_err_probe' declared here
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/hwmon/pmbus/adp1050.c:53:63: error: too few arguments to function call, expected at least 3, have 2
53 | dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
| ~~~~~~~~~~~~~ ^
include/linux/dev_printk.h:277:20: note: 'dev_err_probe' declared here
277 | __printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
7 warnings and 2 errors generated.
vim +47 drivers/hwmon/pmbus/adp1050.c
32
33 static int adp1050_probe(struct i2c_client *client)
34 {
35 u32 vin_scale_monitor, iin_scale_monitor;
36 int ret;
37
38 if (!i2c_check_functionality(client->adapter,
39 I2C_FUNC_SMBUS_WRITE_WORD_DATA))
40 return -ENODEV;
41
42 /* Unlock CHIP's password in order to be able to read/write to it's
43 * VIN_SCALE and IIN_SCALE registers.
44 */
45 ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
46 if (ret < 0) {
> 47 dev_err_probe(&client->dev, "Device can't be unlocked.\n");
48 return ret;
49 }
50
51 ret = i2c_smbus_write_word_data(client, ADP1050_CHIP_PASSWORD, 0xFFFF);
52 if (ret < 0) {
53 dev_err_probe(&client->dev, "Device couldn't be unlocked.\n");
54 return ret;
55 }
56
57 /* If adi,vin-scale-monitor isn't set or is set to 0 means that the
58 * VIN monitor isn't used, therefore 0 is used as scale in order
59 * for the readings to return 0.
60 */
61 if (device_property_read_u32(&client->dev, "adi,vin-scale-monitor",
62 &vin_scale_monitor))
63 vin_scale_monitor = 0;
64
65 /* If adi,iin-scale-monitor isn't set or is set to 0 means that the
66 * IIN monitor isn't used, therefore 0 is used as scale in order
67 * for the readings to return 0.
68 */
69 if (device_property_read_u32(&client->dev, "adi,iin-scale-monitor",
70 &iin_scale_monitor))
71 iin_scale_monitor = 0;
72
73 ret = i2c_smbus_write_word_data(client, ADP1050_VIN_SCALE_MONITOR,
74 vin_scale_monitor);
75 if (ret < 0)
76 return ret;
77
78 ret = i2c_smbus_write_word_data(client, ADP1050_IIN_SCALE_MONITOR,
79 iin_scale_monitor);
80 if (ret < 0)
81 return ret;
82
83 return pmbus_do_probe(client, &adp1050_info);
84 }
85
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread