On 17/11/2020 16:42, Andy Shevchenko wrote:
> On Tue, Nov 17, 2020 at 2:02 PM Dan Scally <djrscally@xxxxxxxxx> wrote:
>> On 16/11/2020 16:16, Andy Shevchenko wrote:
>>> On Mon, Nov 16, 2020 at 02:15:01PM +0000, Dan Scally wrote:
>>>> On 16/11/2020 14:10, Laurent Pinchart wrote:
>>>>> I thought we were looking for ACPI devices, not companion devices, in
>>>>> order to extract information from the DSDT and store it in a software
>>>>> node. I could very well be wrong though.
>>>> This is correct - the code to fetch the various resources we're looking
>>>> at all uses acpi_device. Whether using Andy's iterator suggestions or
>>>> previous bus_for_each_dev(&acpi_bus_type...) I'm just getting the
>>>> acpi_device via to_acpi_dev() and using that.
>>> If you try to get an I²C ore SPI device out of pure ACPI device (with given
>>> APCI _HID) you will fail. So, it's not correct. You are retrieving companion
>>> devices, while they are still in the struct acpi_device.
>>>
>>> And don't ask me, why it's so. I wasn't designed that and didn't affect any
>>> decision made there.
>> Well, in terms of the actual device we're getting, I don't think we're
>> fundamentally doing anything different between the methods...unless I'm
>> really mistaken.
>>
>>
>> Originally implementation was like:
>>
>>
>> const char *supported_devices[] = {
>>
>> "OVTI2680",
>>
>> };
>>
>>
>> static int cio2_bridge_connect_supported_devices(void)
>>
>> {
>>
>> struct acpi_device *adev;
>>
>> int i;
>>
>> for (i = 0; i < ARRAY_SIZE(supported_devices); i++) {
>>
>> adev =
>> acpi_dev_get_first_match_dev(supported_devices[i], NULL, -1);
>>
>> ...
>>
>> }
>>
>>
>> and acpi_dev_get_first_match_dev() likewise just returns adev via
>> to_acpi_device(dev).
>>
>>
>> So, maybe we don't need to do the iterating over all devices with
>> matching _HID at all, in which case it can be dropped, but if we're
>> doing it then I can't see that it's different to the original
>> implementation in terms of the struct acpi_device we're working with or
>> the route taken to get it.
>>
>>
>> Either way; ACPI maintainers asked to be CC'd on the next patchset
>> anyway, so they'll see what we're doing and be able to weigh in.
> Implementation wise the two approaches are quite similar for now, indeed.
> I would rather go with an iterator approach for a simple reason, EFI
> code already has something which may utilize iterators rather than
> using their home grown solution.
Alright - let's stick with that approach and leave the handling multiple
sensors of same model in then. That's the current state of the code
anyway, and it means it can be used elsewhere too.
