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

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

[Armin Wolf]

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
---
 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

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

[Hans de Goede]

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
> 

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

[Armin Wolf]

Am 02.07.24 um 12:52 schrieb Hans de Goede:

> 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

Thanks,

i will contact the stable team and ask them to revert the faulty commit from the stable kernels,
since the commit only makes sense together with the quickstart driver.

Thanks,
Armin Wolf

>
>
>> ---
>>   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
>>