Re: [PATCH] platform/x86: thinkpad_acpi: Fix incorrect use of platform profile on AMD platforms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mark,

On 1/27/22 20:03, 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>
> ---
> Changes in v2:
>  - Rebased on review-hans branch to be in sync with latest
>  - Return -ENODEV if funtion not available
> 
>  drivers/platform/x86/thinkpad_acpi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 33f611af6e51..5e4de74586cd 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10119,6 +10119,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 */
>  
> @@ -10331,6 +10334,15 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  	if (dytc_version < 5)
>  		return -ENODEV;
>  
> +	/* 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)) {

Erm test_bit operates on longs, so it may read an entire long from
&output which is 64 bits on x86_64, thus exceeding the space allocated
on the stack by 32 bits.

To avoid this I've replaced the if with:

	if (!(output & BIT(DYTC_FC_MMC))) {

And verified on a x1c8 that the performance-profiles still work there
after this change.

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

I will include this patch in the upcoming pdx86 fixes for 5.17
pull-req which I will send out soon.

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.

Regards,

Hans


> +		dbg_printk(TPACPI_DBG_INIT, " DYTC MMC mode not supported\n");
> +		return -ENODEV;
> +	}
> +
>  	dbg_printk(TPACPI_DBG_INIT,
>  			"DYTC version %d: thermal mode available\n", dytc_version);
>  	/*
> 




[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux