Hi, On 1/26/22 17:49, Mark Pearson wrote: > > On 1/24/22 05:42, Hans de Goede wrote: >> Hi Mark, >> >> On 1/21/22 21:17, Mark Pearson wrote: >>> Lenovo AMD based platforms have been offering platform_profiles but they >>> are not working correctly. This is because the mode we are using on the >>> Intel platforms (MMC) is not available on the AMD platforms. >>> >>> This commit adds checking of the functional capabilities returned by the >>> BIOS to confirm if MMC is supported or not. Profiles will not be >>> available if the platform is not MMC capable. >>> >>> I'm investigating and working on an alternative for AMD platforms but >>> that is still work-in-progress. >>> >>> Signed-off-by: Mark Pearson <markpearson@xxxxxxxxxx> >>> --- >>> drivers/platform/x86/thinkpad_acpi.c | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c >>> index 9c632df734bb..42a04e44135b 100644 >>> --- a/drivers/platform/x86/thinkpad_acpi.c >>> +++ b/drivers/platform/x86/thinkpad_acpi.c >>> @@ -10026,6 +10026,9 @@ static struct ibm_struct proxsensor_driver_data = { >>> #define DYTC_CMD_MMC_GET 8 /* To get current MMC function and mode */ >>> #define DYTC_CMD_RESET 0x1ff /* To reset back to default */ >>> +#define DYTC_CMD_FUNC_CAP 3 /* To get DYTC capabilities */ >>> +#define DYTC_FC_MMC 27 /* MMC Mode supported */ >>> + >>> #define DYTC_GET_FUNCTION_BIT 8 /* Bits 8-11 - function setting */ >>> #define DYTC_GET_MODE_BIT 12 /* Bits 12-15 - mode setting */ >>> @@ -10253,6 +10256,16 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm) >>> if (dytc_version >= 5) { >>> dbg_printk(TPACPI_DBG_INIT, >>> "DYTC version %d: thermal mode available\n", dytc_version); >> This code has been refactored and this will not apply as is: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/thinkpad_acpi.c?id=0b0d2fba4f3302b601c429c9286e66b3af2d29cb >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/thinkpad_acpi.c?id=798682e236893a20e5674de02ede474373dd342d >> >> Please rebase on top of 5.17-rc1 > My apologies - I thought I was on the latest. Will rebase No problem. >> >>> + >>> + /* Check what capabilities are supported. Currently MMC is needed */ >>> + err = dytc_command(DYTC_CMD_FUNC_CAP, &output); >>> + if (err) >>> + return err; >>> + if (!test_bit(DYTC_FC_MMC, (void *)&output)) { >>> + dbg_printk(TPACPI_DBG_INIT, " DYTC MMC mode not supported\n"); >>> + return 0; >> This should be return -ENODEV; > > I'll double check, but I don't think we want an error here as we want to continue on, it's just the feature is not supported so 0 is OK? -ENODEV is treated as "feature not supported" and will not cause any errors be printed and probing will continue normally with an -ENODEV error. Where as 0 will still cause the corresponding exit function to get called on module unload, causing platform_profile_remove() to get called even though we never registered the platform_profile handler. So -ENODEV is better. Regards, Hans