Re: [PATCH] iio: light: ltrf216a: Document the device name

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 22 Jul 2024 15:31:20 +0100
"Shreeya Patel" <shreeya.patel@collabora.com> wrote:

> On Saturday, July 20, 2024 18:52 IST, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Mon, 15 Jul 2024 01:07:09 +0530
> > Shreeya Patel <shreeya.patel@collabora.com> wrote:
> >   
> > > Compatible 'ltr,ltrf216a' is used by Valve's Steamdeck device
> > > via the ACPI + PRP0001 mechanism.
> > > Document the device name alognside the compatible.
> > > 
> > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>  
> > Hi Shreeya,
> > 
> > I'm guessing you used get_maintainers for this.  Always check that there
> > isn't anyone else who should be added.
> > 
> > Given the root of this was the discussion about dropping this undocumented
> > binding the DT maintainers and list should definitely be CC'd + Marek
> > who was part of the earlier discussion.
> >   
> 
> Sorry about that, I used get_maintainers like you said but I'll keep this in
> mind next time.
> 
> > The fact that this isn't a typical ID from a Windows consumer device does
> > change things a little, but I'd imagine a firmware update for the steamdeck
> > is equally unlikely to happen in order to fix the vendor-id.
> > (I was surprised to discover they provide windows drivers for it but
> > not seeing the light sensor in the list)
> > 
> > So my opinion is that we should keep this ID in place with an enhanced
> > description.
> > 
> > The other suggestion from the Krzysztof's thread was the risk of this getting
> > abused.  Perhaps we should specifically fail to probe if that ID is used and
> > it's not an ACPI firmware?
> >   
> 
> Rob said in another thread that use of it can now not pass the validation
> with dtschema. So I'm guessing we don't need to add any extra checks anymore?
That's how I interpreted Rob's reply as well.

> 
> However, I'll send a v2 with enhanced description alongside the ID.
Great.

J
> 
> Thanks,
> Shreeya Patel
> 
> >   
> > > ---
> > >  drivers/iio/light/ltrf216a.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c
> > > index 68dc48420a88..e97ff8f7ecd0 100644
> > > --- a/drivers/iio/light/ltrf216a.c
> > > +++ b/drivers/iio/light/ltrf216a.c
> > > @@ -528,7 +528,7 @@ MODULE_DEVICE_TABLE(i2c, ltrf216a_id);
> > >  
> > >  static const struct of_device_id ltrf216a_of_match[] = {
> > >  	{ .compatible = "liteon,ltrf216a" },
> > > -	{ .compatible = "ltr,ltrf216a" },
> > > +	{ .compatible = "ltr,ltrf216a" }, /* For Valve's Steamdeck device */  
> > 
> > Add that the steamdeck is an ACPI platform using PRP0001 as that's
> > the key part that is different from a normal 'wrong' of_device_id.
> > 
> >   
> > >  	{}
> > >  };
> > >  MODULE_DEVICE_TABLE(of, ltrf216a_of_match);  
> >  
>