Hi Andy, On Mon, Oct 26, 2020 at 06:10:50PM +0200, Andy Shevchenko wrote: > On Sat, Oct 24, 2020 at 12:37:02PM +0300, Laurent Pinchart wrote: > > On Sat, Oct 24, 2020 at 09:50:07AM +0100, Dan Scally wrote: > > > On 24/10/2020 02:24, Laurent Pinchart wrote: > > > > On Mon, Oct 19, 2020 at 11:59:03PM +0100, Daniel Scally wrote: > > > > >> + adev = acpi_dev_get_first_match_dev(supported_devices[i], NULL, -1); > > > > > > > > What if there are multiple sensor of the same model ? > > > > > > Hmm, yeah, that would be a bit of a pickle. I guess the newer > > > smartphones have multiple sensors on the back, which I presume are the > > > same model. So that will probably crop up at some point. How about > > > instead I use bus_for_each_dev() and in the applied function check if > > > the _HID is in the supported list? > > > > Sounds good to me. > > > > > >> + if (!adev) > > > >> + continue; > > Please, don't. > > If we have so weird ACPI tables it must be w/a differently. The all, even badly > formed, ACPI tables I have seen so far are using _UID to distinguish instance > of the device (see second parameter to the above function). > > If we meet the very broken table I would like rather to know about, then > silently think ahead what could be best. > > I.o.w. don't change this until we will have a real example of the problematic > firmware. I'm not sure to follow you. Daniel's current code loops over all the supported HID (as stored in the supported_devices table), and then gets the first ACPI device for each of them. If multiple ACPI devices exist with the same HID, we need to handle them all, so enumerating all ACPI devices and checking whether their HID is one we handle seems to be the right option to me. -- Regards, Laurent Pinchart
