Re: [PATCH 3/5] ACPI: button: Turn lid_blacklst dmi table into a generic quirk table

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

Hi,

On 21-10-2019 11:08, Andy Shevchenko wrote:
> On Fri, Oct 18, 2019 at 09:41:13PM +0200, Hans de Goede wrote:
>> Commit 3540c32a9ae4 ("ACPI / button: Add quirks for initial lid state
>> notification") added 3 different modes to the LID handling code to deal
>> with various buggy implementations.
>>
>> Until now users which need one of the 2 non-default modes to get their
>> hw to work have to pass a kernel commandline option for this.
>>
>> E.g. https://bugzilla.kernel.org/show_bug.cgi?id=106151 was closed with a
>> note that the user has to add "button.lid_init_state=open" to the kernel
>> commandline to get the LID code to not cause undesirable suspends on his
>> Samsung N210 Plus.
>>
>> This commit modifies the existing lid_blacklst dmi table so that it can
>> be used not only to completely disable the LID code on devices where
>> the ACPI tables are broken beyond repair, but also to select one of the 2
>> non default LID handling modes on devices where this is necessary.
>>
>> This will allow us to add quirks to make the LID work OOTB on broken
>> devices. Getting this working OOTB is esp. important because the typical
>> breakage is false LID closed reporting, causing undesirable suspends which
>> basically make the system unusable.
> 
>> +static int lid_init_state = -1;
> 
>>   static int acpi_button_register_driver(struct acpi_driver *driver)
>>   {
>> +	const struct dmi_system_id *dmi_id;
>> +
>> +	if (lid_init_state == -1) {
>> +		dmi_id = dmi_first_match(dmi_lid_quirks);
>> +		if (dmi_id)
> 
>> +			lid_init_state = (long)dmi_id->driver_data;
> 
> I would rather see here (intptr_t), though up to you and Rafael.
> Or mark a variable long itself?

I will make the variable itself long for v2.

>> +		else
>> +			lid_init_state = ACPI_BUTTON_LID_INIT_METHOD;
> 
> Can't we simple default the value to this?

No then we do not know if this value was set as module option by
the user, or if it is just the default and we should not use the
dmi based quirks when the value is set by the user, so we must
be able to distinguish the 2 cases.

Regards,

Hans