Re: [PATCH 1/4] HID: picolcd: fix deferred_io init/cleanup to (un)register_framebuffer ordering

[Jaya Kumar]

On Tue, Jun 29, 2010 at 4:29 AM, Bruno Prémont
<bonbons@xxxxxxxxxxxxxxxxx> wrote:
> We need to call fb_deferred_io_init() before we register_framebuffer()
> as otherwise, in case fbcon uses our framebuffer, we will get a BUG()
> because in picolcd_fb_imageblit() we schedule defio which has not
> been initialized yet.

Hi Bruno,

The comment above seems confusing to me in that it talks about fbcon
and defio schedule.

What I see is that you originally had in picolcd:

>        error = register_framebuffer(info);
...
>        fb_deferred_io_init(info);

which was a bug because it called register_framebuffer (ie: made the
framebuffer available for use) and only then initialized the defio
handlers which were needed for that framebuffer memory to be used.
Drivers must always call defio_init _before_ register_framebuffer. The
bug fix to picolcd below looks correct because it now does that
sequence in the correct order.

Thanks,
jaya



>
> Note: this BUG() deadlocks ttys.
>
> Signed-off-by: Bruno Prémont <bonbons@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/hid/hid-picolcd.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-picolcd.c b/drivers/hid/hid-picolcd.c
> index 95253b3..883d720 100644
> --- a/drivers/hid/hid-picolcd.c
> +++ b/drivers/hid/hid-picolcd.c
> @@ -707,18 +707,19 @@ static int picolcd_init_framebuffer(struct picolcd_data *data)
>                dev_err(dev, "failed to create sysfs attributes\n");
>                goto err_cleanup;
>        }
> +       fb_deferred_io_init(info);
>        data->fb_info    = info;
>        error = register_framebuffer(info);
>        if (error) {
>                dev_err(dev, "failed to register framebuffer\n");
>                goto err_sysfs;
>        }
> -       fb_deferred_io_init(info);
>        /* schedule first output of framebuffer */
>        schedule_delayed_work(&info->deferred_work, 0);
>        return 0;
>
>  err_sysfs:
> +       fb_deferred_io_cleanup(info);
>        device_remove_file(dev, &dev_attr_fb_update_rate);
>  err_cleanup:
>        data->fb_vbitmap = NULL;
> @@ -747,7 +748,6 @@ static void picolcd_exit_framebuffer(struct picolcd_data *data)
>        data->fb_bpp     = 0;
>        data->fb_info    = NULL;
>        device_remove_file(&data->hdev->dev, &dev_attr_fb_update_rate);
> -       fb_deferred_io_cleanup(info);
>        unregister_framebuffer(info);
>        vfree(fb_bitmap);
>        kfree(fb_vbitmap);
> --
> 1.7.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/