Re: [PATCH 2/2] iio: light: ltrf216a: Add LTR-308 support

[Marek Vasut <marex@xxxxxxx>]

On 7/5/24 11:40 AM, Krzysztof Kozlowski wrote:
> On 05/07/2024 11:11, Marek Vasut wrote:
> > Add LiteOn LTR-308 support into LTR-F216A kernel driver.
> >
> > The two devices seem to have almost identical register map, except that
> > the LTR-308 does not have three CLEAR_DATA registers, which are unused
> > by this driver. Furthermore, LTR-308 and LTR-F216A use different lux
> > calculation constants, 0.6 and 0.45 respectively. Both differences are
> > handled using chip info data.
> >
> > https://optoelectronics.liteon.com/upload/download/DS86-2016-0027/LTR-308ALS_Final_%20DS_V1%201.pdf
> > https://optoelectronics.liteon.com/upload/download/DS86-2019-0016/LTR-F216A_Final_DS_V1.4.PDF
> >
> > Signed-off-by: Marek Vasut <marex@xxxxxxx>
> > ---
>
> ...
>
> >   	mutex_init(&data->lock);
> >   
> > @@ -520,15 +537,27 @@ static int ltrf216a_runtime_resume(struct device *dev)
> >   static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend,
> >   				 ltrf216a_runtime_resume, NULL);
> >   
> > +struct ltr_chip_info ltr308_chip_info = {
>
> static const
>
> > +	.has_clear_data		= false,
> > +	.lux_multiplier		= 60,
> > +};
> > +
> > +struct ltr_chip_info ltrf216a_chip_info = {
>
> static const

Both fixed, thanks.

> > +	.has_clear_data		= true,
> > +	.lux_multiplier		= 45,
> > +};
> > +
> >   static const struct i2c_device_id ltrf216a_id[] = {
> > -	{ "ltrf216a" },
> > +	{ "ltr308", .driver_data = (kernel_ulong_t)&ltr308_chip_info },
> > +	{ "ltrf216a", .driver_data = (kernel_ulong_t)&ltrf216a_chip_info },
> >   	{}
> >   };
> >   MODULE_DEVICE_TABLE(i2c, ltrf216a_id);
> >   
> >   static const struct of_device_id ltrf216a_of_match[] = {
> > -	{ .compatible = "liteon,ltrf216a" },
> > -	{ .compatible = "ltr,ltrf216a" },
> > +	{ .compatible = "liteon,ltr308", .data = &ltr308_chip_info },
> > +	{ .compatible = "liteon,ltrf216a", .data = &ltrf216a_chip_info },
> > +	{ .compatible = "ltr,ltrf216a", .data = &ltrf216a_chip_info },
>
> Drop this one. You cannot have undocumented compatibles - and checkpatch
> tells you this - and we do not want to accept stuff just because someone
> made something somewhere (e.g. ACPI, out of tree junk etc). There was
> similar effort in the past and we made it clear.

The "ltr,ltrf216a" was already part of the driver, wouldn't removing it 
be an ABI break ? I will add a separate patch to remove it, so it can be 
reverted if someone complains. Thanks .