linux-kernel - Re: [PATCH v4 2/3] iio: light: add support for veml6046x00 RGBIR color sensor

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: <20250525151633.18b82382@jic23-huawei>
Date: Sun, 25 May 2025 15:16:33 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andreas Klinger <ak@...klinger.de>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, lars@...afoo.de,
 javier.carrasco.cruz@...il.com, mazziesaccount@...il.com,
 arthur.becker@...tec.com, perdaniel.olsson@...s.com,
 mgonellabolduc@...onoff.com, muditsharma.info@...il.com,
 clamor95@...il.com, emil.gedenryd@...s.com, devicetree@...r.kernel.org,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] iio: light: add support for veml6046x00 RGBIR
 color sensor

On Sun, 25 May 2025 09:34:52 +0200
Andreas Klinger <ak@...klinger.de> wrote:

> Hi Andy,
> 
> thanks for the review. I have a question and a comment below.
> 
> Andy Shevchenko <andriy.shevchenko@...ux.intel.com> schrieb am Mo, 19. Mai 14:04:
> > > +/*
> > > + * veml6046x00_gain_pd - translation from gain index (used in the driver) to
> > > + * gain (sensor) and PD
> > > + * @gain_sen:	Gain used in the sensor as described in the datasheet of the
> > > + *		sensor
> > > + * @pd:		Photodiode size in the sensor  
> > 
> > This is made to look like kernel-doc, but it's not marked as a such, why?  
> 
> I'll remove the '@'

Better to make it kernel doc!  That is add /** and check for
errors using scripts/kernel-doc 


> 
> ...
> 
> > > +	ret = regmap_clear_bits(data->regmap, VEML6046X00_REG_CONF0,
> > > +							VEML6046X00_CONF0_ON_0);  
> > 
> > Something wrong with the indentation. Please, fix all places like this...
> >   
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to set bit for power on %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	return regmap_clear_bits(data->regmap, VEML6046X00_REG_CONF1,
> > > +							VEML6046X00_CONF1_ON_1);  
> > 
> > ...or like this.
> >   
> > > +}  
> 
> I don't get the point what is wrong with the indentation. In the coding-style it
> says the decendant line should be placed to the right.
> Did i miss something?

Where there are no other constraints it should be aligned under the first parameter
on the line above.

	return regmap_clear_bits(data->regmap, VEML6046X00_REG_CONF1,
				 VEML6046X00_CONF1_ON_1);  

This is one of those things that has never been added explicitly to the coding style
doc because there lots of subtle corner cases where it isn't appropriate.
But all significantly to the right usually means is 1 tab or more.
Where possible most people prefer the above style.

Jonathan

> 
> Best regards,
> 
> Andreas
>