Re: [PATCH] platform/x86: thinkpad_acpi: Fix registration of tpacpi platform driver

["Kurt Borja" <kuurtb@gmail.com>]

On Sat Feb 8, 2025 at 11:26 AM -05, Mark Pearson wrote:
> Thanks Kurt,
>
> On Fri, Feb 7, 2025, at 11:56 PM, Kurt Borja wrote:
>> Hi Mark,
>>
>> On Sat Feb 8, 2025 at 4:14 AM -05, Mark Pearson wrote:
>>> When reviewing and testing the recent platform profile changes I had
>>> missed that they prevent the tpacpi platform driver from registering.
>>> This error is seen in the kernel logs, and the various tpacpi entries
>>> are not created:
>>> [ 7550.642171] platform thinkpad_acpi: Resources present before probing
>>
>> This happens because in thinkpad_acpi_module_init(), ibm_init() is
>> called before platform_driver_register(&tpacpi_pdriver), therefore
>> devm_platform_profile_register() is called before tpacpi_pdev probes.
>>
>> As you can verify in [1], in the probing sequence, the driver core
>> verifies the devres list is empty, which in this case is not because of
>> the devm_ call.
>>
>>>
>>> I believe this is because the platform_profile driver registers the
>>> device as part of it's initialisation in devm_platform_profile_register,
>>> and the thinkpad_acpi driver later fails as the resource is already used.
>>>
>>> Modified thinkpad_acpi so that it has a separate platform driver for the
>>> profile handling, leaving the existing tpacpi_pdev to register
>>> successfully.
>>
>> While this works, it does not address the problem directly. Also it is
>> discouraged to create "fake" platform devices [2].
>>
>> May I suggest moving tpacpi_pdriver registration before ibm_init()
>> instead, so ibm_init_struct's .init callbacks can use devres?
>>
>
> Yep - you're right. Moving it before the init does also fix it.
>
> I can't see any issues with this approach, but I'll test it out a bit more carefully and do an updated version with this approach.

Thinking about it a bit more. With this approach you should maybe create
the tpacpi_pdev with platform_create_bundle() (I'm pretty sure you can
pass a NULL (*probe) callback) to avoid races.

platform_create_bundle() only returns after the device has been
successfully bound to the driver.

~ Kurt

>
> Thanks for the pointer.
>
> Mark