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; > > +} -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html