| Message ID | 20250211173620.16522-1-mpearson-lenovo@squebb.ca (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v3] platform/x86: thinkpad_acpi: Fix registration of tpacpi platform driver | expand |
On Tue Feb 11, 2025 at 12:36 PM -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 devm_platform_profile_register() is called before > tpacpi_pdev probes (thanks to Kurt Borja for identifying root cause) > > For now revert back to the old platform_profile_register to fix the > issue. Will work on re-implementing this later as more testing is needed > for full solution. > > Tested on X1 Carbon G12. > > Fixes: 31658c916fa6 ("platform/x86: thinkpad_acpi: Use devm_platform_profile_register()") > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> I believe this is done now! Reviewed-by: Kurt Borja <kuurtb@gmail.com> > --- > Changes in v2: > Modified approach to instead revert to old platform_profile_register > method. Will revisit using devm_ version in the future as more testing > needed. > Changes in v3: > Add check if tpacpi_pprof is valid before releasing. > > drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 1fcb0f99695a..9f6d7e26e700 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -10646,8 +10646,8 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) > "DYTC version %d: thermal mode available\n", dytc_version); > > /* Create platform_profile structure and register */ > - tpacpi_pprof = devm_platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi", > - NULL, &dytc_profile_ops); > + tpacpi_pprof = platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi-profile", > + NULL, &dytc_profile_ops); > /* > * If for some reason platform_profiles aren't enabled > * don't quit terminally. > @@ -10665,8 +10665,15 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) > return 0; > } > > +static void dytc_profile_exit(void) > +{ > + if (!IS_ERR_OR_NULL(tpacpi_pprof)) > + platform_profile_remove(tpacpi_pprof); > +} > + > static struct ibm_struct dytc_profile_driver_data = { > .name = "dytc-profile", > + .exit = dytc_profile_exit, > }; > > /*************************************************************************
On Tue, 11 Feb 2025, Kurt Borja wrote: > On Tue Feb 11, 2025 at 12:36 PM -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 devm_platform_profile_register() is called before > > tpacpi_pdev probes (thanks to Kurt Borja for identifying root cause) > > > > For now revert back to the old platform_profile_register to fix the > > issue. Will work on re-implementing this later as more testing is needed > > for full solution. Hi Mark, I've applied this to the review-ilpo-branch. I had to rewrite parts of your changelog though to not say "I did/will do this and that". In future, please just state the simple facts. What the issue is and so on, do not write a story about how you came to find that out. :-)
On Wed, Feb 12, 2025, at 6:50 AM, Ilpo Järvinen wrote: > On Tue, 11 Feb 2025, Kurt Borja wrote: > >> On Tue Feb 11, 2025 at 12:36 PM -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 devm_platform_profile_register() is called before >> > tpacpi_pdev probes (thanks to Kurt Borja for identifying root cause) >> > >> > For now revert back to the old platform_profile_register to fix the >> > issue. Will work on re-implementing this later as more testing is needed >> > for full solution. > > Hi Mark, > > I've applied this to the review-ilpo-branch. I had to rewrite parts of > your changelog though to not say "I did/will do this and that". In future, > please just state the simple facts. What the issue is and so on, do not > write a story about how you came to find that out. :-) > Ah - apologies, and thank you. Mark > -- > i. > >> > Tested on X1 Carbon G12. >> > >> > Fixes: 31658c916fa6 ("platform/x86: thinkpad_acpi: Use devm_platform_profile_register()") >> > >> > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> >> I believe this is done now! >> >> Reviewed-by: Kurt Borja <kuurtb@gmail.com> >> >> > --- >> > Changes in v2: >> > Modified approach to instead revert to old platform_profile_register >> > method. Will revisit using devm_ version in the future as more testing >> > needed. >> > Changes in v3: >> > Add check if tpacpi_pprof is valid before releasing. >> > >> > drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++-- >> > 1 file changed, 9 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c >> > index 1fcb0f99695a..9f6d7e26e700 100644 >> > --- a/drivers/platform/x86/thinkpad_acpi.c >> > +++ b/drivers/platform/x86/thinkpad_acpi.c >> > @@ -10646,8 +10646,8 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) >> > "DYTC version %d: thermal mode available\n", dytc_version); >> > >> > /* Create platform_profile structure and register */ >> > - tpacpi_pprof = devm_platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi", >> > - NULL, &dytc_profile_ops); >> > + tpacpi_pprof = platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi-profile", >> > + NULL, &dytc_profile_ops); >> > /* >> > * If for some reason platform_profiles aren't enabled >> > * don't quit terminally. >> > @@ -10665,8 +10665,15 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) >> > return 0; >> > } >> > >> > +static void dytc_profile_exit(void) >> > +{ >> > + if (!IS_ERR_OR_NULL(tpacpi_pprof)) >> > + platform_profile_remove(tpacpi_pprof); >> > +} >> > + >> > static struct ibm_struct dytc_profile_driver_data = { >> > .name = "dytc-profile", >> > + .exit = dytc_profile_exit, >> > }; >> > >> > /************************************************************************* >>
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 1fcb0f99695a..9f6d7e26e700 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -10646,8 +10646,8 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) "DYTC version %d: thermal mode available\n", dytc_version); /* Create platform_profile structure and register */ - tpacpi_pprof = devm_platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi", - NULL, &dytc_profile_ops); + tpacpi_pprof = platform_profile_register(&tpacpi_pdev->dev, "thinkpad-acpi-profile", + NULL, &dytc_profile_ops); /* * If for some reason platform_profiles aren't enabled * don't quit terminally. @@ -10665,8 +10665,15 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) return 0; } +static void dytc_profile_exit(void) +{ + if (!IS_ERR_OR_NULL(tpacpi_pprof)) + platform_profile_remove(tpacpi_pprof); +} + static struct ibm_struct dytc_profile_driver_data = { .name = "dytc-profile", + .exit = dytc_profile_exit, }; /*************************************************************************
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 devm_platform_profile_register() is called before tpacpi_pdev probes (thanks to Kurt Borja for identifying root cause) For now revert back to the old platform_profile_register to fix the issue. Will work on re-implementing this later as more testing is needed for full solution. Tested on X1 Carbon G12. Fixes: 31658c916fa6 ("platform/x86: thinkpad_acpi: Use devm_platform_profile_register()") Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> --- Changes in v2: Modified approach to instead revert to old platform_profile_register method. Will revisit using devm_ version in the future as more testing needed. Changes in v3: Add check if tpacpi_pprof is valid before releasing. drivers/platform/x86/thinkpad_acpi.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)