Re: [PATCH v5 3/3] iio: light: Add support for APDS9306 Light Sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 21, 2024 at 10:22:50AM +0100, Christophe JAILLET wrote:
> Le 21/01/2024 à 06:17, Subhajit Ghosh a écrit :

...

> > +#define APDS9306_ALS_THRES_VAL_MAX	(BIT(20) - 1)
> > +#define APDS9306_ALS_THRES_VAR_VAL_MAX	(BIT(3) - 1)
> > +#define APDS9306_ALS_PERSIST_VAL_MAX	(BIT(4) - 1)
> 
> Nit: GENMASK()?

I think no, these are plain numbers given in the form of BIT(x) - 1 to show the
HW limit on the values (in bits).

...

> > +	intg_time = iio_gts_find_int_time_by_sel(&data->gts, intg_time_idx);
> > +	if (intg_time < 0)
> > +		delay = apds9306_repeat_rate_period[repeat_rate_idx];
> 
> 'delay' is always overwritten by the line below.

Seems like entire conditional here is not needed as it's implied in the max().
And max() should take both signed or unsigned types, different types might have
a side effect.

> > +	/*
> > +	 * Whichever is greater - integration time period or
> > +	 * sampling period.
> > +	 */
> > +	delay = max(intg_time,
> > +		    apds9306_repeat_rate_period[repeat_rate_idx]);

-- 
With Best Regards,
Andy Shevchenko






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux