> -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Sent: Thursday, November 7, 2024 12:01 AM > To: Guenter Roeck <linux@xxxxxxxxxxxx> > Cc: Torreno, Alexis Czezar <AlexisCzezar.Torreno@xxxxxxxxxx>; linux- > doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx; Sabau, Radu > bogdan <Radu.Sabau@xxxxxxxxxx>; Jean Delvare <jdelvare@xxxxxxxx>; Rob > Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor > Dooley <conor+dt@xxxxxxxxxx>; Jonathan Corbet <corbet@xxxxxxx>; Uwe > Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > Subject: Re: [PATCH 2/2] hwmon: (pmbus/adp1050): Support adp1051 and > adp1055 > > [External] > > 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). > 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? > ... > > > > > +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 > 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. Should I leave the original untouched or should I format it too? Regards, Alexis