Hi,
On 7/28/21 11:08 AM, Hans de Goede wrote:
> Hi,
>
> On 7/23/21 10:21 PM, Michael Bottini wrote:
>> Some products in the field, like Intel Rocket Lake systems, contain
>> AML code that can modify _DSD properties after they have been
>> evaluated by ACPI init code. Therefore, there is a need for drivers
>> to be able to reevaluate _DSDs so that the updated property values can
>> be read. Export acpi_init_properties() for this purpose.
>>
>> Signed-off-by: Michael Bottini <michael.a.bottini@xxxxxxxxxxxxxxx>
>
> My first instinct here is this is a firmware bug and we should
> go out of our way here to not support this and to instead apply
> pressure on the vendor to get the firmware fixed.
>
> Let me explain, the standard use of _DSD is to allow embedding
> open-firmware/devicetree style properties inside ACPI nodes.
>
> devicetree files, unlike AML contain static information, which
> is parsed once and only once.
>
> Allowing AML code to dynamically change _DSD results pretty
> much breaks this entire model.
>
> So I might be shooting from the hip a bit here:
> "no, just no". IOW nack.
I should have read the rest of the thread first I guess.
I see that Andy and Rafael are saying the same thing.
So here we have 3 people who all 3 are somewhat experts in ACPI
saying no to this. So yes please talk to the BIOS team as you
indicated elsewhere in the thread.
Regards,
Hans
>> ---
>> drivers/acpi/property.c | 1 +
>> include/linux/acpi.h | 6 ++++++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
>> index e312ebaed8db..2c1f8cf1a8f0 100644
>> --- a/drivers/acpi/property.c
>> +++ b/drivers/acpi/property.c
>> @@ -432,6 +432,7 @@ void acpi_init_properties(struct acpi_device *adev)
>> if (!adev->data.pointer)
>> acpi_extract_apple_properties(adev);
>> }
>> +EXPORT_SYMBOL(acpi_init_properties);
>>
>> static void acpi_destroy_nondev_subnodes(struct list_head *list)
>> {
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 72e4f7fd268c..57defc3bc9b9 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -716,6 +716,8 @@ static inline u64 acpi_arch_get_root_pointer(void)
>>
>> int acpi_get_local_address(acpi_handle handle, u32 *addr);
>>
>> +void acpi_init_properties(struct acpi_device *adev);
>> +
>> #else /* !CONFIG_ACPI */
>>
>> #define acpi_disabled 1
>> @@ -976,6 +978,10 @@ static inline int acpi_get_local_address(acpi_handle handle, u32 *addr)
>> return -ENODEV;
>> }
>>
>> +static inline void acpi_init_properties(struct acpi_device *adev)
>> +{
>> +}
>> +
>> #endif /* !CONFIG_ACPI */
>>
>> #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>>
