linux-kernel - RE: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055

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:
 <PH0PR03MB63514E801BD8D2C08BE206E7F15C2@PH0PR03MB6351.namprd03.prod.outlook.com>
Date: Thu, 7 Nov 2024 08:34:31 +0000
From: "Torreno, Alexis Czezar" <AlexisCzezar.Torreno@...log.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Guenter Roeck <linux@...ck-us.net>,
        "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>,
        "Sabau, Radu bogdan" <Radu.Sabau@...log.com>,
        Jean Delvare <jdelvare@...e.com>, Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Jonathan
 Corbet <corbet@....net>,
        Uwe Kleine-König
	<u.kleine-koenig@...gutronix.de>
Subject: RE: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and adp1055



> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Sent: Thursday, November 7, 2024 4:05 PM
> To: Torreno, Alexis Czezar <AlexisCzezar.Torreno@...log.com>
> Cc: Guenter Roeck <linux@...ck-us.net>; linux-doc@...r.kernel.org; linux-
> kernel@...r.kernel.org; devicetree@...r.kernel.org; linux-
> hwmon@...r.kernel.org; Sabau, Radu bogdan <Radu.Sabau@...log.com>;
> Jean Delvare <jdelvare@...e.com>; Rob Herring <robh@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>;
> Jonathan Corbet <corbet@....net>; Uwe Kleine-König <u.kleine-
> koenig@...gutronix.de>
> Subject: Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and
> adp1055
> 
> [External]
> 
> On Thu, Nov 07, 2024 at 01:17:04AM +0000, Torreno, Alexis Czezar wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > > Sent: Thursday, November 7, 2024 12:01 AM On Wed, Nov 06, 2024 at
> > > 07:55:30AM -0800, Guenter Roeck wrote:
> > > > On 11/6/24 03:24, Andy Shevchenko wrote:
> > > > > On Wed, Nov 06, 2024 at 05:03:11PM +0800, Alexis Cezar Torreno
> wrote:
> 
> ...
> 
> > > > Is that an official tag ? Frankly, if so, I think it is quite
> > > > useless in the patch description because datasheet locations keep
> changing.
> > > > I think it is much better to provide a link in the driver documentation.
> > >
> > > I believe it's semi-official, meaning that people use it from time to time.
> > > I'm fine with the Link in the documentation. Actually with any
> > > solution that saves the respective link in the kernel source tree
> > > (either in form of commit message or documentation / comments in the
> code).
> >
> > Will add the blank line after description.
> > Am I right to understand that we leave this as is? No need to add
> > driver link in patch description since it is in driver documentation?
> 
> Add it to the documentation.
> 

Already added the links for the datasheets in the documentation.

> ...
> 
> > > > > > +static struct pmbus_driver_info adp1055_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_VIN | PMBUS_HAVE_IIN |
> > > PMBUS_HAVE_VOUT
> > > > > > +		   | PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP2 |
> > > PMBUS_HAVE_TEMP3
> > > > > > +		   | PMBUS_HAVE_POUT |
> PMBUS_HAVE_STATUS_VOUT
> > > > > > +		   | PMBUS_HAVE_STATUS_IOUT |
> > > PMBUS_HAVE_STATUS_INPUT
> > > > > > +		   | PMBUS_HAVE_STATUS_TEMP,
> > > > >
> > > > > Ditto.
> > > >
> > > > That one slipped through with the original driver submission.
> > > > I thought that checkpatch complains about that, but it turns out
> > > > that it doesn't. I agree, though, that the usual style should be used.
> > >
> > > Oh, okay, that's up to you then.
> 
> > I based my code style on the original, but I agree that the usual
> > style should be followed.
> 
> > I will change it to follow the usual style.
> 
> No, please be consistent with the existing style. If you want to change it, add an
> additional patch to do that for the _existing_ code first and base your patch on
> top of that.
> 

I see, I'll keep it consistent with the existing style.

Thank you, will send an updated patch soon.

Regards,
Alexis

> > Should I leave the original untouched or should I format it too?
> 
> --
> With Best Regards,
> Andy Shevchenko
>