linux-kernel - Re: [PATCH 1/2] hwmon: (pmbus) Add support for MPS multi-phase mp2869a/mp29612a controllers

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] [day] [month] [year] [list]

Message-ID: <331d77c4-fa53-4d3b-a17e-22481bf72d32@roeck-us.net>
Date: Wed, 25 Jun 2025 06:11:11 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: 吳梓豪 <tzuhao.wtmh@...il.com>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Naresh Solanki <naresh.solanki@...ements.com>,
	Rodrigo Gobbi <rodrigo.gobbi.7@...il.com>,
	Michal Simek <michal.simek@....com>,
	Fabio Estevam <festevam@...il.com>,
	Henry Wu <Henry_Wu@...ntatw.com>,
	Grant Peltier <grantpeltier93@...il.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Cedric Encarnacion <cedricjustine.encarnacion@...log.com>,
	Kim Seer Paller <kimseer.paller@...log.com>,
	Leo Yang <leo.yang.sy0@...il.com>,
	Ninad Palsule <ninad@...ux.ibm.com>,
	Alex Vdovydchenko <xzeol@...oo.com>,
	John Erasmus Mari Geronimo <johnerasmusmari.geronimo@...log.com>,
	Nuno Sa <nuno.sa@...log.com>, Jerome Brunet <jbrunet@...libre.com>,
	Noah Wang <noahwang.wang@...look.com>,
	Mariel Tinaco <Mariel.Tinaco@...log.com>,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 1/2] hwmon: (pmbus) Add support for MPS multi-phase
 mp2869a/mp29612a controllers

On Wed, Jun 25, 2025 at 02:31:02PM +0800, 吳梓豪 wrote:
> Krzysztof Kozlowski <krzk@...nel.org> 於 2025年6月24日 週二 下午3:48寫道：
> >
> > On 24/06/2025 09:41, tzuhao.wtmh@...il.com wrote:
> > > +static int
> > > +MP2869A_read_byte_data(struct i2c_client *client, int page, int reg)
> > > +{
> > > +     switch (reg) {
> > > +     case PMBUS_VOUT_MODE:
> > > +             /* Enforce VOUT direct format. */
> > > +             return PB_VOUT_MODE_DIRECT;
> > > +     default:
> > > +             return -ENODATA;
> > > +     }
> > > +}
> > > +
> > > +static int
> > > +MP2869A_identify_vout_format(struct i2c_client *client,
> >
> > Use Linux coding style, so lowercase for variables, types and functions.
> > Everywhere (except when coding style tells you different, so please read
> > it).
> >
> > > +                         struct MP2869A_data *data)
> > > +{
> > > +     int i, ret;
> > > +
> > > +     for (i = 0; i < data->info.pages; i++) {
> > > +             ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
> > > +             if (ret < 0)
> > > +                     return ret;
> > > +
> > > +             ret = i2c_smbus_read_word_data(client, MP2869A_VOUT_MODE);
> > > +             if (ret < 0)
> > > +                     return ret;
> > > +
> > > +             switch (ret & MP2869A_VOUT_MODE_MASK) {
> > > +             case MP2869A_VOUT_MODE_VID:
> > > +                     data->vout_format[i] = vid;
> > > +                     break;
> > > +             default:
> > > +             return -EINVAL;
> > > +             }
> > > +             }
> >
> > Messed indentation in multiple places.
> >
> > > +     return 0;
> > > +}
> > > +
> > > +static struct pmbus_driver_info MP2869A_info = {
> >
> > This is const.
> Since info will be modified by mp2869a_read_vout at runtime, I chose
> not to make it constant

That is a no-go. There can be multiple instances of the chip in a system,
each requiring its own info data structure. If the structure is modified
at runtime it needs to be copied first.

Guenter