Hi Hans On 04/02/2021 04:18, Hans de Goede wrote: > Hi, > > On 2/3/21 3:46 PM, Mark Pearson wrote: >> On 02/02/2021 09:49, Hans de Goede wrote: >>> Hi, >>> >>> On 1/11/21 5:22 PM, Mark Pearson wrote: >>>> Add support to thinkpad_acpi for Lenovo platforms that have DYTC >>>> version 5 support or newer to use the platform profile feature. >>>> >>>> This will allow users to determine and control the platform modes >>>> between low-power, balanced operation and performance modes. >>>> >>>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> >>> >>> Now that the acpi/platform_profile changes have landed I have >>> merged this patch (solving a trivial conflict caused by the >>> keyboard_lang changes). >>> >>> Thank you for your patch, I've applied this patch to my review-hans >>> branch: >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans >>> >>> Note it will show up in my review-hans branch once I've pushed my >>> local branch there, which might take a while. >>> >>> Once I've run some tests on this branch the patches there will be >>> added to the platform-drivers-x86/for-next branch and eventually >>> will be included in the pdx86 pull-request to Linus for the next >>> merge-window. >>> >> Thanks Hans - sounds great. >> Let me know if you find any issues or need any extra tests running. > > So the build-bots have found 2 issues: > > 1. Some symbols which are not exported need to be marked static (I will fix this myself, > that is the easiest / fastest): > > drivers/platform/x86/thinkpad_acpi.c:10081:5: warning: no previous prototype for 'dytc_profile_get' [-Wmissing-prototypes] > drivers/platform/x86/thinkpad_acpi.c:10095:5: warning: no previous prototype for 'dytc_cql_command' [-Wmissing-prototypes] > drivers/platform/x86/thinkpad_acpi.c:10133:5: warning: no previous prototype for 'dytc_profile_set' [-Wmissing-prototypes] > > 2. This is a bigger problem, this is the result of a random-config test-build and I'm > pretty sure that the issue is that acpi_platform was build as a module while > thinkpad_acpi was builtin and builtin code cannot rely on modules. > > drivers/platform/x86/thinkpad_acpi.c:10186: undefined reference to `platform_profile_notify' > drivers/platform/x86/thinkpad_acpi.c:10226: undefined reference to `platform_profile_register' > drivers/platform/x86/thinkpad_acpi.c:10246: undefined reference to `platform_profile_remove' > > There are 2 ways to solve this: > > 1. Change > > #if IS_ENABLED(CONFIG_ACPI_PLATFORM_PROFILE) > > to: > > #if IS_REACHABLE(CONFIG_ACPI_PLATFORM_PROFILE) > > Which will disable the platform-profile support in acpi_thinkpad in the above scenario. > > or. > > 2. Drop the #if IS_ENABLED(...) and add a > > depends on ACPI_PLATFORM_PROFILE > > To the THINKPAD_ACPI Kconfig symbol. > > > I personally think 2. would be slightly better as it ensures that platform-profile > support is always available when thinkpad_acpi is build, hopefully leading to less > confusing bug-reports about it sometimes not working. > > If you can let me know what you want, then I can fix this locally too and get > the fix in place before the merge-window opens. > > Regards, > > Hans > I think you've fixed both of those already - and I agree with #2. Thanks for jumping on these. Let me know if I need to do anything to help. Thanks! Mark
