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

[Jonathan Cameron <jic23@kernel.org>]

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

> Hi Andy,
> 
> thanks for the review. I have a question and a comment below.
> 
> Andy Shevchenko <andriy.shevchenko@linux.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
>