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

Message-ID: <ZyuSzPXnxRYG4Gk3@smile.fi.intel.com>
Date: Wed, 6 Nov 2024 18:01:16 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Alexis Cezar Torreno <alexisczezar.torreno@...log.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, linux-hwmon@...r.kernel.org,
	Radu Sabau <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

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:
> > > ADP1051: 6 PWM for I/O Voltage, I/O Current, Temperature
> > > ADP1055: 6 PWM for I/O Voltage, I/O Current, Power, Temperature
> > 
> > Missing blank line and perhaps you can add Datasheet: tag(s) for these HW?
> > (see `git log --no-merges --grep Datasheet:` for the example)
> 
> 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).

...

> > > +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.

-- 
With Best Regards,
Andy Shevchenko