Re: [PATCH v2] platform/x86: toshiba_acpi: Fix quickstart quirk handling

[Hans de Goede <hdegoede@redhat.com>]

Hi Armin,

On 7/1/24 9:45 PM, Armin Wolf wrote:
> The global hci_hotkey_quickstart quirk flag is tested in
> toshiba_acpi_enable_hotkeys() before the quirk flag is properly
> initialized based on SMBIOS data. This causes the quirk to be
> applied to all models, some of which behave erratically as a
> result.
> 
> Fix this by initializing the global quirk flags during module
> initialization before registering the ACPI driver. This also
> allows us to mark toshiba_dmi_quirks[] as __initconst.
> 
> Fixes: 23f1d8b47d12 ("platform/x86: toshiba_acpi: Add quirk for buttons on Z830")
> Reported-by: kemal <kmal@cock.li>
> Tested-by: kemal <kmal@cock.li>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
> Changes since v1:
>  - add Tested-by tag

Thank you for fixing this, the patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

I was hoping my pdx86 fixes pull-request last Saturday would
be the last one for this cycle, but I'll prep another one
with this patch sometime this week:

Thank you for your patch/series, I've applied this patch
(series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

I have added a:

Closes: https://lore.kernel.org/platform-driver-x86/R4CYFS.TWB8QUU2SHWI1@cock.li/

tag whole applying this.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans



Regards,

Hans



> ---
>  drivers/platform/x86/toshiba_acpi.c | 31 +++++++++++++++++------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 3a8d8df89186..10d0ce6c8342 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -3271,7 +3271,7 @@ static const char *find_hci_method(acpi_handle handle)
>   */
>  #define QUIRK_HCI_HOTKEY_QUICKSTART		BIT(1)
> 
> -static const struct dmi_system_id toshiba_dmi_quirks[] = {
> +static const struct dmi_system_id toshiba_dmi_quirks[] __initconst = {
>  	{
>  	 /* Toshiba Portégé R700 */
>  	 /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
> @@ -3306,8 +3306,6 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  	struct toshiba_acpi_dev *dev;
>  	const char *hci_method;
>  	u32 dummy;
> -	const struct dmi_system_id *dmi_id;
> -	long quirks = 0;
>  	int ret = 0;
> 
>  	if (toshiba_acpi)
> @@ -3460,16 +3458,6 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>  	}
>  #endif
> 
> -	dmi_id = dmi_first_match(toshiba_dmi_quirks);
> -	if (dmi_id)
> -		quirks = (long)dmi_id->driver_data;
> -
> -	if (turn_on_panel_on_resume == -1)
> -		turn_on_panel_on_resume = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
> -
> -	if (hci_hotkey_quickstart == -1)
> -		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);
> -
>  	toshiba_wwan_available(dev);
>  	if (dev->wwan_supported)
>  		toshiba_acpi_setup_wwan_rfkill(dev);
> @@ -3618,10 +3606,27 @@ static struct acpi_driver toshiba_acpi_driver = {
>  	.drv.pm	= &toshiba_acpi_pm,
>  };
> 
> +static void __init toshiba_dmi_init(void)
> +{
> +	const struct dmi_system_id *dmi_id;
> +	long quirks = 0;
> +
> +	dmi_id = dmi_first_match(toshiba_dmi_quirks);
> +	if (dmi_id)
> +		quirks = (long)dmi_id->driver_data;
> +
> +	if (turn_on_panel_on_resume == -1)
> +		turn_on_panel_on_resume = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
> +
> +	if (hci_hotkey_quickstart == -1)
> +		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);
> +}
> +
>  static int __init toshiba_acpi_init(void)
>  {
>  	int ret;
> 
> +	toshiba_dmi_init();
>  	toshiba_proc_dir = proc_mkdir(PROC_TOSHIBA, acpi_root_dir);
>  	if (!toshiba_proc_dir) {
>  		pr_err("Unable to create proc dir " PROC_TOSHIBA "\n");
> --
> 2.39.2
>