Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

Limonciello, Mario
2023-06-21T16:41:09Z

On 6/21/2023 11:31 AM, Andrew Lunn wrote:
>> I think there is enough details for this to happen. It's done
>> so that either the AML can natively behave as a consumer or a
>> driver can behave as a consumer.
>>>>>> +/**
>>>>>> + * APIs needed by drivers/subsystems for contributing frequencies:
>>>>>> + * During probe, check `wbrf_supported_producer` to see if WBRF is supported.
>>>>>> + * If adding frequencies, then call `wbrf_add_exclusion` with the
>>>>>> + * start and end points specified for the frequency ranges added.
>>>>>> + * If removing frequencies, then call `wbrf_remove_exclusion` with
>>>>>> + * start and end points specified for the frequency ranges added.
>>>>>> + */
>>>>>> +bool wbrf_supported_producer(struct acpi_device *adev);
>>>>>> +int wbrf_add_exclusion(struct acpi_device *adev,
>>>>>> + struct wbrf_ranges_in *in);
>>>>>> +int wbrf_remove_exclusion(struct acpi_device *adev,
>>>>>> + struct wbrf_ranges_in *in);
>>>>> Could struct device be used here, to make the API agnostic to where
>>>>> the information is coming from? That would then allow somebody in the
>>>>> future to implement a device tree based information provider.
>>>> That does make sense, and it wouldn't even be that much harder if we
>>>> assume in a given platform there's only one provider
>>> That seems like a very reasonable assumption. It is theoretically
>>> possible to build an ACPI + DT hybrid, but i've never seen it actually
>>> done.
>>>
>>> If an ARM64 ACPI BIOS could implement this, then i would guess the low
>>> level bits would be solved, i guess jumping into the EL1
>>> firmware. Putting DT on top instead should not be too hard.
>>>
>>> Andrew
>> To make life easier I'll ask whether we can include snippets of
>> the matching ASL for this first implementation as part of the
>> public ACPI spec that matches this code when we release it.
> So it sounds like you are pretty open about this, there should be
> enough information for independent implementations. So please do make
> the APIs between the providers and the consumers abstract, struct
> device, not an ACPI object.
>
> Andrew
Think a little more about what a non-ACPI implementation
would look like:

1) Would producers and consumers still need you to set CONFIG_ACPI_WBRF?
2) How would you indicate you need WBRF support?
3) How would notifications from one device to another work?

I don't think those are trivial problems that can be solved by
just making the pointer 'struct device' particularly as with the
ACPI implementation consumers are expecting the notification from
ACPI.