linux-kernel - Re: [PATCH v3] hwmon: (pmbus/mp2975) Fix driver initialization for MP2975 device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 27 Jan 2024 09:56:31 +0100
From: Patrick Rudolph <patrick.rudolph@...ements.com>
To: Konstantin Aladyshev <aladyshev22@...il.com>
Cc: Guenter Roeck <linux@...ck-us.net>, Jean Delvare <jdelvare@...e.com>, 
	Naresh Solanki <Naresh.Solanki@...ements.com>, linux-hwmon@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] hwmon: (pmbus/mp2975) Fix driver initialization for
 MP2975 device

Hi Konstantin,
Thank you for fixing this regression.

The comment is no longer true as the driver doesn't internally convert
from VID to direct,
but rather configures READ_VOUT using MFR_DC_LOOP_CTRL.

The comment thus should read as the following:

Report direct format as configured by MFR_DC_LOOP_CTRL.
Unlike on MP2971/MP2973 the reported VOUT_MODE isn't automatically
internally updated,
but always reads as PB_VOUT_MODE_VID.

Regards,
Patrick
On Fri, Jan 26, 2024 at 9:59 PM Konstantin Aladyshev
<aladyshev22@...il.com> wrote:
>
> The commit 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code")
> has introduced a bug that makes it impossible to initialize MP2975
> device:
> """
> mp2975 5-0020: Failed to identify chip capabilities
> i2c i2c-5: new_device: Instantiated device mp2975 at 0x20
> i2c i2c-5: delete_device: Deleting device mp2975 at 0x20
> """
> Since the 'read_byte_data' function was removed from the
> 'pmbus_driver_info ' structure the driver no longer reports correctly
> that VOUT mode is direct. Therefore 'pmbus_identify_common' fails
> with error, making it impossible to initialize the device.
>
> Restore 'read_byte_data' function to fix the issue.
>
> Tested:
> - before: it is not possible to initialize MP2975 device with the
> 'mp2975' driver,
> - after: 'mp2975' correctly initializes MP2975 device and all sensor
> data is correct.
>
> Fixes: 1feb31e810b0 ("hwmon: (pmbus/mp2975) Simplify VOUT code")
>
> Signed-off-by: Konstantin Aladyshev <aladyshev22@...il.com>
> ---
> Changes in v3:
>  - Drop accidentally added file from patch
>
> Changes in v2:
>  - Fix indentation issues
>
>  drivers/hwmon/pmbus/mp2975.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/mp2975.c b/drivers/hwmon/pmbus/mp2975.c
> index b9bb469e2d8f..5bbfdacb61a7 100644
> --- a/drivers/hwmon/pmbus/mp2975.c
> +++ b/drivers/hwmon/pmbus/mp2975.c
> @@ -126,6 +126,22 @@ static const struct regulator_desc __maybe_unused mp2975_reg_desc[] = {
>
>  #define to_mp2975_data(x)  container_of(x, struct mp2975_data, info)
>
> +static int mp2975_read_byte_data(struct i2c_client *client, int page, int reg)
> +{
> +       switch (reg) {
> +       case PMBUS_VOUT_MODE:
> +               /*
> +                * Enforce VOUT direct format, since device allows to set the
> +                * different formats for the different rails. Conversion from
> +                * VID to direct provided by driver internally, in case it is
> +                * necessary.
> +                */
> +               return PB_VOUT_MODE_DIRECT;
> +       default:
> +               return -ENODATA;
> +       }
> +}
> +
>  static int
>  mp2975_read_word_helper(struct i2c_client *client, int page, int phase, u8 reg,
>                         u16 mask)
> @@ -869,6 +885,7 @@ static struct pmbus_driver_info mp2975_info = {
>                 PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
>                 PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_POUT |
>                 PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT | PMBUS_PHASE_VIRTUAL,
> +       .read_byte_data = mp2975_read_byte_data,
>         .read_word_data = mp2975_read_word_data,
>  #if IS_ENABLED(CONFIG_SENSORS_MP2975_REGULATOR)
>         .num_regulators = 1,
> --
> 2.34.1
>