Re: [PATCH] mfd: kempld-core: don't replace resources provided by ACPI

[Michael Brunner]

Added Lee Jones again, as I erroneously used an old eMail address in
my original mail.

Thanks Guenter, for the review!

On Wed, 2024-02-21 at 09:34 -0800, Guenter Roeck wrote:
>> The current implementation to retrieve ACPI resources is faulty
>> and may cause issues that even can lead to non-booting systems.
>>
>> When adding data from an ACPI device, the resources are already
>> assigned to the platform device. Therefore there is no need to
>> retrieve the resource list from ACPI and manually assign it to
>> the platform device. Also there shouldn't be any BIOS in the wild
>> anymore, that does not have resources added to the KEMPLD ACPI
>> data.
>>
>> In particular this fixes an issue where the retrieval of the
>> resource list using /proc/ioports is disturbed and does not list
>> the assigned resource for the kempld device or even no resources
>> at all.
>> On some distributions this also leads to problems during system
>> initialization (e.g. with udev) and causes the system to not
>> boot at all.
>>
>> I have reproduced the issue with the following kernel versions:
>>      5.10.209
>>      5.15.148
>>      6.1.25
>>      6.6.17
>>      6.7.5
>>      6.8-rc5
>>
>> The patch applies to all of those versions and seems to resolve
>> the issue.
>>
>> Signed-off-by: Michael Brunner <michael.brunner@xxxxxxxxxxx>
>
>Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>
>> ---
>>   drivers/mfd/kempld-core.c | 37 -------------------------------------
>>   1 file changed, 37 deletions(-)
>>
>> diff --git a/drivers/mfd/kempld-core.c b/drivers/mfd/kempld-core.c
>> index 67af36a38913..5557f023a173 100644
>> --- a/drivers/mfd/kempld-core.c
>> +++ b/drivers/mfd/kempld-core.c
>> @@ -428,50 +428,13 @@ static int kempld_detect_device(struct kempld_device_data *pld)
>>   #ifdef CONFIG_ACPI
>>   static int kempld_get_acpi_data(struct platform_device *pdev)
>>   {
>> -     struct list_head resource_list;
>> -     struct resource *resources;
>> -     struct resource_entry *rentry;
>>       struct device *dev = &pdev->dev;
>> -     struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
>>       const struct kempld_platform_data *pdata;
>>       int ret;
>> -     int count;
>>
>>       pdata = acpi_device_get_match_data(dev);
>>       ret = platform_device_add_data(pdev, pdata,
>>                                      sizeof(struct kempld_platform_data));
>> -     if (ret)
>> -             return ret;
>> -
>> -     INIT_LIST_HEAD(&resource_list);
>> -     ret = acpi_dev_get_resources(acpi_dev, &resource_list, NULL, NULL);
>> -     if (ret < 0)
>> -             goto out;
>> -
>> -     count = ret;
>> -
>> -     if (count == 0) {
>> -             ret = platform_device_add_resources(pdev, pdata->ioresource, 1);
>> -             goto out;
>> -     }
>> -
>> -     resources = devm_kcalloc(&acpi_dev->dev, count, sizeof(*resources),
>> -                              GFP_KERNEL);
>> -     if (!resources) {
>> -             ret = -ENOMEM;
>> -             goto out;
>> -     }
>> -
>> -     count = 0;
>> -     list_for_each_entry(rentry, &resource_list, node) {
>> -             memcpy(&resources[count], rentry->res,
>> -                    sizeof(*resources));
>> -             count++;
>> -     }
>> -     ret = platform_device_add_resources(pdev, resources, count);
>> -
>> -out:
>> -     acpi_dev_free_resource_list(&resource_list);
>>
>>       return ret;
>>   }