From 91c05c667b2d8e43e0bbc5f269bf45d4821001d6 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 5 Nov 2007 11:43:29 -0500 Subject: ACPI: video - fit input device into sysfs tree Properly set up parent on input device registered by the video driver. Signed-off-by: Dmitry Torokhov Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index bac956b30c5..c0e4e7423fc 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1961,6 +1961,7 @@ static int acpi_video_bus_add(struct acpi_device *device) input->phys = video->phys; input->id.bustype = BUS_HOST; input->id.product = 0x06; + input->dev.parent = &device->dev; input->evbit[0] = BIT(EV_KEY); set_bit(KEY_SWITCHVIDEOMODE, input->keybit); set_bit(KEY_VIDEO_NEXT, input->keybit); @@ -1990,7 +1991,7 @@ static int acpi_video_bus_add(struct acpi_device *device) video->flags.rom ? "yes" : "no", video->flags.post ? "yes" : "no"); - end: + end: if (result) kfree(video); -- cgit v1.2.3 From f51e83916a0a022d3d0ea39ae2f877c703032923 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 5 Nov 2007 11:43:30 -0500 Subject: ACPI: video - add missing input_free_device() If input_register_device() fails input_free_device() must be called to release memory allocated for the device. Also consolidate error handling in acpi_bus_video_add() and handle input_allocate_device() failures. Signed-off-by: Dmitry Torokhov Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 69 ++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 35 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index c0e4e7423fc..be66a7c04d3 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1897,14 +1897,10 @@ static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data) static int instance; static int acpi_video_bus_add(struct acpi_device *device) { - int result = 0; - acpi_status status = 0; - struct acpi_video_bus *video = NULL; + acpi_status status; + struct acpi_video_bus *video; struct input_dev *input; - - - if (!device) - return -EINVAL; + int error; video = kzalloc(sizeof(struct acpi_video_bus), GFP_KERNEL); if (!video) @@ -1923,13 +1919,13 @@ static int acpi_video_bus_add(struct acpi_device *device) acpi_driver_data(device) = video; acpi_video_bus_find_cap(video); - result = acpi_video_bus_check(video); - if (result) - goto end; + error = acpi_video_bus_check(video); + if (error) + goto err_free_video; - result = acpi_video_bus_add_fs(device); - if (result) - goto end; + error = acpi_video_bus_add_fs(device); + if (error) + goto err_free_video; init_MUTEX(&video->sem); INIT_LIST_HEAD(&video->video_device_list); @@ -1943,16 +1939,15 @@ static int acpi_video_bus_add(struct acpi_device *device) if (ACPI_FAILURE(status)) { ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Error installing notify handler\n")); - acpi_video_bus_stop_devices(video); - acpi_video_bus_put_devices(video); - kfree(video->attached_array); - acpi_video_bus_remove_fs(device); - result = -ENODEV; - goto end; + error = -ENODEV; + goto err_stop_video; } - video->input = input = input_allocate_device(); + if (!input) { + error = -ENOMEM; + goto err_uninstall_notify; + } snprintf(video->phys, sizeof(video->phys), "%s/video/input0", acpi_device_hid(video->device)); @@ -1972,18 +1967,10 @@ static int acpi_video_bus_add(struct acpi_device *device) set_bit(KEY_BRIGHTNESS_ZERO, input->keybit); set_bit(KEY_DISPLAY_OFF, input->keybit); set_bit(KEY_UNKNOWN, input->keybit); - result = input_register_device(input); - if (result) { - acpi_remove_notify_handler(video->device->handle, - ACPI_DEVICE_NOTIFY, - acpi_video_bus_notify); - acpi_video_bus_stop_devices(video); - acpi_video_bus_put_devices(video); - kfree(video->attached_array); - acpi_video_bus_remove_fs(device); - goto end; - } + error = input_register_device(input); + if (error) + goto err_free_input_dev; printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s rom: %s post: %s)\n", ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device), @@ -1991,11 +1978,23 @@ static int acpi_video_bus_add(struct acpi_device *device) video->flags.rom ? "yes" : "no", video->flags.post ? "yes" : "no"); - end: - if (result) - kfree(video); + return 0; + + err_free_input_dev: + input_free_device(input); + err_uninstall_notify: + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_video_bus_notify); + err_stop_video: + acpi_video_bus_stop_devices(video); + acpi_video_bus_put_devices(video); + kfree(video->attached_array); + acpi_video_bus_remove_fs(device); + err_free_video: + kfree(video); + acpi_driver_data(device) = NULL; - return result; + return error; } static int acpi_video_bus_remove(struct acpi_device *device, int type) -- cgit v1.2.3 From ff102ea99099c36250e93a87a9794b5233801020 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 5 Nov 2007 11:43:31 -0500 Subject: ACPI: video - remove unsafe uses of list_for_each_safe() list_for_each_safe() only protects list from list alterations performed by the same thread. One still needs to implement proper locking when list is being accessed from several threads. Signed-off-by: Dmitry Torokhov Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 71 +++++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 37 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index be66a7c04d3..36b64a75167 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1462,12 +1462,14 @@ acpi_video_bus_get_one_device(struct acpi_device *device, static void acpi_video_device_rebind(struct acpi_video_bus *video) { - struct list_head *node, *next; - list_for_each_safe(node, next, &video->video_device_list) { - struct acpi_video_device *dev = - container_of(node, struct acpi_video_device, entry); + struct acpi_video_device *dev; + + down(&video->sem); + + list_for_each_entry(dev, &video->video_device_list, entry) acpi_video_device_bind(video, dev); - } + + up(&video->sem); } /* @@ -1592,30 +1594,33 @@ static int acpi_video_device_enumerate(struct acpi_video_bus *video) static int acpi_video_switch_output(struct acpi_video_bus *video, int event) { - struct list_head *node, *next; + struct list_head *node; struct acpi_video_device *dev = NULL; struct acpi_video_device *dev_next = NULL; struct acpi_video_device *dev_prev = NULL; unsigned long state; int status = 0; + down(&video->sem); - list_for_each_safe(node, next, &video->video_device_list) { + list_for_each(node, &video->video_device_list) { dev = container_of(node, struct acpi_video_device, entry); status = acpi_video_device_get_state(dev, &state); if (state & 0x2) { - dev_next = - container_of(node->next, struct acpi_video_device, - entry); - dev_prev = - container_of(node->prev, struct acpi_video_device, - entry); + dev_next = container_of(node->next, + struct acpi_video_device, entry); + dev_prev = container_of(node->prev, + struct acpi_video_device, entry); goto out; } } + dev_next = container_of(node->next, struct acpi_video_device, entry); dev_prev = container_of(node->prev, struct acpi_video_device, entry); - out: + + out: + up(&video->sem); + switch (event) { case ACPI_VIDEO_NOTIFY_CYCLE: case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT: @@ -1691,24 +1696,17 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video, struct acpi_device *device) { int status = 0; - struct list_head *node, *next; - + struct acpi_device *dev; acpi_video_device_enumerate(video); - list_for_each_safe(node, next, &device->children) { - struct acpi_device *dev = - list_entry(node, struct acpi_device, node); - - if (!dev) - continue; + list_for_each_entry(dev, &device->children, node) { status = acpi_video_bus_get_one_device(dev, video); if (ACPI_FAILURE(status)) { ACPI_EXCEPTION((AE_INFO, status, "Cant attach device")); continue; } - } return status; } @@ -1724,9 +1722,6 @@ static int acpi_video_bus_put_one_device(struct acpi_video_device *device) video = device->video; - down(&video->sem); - list_del(&device->entry); - up(&video->sem); acpi_video_device_remove_fs(device->dev); status = acpi_remove_notify_handler(device->dev->handle, @@ -1734,32 +1729,34 @@ static int acpi_video_bus_put_one_device(struct acpi_video_device *device) acpi_video_device_notify); backlight_device_unregister(device->backlight); video_output_unregister(device->output_dev); + return 0; } static int acpi_video_bus_put_devices(struct acpi_video_bus *video) { int status; - struct list_head *node, *next; + struct acpi_video_device *dev, *next; + down(&video->sem); - list_for_each_safe(node, next, &video->video_device_list) { - struct acpi_video_device *data = - list_entry(node, struct acpi_video_device, entry); - if (!data) - continue; + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) { - status = acpi_video_bus_put_one_device(data); + status = acpi_video_bus_put_one_device(dev); if (ACPI_FAILURE(status)) printk(KERN_WARNING PREFIX "hhuuhhuu bug in acpi video driver.\n"); - if (data->brightness) - kfree(data->brightness->levels); - kfree(data->brightness); - kfree(data); + if (dev->brightness) { + kfree(dev->brightness->levels); + kfree(dev->brightness); + } + list_del(&dev->entry); + kfree(dev); } + up(&video->sem); + return 0; } -- cgit v1.2.3 From bbac81f5487175e4bd5602a80c17689d8f82a63e Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 5 Nov 2007 11:43:32 -0500 Subject: ACPI: video - convert semaphore to a mutex Signed-off-by: Dmitry Torokhov Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 36b64a75167..d54c83d7053 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -135,8 +136,8 @@ struct acpi_video_bus { u8 attached_count; struct acpi_video_bus_cap cap; struct acpi_video_bus_flags flags; - struct semaphore sem; struct list_head video_device_list; + struct mutex device_list_lock; /* protects video_device_list */ struct proc_dir_entry *dir; struct input_dev *input; char phys[32]; /* for input device */ @@ -1436,9 +1437,9 @@ acpi_video_bus_get_one_device(struct acpi_device *device, return -ENODEV; } - down(&video->sem); + mutex_lock(&video->device_list_lock); list_add_tail(&data->entry, &video->video_device_list); - up(&video->sem); + mutex_unlock(&video->device_list_lock); acpi_video_device_add_fs(device); @@ -1464,12 +1465,12 @@ static void acpi_video_device_rebind(struct acpi_video_bus *video) { struct acpi_video_device *dev; - down(&video->sem); + mutex_lock(&video->device_list_lock); list_for_each_entry(dev, &video->video_device_list, entry) acpi_video_device_bind(video, dev); - up(&video->sem); + mutex_unlock(&video->device_list_lock); } /* @@ -1601,7 +1602,7 @@ static int acpi_video_switch_output(struct acpi_video_bus *video, int event) unsigned long state; int status = 0; - down(&video->sem); + mutex_lock(&video->device_list_lock); list_for_each(node, &video->video_device_list) { dev = container_of(node, struct acpi_video_device, entry); @@ -1619,7 +1620,7 @@ static int acpi_video_switch_output(struct acpi_video_bus *video, int event) dev_prev = container_of(node->prev, struct acpi_video_device, entry); out: - up(&video->sem); + mutex_unlock(&video->device_list_lock); switch (event) { case ACPI_VIDEO_NOTIFY_CYCLE: @@ -1738,7 +1739,7 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video) int status; struct acpi_video_device *dev, *next; - down(&video->sem); + mutex_lock(&video->device_list_lock); list_for_each_entry_safe(dev, next, &video->video_device_list, entry) { @@ -1755,7 +1756,7 @@ static int acpi_video_bus_put_devices(struct acpi_video_bus *video) kfree(dev); } - up(&video->sem); + mutex_unlock(&video->device_list_lock); return 0; } @@ -1924,7 +1925,7 @@ static int acpi_video_bus_add(struct acpi_device *device) if (error) goto err_free_video; - init_MUTEX(&video->sem); + mutex_init(&video->device_list_lock); INIT_LIST_HEAD(&video->video_device_list); acpi_video_bus_get_devices(video, device); -- cgit v1.2.3 From a4f0c2767e9c55123d7dad7176554e9d6e6056bc Mon Sep 17 00:00:00 2001 From: Len Brown Date: Wed, 14 Nov 2007 12:49:13 -0500 Subject: ACPI: video - delete stray run-time printk printk("video bus notify\n"); Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index d54c83d7053..dce0a6e47f5 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1780,9 +1780,6 @@ static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data) struct input_dev *input; int keycode; - - printk("video bus notify\n"); - if (!video) return; -- cgit v1.2.3 From c88c5786d3df51ccfa4e2d111fc9c8fc0f5b2797 Mon Sep 17 00:00:00 2001 From: Danny Baumann Date: Fri, 2 Nov 2007 13:47:53 +0100 Subject: ACPI: Video: Increase buffer size for writes to brightness proc file. In order to be able to write the value "100" to /proc/acpi/video/.../brightness, we have to allocate 5 bytes: 4 characters will be written (1, 0, 0 plus null byte), and 1 byte should be buffer for a terminating NULL character. http://bugzilla.kernel.org/show_bug.cgi?id=9278 Signed-off-by: Danny Baumann Acked-by: Zhang Rui Signed-off-by: Len Brown --- drivers/acpi/video.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index dce0a6e47f5..44a0d9ba9bd 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -897,7 +897,7 @@ acpi_video_device_write_brightness(struct file *file, { struct seq_file *m = file->private_data; struct acpi_video_device *dev = m->private; - char str[4] = { 0 }; + char str[5] = { 0 }; unsigned int level = 0; int i; -- cgit v1.2.3