Re: [PATCH 2/2] ACPI / osi: add DMI quirk for Dell systems

[Jean Delvare <jdelvare@suse.de>]

On Mon, 5 Feb 2018 17:24:43 +0000, Mario.Limonciello@dell.com wrote:
> > > +static int __init dmi_oem_osi_add(const struct dmi_system_id *d)
> > > +{
> > > +	struct acpi_osi_entry *osi;
> > > +	const char *str = d->driver_data;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
> > > +		osi = &osi_setup_entries[i];
> > > +		if (!strcmp(osi->string, str)) {  
> > 
> > This can only happen if the user passes acpi_osi=Linux-Dell-Video or
> > acpi_osi=!Linux-Dell-Video on the boot command line, right?
> >   
> > > +			osi->enable = true;  
> > 
> > Does this not prevent the user from explicitly disabling it with
> > acpi_osi=!Linux-Dell-Video ?
> >   
> > > +			continue;  
> > 
> > Are you not done at this point? I think you want to break, not
> > continue, else you may add a duplicate Linux-Dell-Video entry below.  
> 
> You might have two different entries that apply to the same system in the future.
> 
> At least the way that Dell is intending to use this is that "Linux-Dell-Video" would
> only apply to ASL related to video configuration.
> 
> If for example there is later an issue with audio on a different platform that also
> needed "Linux-Dell-Video", the ASL to do the audio configuration would be activated
> by "Linux-Dell-Audio".

I understand what you say, but can't see how this relates to my
concerns above.

-- 
Jean Delvare
SUSE L3 Support