Hi Dmitry,
Thanks for your review and suggestions first!
於 一，2013-04-22 於 21:12 -0700，Dmitry Torokhov 提到：
> On Mon, Apr 22, 2013 at 08:39:15PM +0800, Chun-Yi Lee wrote:
> > From: "Lee, Chun-Yi" <jlee@xxxxxxxx>
> > +static acpi_status
> > +find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context,
> > + void **rv)
> > +{
> > + struct acpi_device *acpi_dev;
> > + struct acpi_video_bus *video = NULL;
>
> Gratuitous initialization of local variables prevents compiler from
> warning when you using variable uninitialized.
Yes, I agreed should not put gratuitous initialization, I will remove it
in v2 patch.
>
> > + struct acpi_video_device *dev, *next;
> > +
> > + if (acpi_bus_get_device(handle, &acpi_dev))
> > + return AE_OK;
> > +
> > + if (!acpi_match_device_ids(acpi_dev, video_device_ids)) {
> > + video = acpi_driver_data(acpi_dev);
> > + acpi_video_bus_stop_devices(video);
> > + mutex_lock(&video->device_list_lock);
> > + list_for_each_entry_safe(dev, next, &video->video_device_list,
> > + entry) {
> > + if (dev->backlight) {
> > + backlight_device_unregister(dev->backlight);
> > + dev->backlight = NULL;
> > + kfree(dev->brightness->levels);
> > + kfree(dev->brightness);
> > + }
> > + }
> > + mutex_unlock(&video->device_list_lock);
> > + acpi_video_bus_start_devices(video);
> > + }
> > + return AE_OK;
> > +}
> > +
> > +void acpi_video_backlight_unregister(void)
> > +{
> > + if (!register_count) {
>
> Locking? It looks like the rest of the driver ignores locking too...
Did you mean locking video_device_list?
The acpi/video locks video_device_list when add, remove and notify acpi
video bus driver. It always do the mutex lock before control
video_device_list, so I also add lock when unregister all backlight of
devices.
Thanks a lot!
Joey Lee
--
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
