Re: [PATCH 02/10] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger <trenn@suse.de>]

On Friday 14 November 2008 04:27:42 pm Bjorn Helgaas wrote:
> On Friday 01 August 2008 09:37:55 am Thomas Renninger wrote:
> > @@ -1013,7 +983,7 @@ static void acpi_device_set_id(struct acpi_device
> > *device, will get autoloaded and the device might still match
> >  		   against another driver.
> >  		*/
> > -		if (ACPI_SUCCESS(acpi_video_bus_match(device)))
> > +		if (acpi_is_video_device(device))
> >  			cid_add = ACPI_VIDEO_HID;
> >  		else if (ACPI_SUCCESS(acpi_bay_match(device)))
> >  			cid_add = ACPI_BAY_HID;
>
> It doesn't seem right to me to make this core behavior depend
> on a config option.  With this approach, the ACPI device tree may
> or may not contain an ACPI_VIDEO_HID device, depending on whether
> CONFIG_ACPI_VIDEO is set, and that seems capricious.
>
> What is the benefit of moving this code out of scan.c?  It's not
> a very big function, and I think the consistency is worth the extra
> code.
This was done because the function is re-used later to detect which kind of
video functionalities the video device provides.
I somehow agree, but I don't see much of a disadvantage having this separated. 
You do not have the video device marked as such if CONFIG_ACPI_VIDEO is not 
compiled in, but it shouldn't hurt?

The two corresponding functions:
acpi_backlight_cap_match
acpi_is_video_device

might also go back to video.c again and get exported there. I can do that if 
you think it's worth it.

    Thomas
>
> > +#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
> > +
> > +extern long acpi_video_get_capabilities(acpi_handle
> > graphics_dev_handle); +extern long acpi_is_video_device(struct
> > acpi_device *device);
> > +extern int acpi_video_backlight_support(void);
> > +extern int acpi_video_display_switch_support(void);
> > +
> > +#else
> > +
> > +static inline long acpi_video_get_capabilities(acpi_handle
> > graphics_dev_handle) +{
> > +	return 0;
> > +}
> > +
> > +static inline long acpi_is_video_device(struct acpi_device *device)
> > +{
> > +	return 0;
> > +}