| Message ID | 20220727073550.1491126-1-peter.suti@streamunlimited.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v3] staging: fbtft: core: set smem_len before fb_deferred_io_init call | expand |
Hi Peter, > -----Original Message----- > From: Peter Suti <peter.suti@streamunlimited.com> > Sent: Wednesday, July 27, 2022 3:36 PM > To: Liu, Chuansheng <chuansheng.liu@intel.com>; dan.carpenter@oracle.com; > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Thomas Zimmermann > <tzimmermann@suse.de>; Javier Martinez Canillas <javierm@redhat.com> > Cc: Peter Suti <peter.suti@streamunlimited.com>; dri- > devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux- > staging@lists.linux.dev; linux-kernel@vger.kernel.org > Subject: [PATCH v3] staging: fbtft: core: set smem_len before > fb_deferred_io_init call > > The fbtft_framebuffer_alloc() calls fb_deferred_io_init() before > initializing info->fix.smem_len. It is set to zero by the > framebuffer_alloc() function. It will trigger a WARN_ON() at the > start of fb_deferred_io_init() and the function will not do anything. Please show us the WARN_ON() call stack, then everything is clear. It is the driver itself issue to be fixed. I saw Thomas made such WARN_ON(). > > Fixes: 856082f021a2 ("fbdev: defio: fix the pagelist corruption") Don't agree with such description. Best Regards Chuansheng
On Thu, Jul 28, 2022 at 12:49:15PM +0000, Liu, Chuansheng wrote: > Hi Peter, > > > > > The fbtft_framebuffer_alloc() calls fb_deferred_io_init() before > > initializing info->fix.smem_len. It is set to zero by the > > framebuffer_alloc() function. It will trigger a WARN_ON() at the > > start of fb_deferred_io_init() and the function will not do anything. > > Please show us the WARN_ON() call stack, then everything is clear. > It is the driver itself issue to be fixed. I saw Thomas made such > WARN_ON(). I don't understand the confusion here. The code is very simple and the description seems very clear. No need to redo the patch. I think Peter tested it before the WARN_ON() was added so he probably didn't see the WARN_ON(). I told him to add that because it's pretty obvious what will happen just from reading the code. > > > > > Fixes: 856082f021a2 ("fbdev: defio: fix the pagelist corruption") > > Don't agree with such description. I don't see how you can disagree? Before your patch the fb_deferred_io_init() did not use info->fix.smem_len and now it does. regards, dan carpenter
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 9b3eaed80cdd..afaba94d1d1c 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -654,7 +654,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, fbdefio->delay = HZ / fps; fbdefio->sort_pagereflist = true; fbdefio->deferred_io = fbtft_deferred_io; - fb_deferred_io_init(info); snprintf(info->fix.id, sizeof(info->fix.id), "%s", dev->driver->name); info->fix.type = FB_TYPE_PACKED_PIXELS; @@ -665,6 +664,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display, info->fix.line_length = width * bpp / 8; info->fix.accel = FB_ACCEL_NONE; info->fix.smem_len = vmem_size; + fb_deferred_io_init(info); info->var.rotate = pdata->rotate; info->var.xres = width;
The fbtft_framebuffer_alloc() calls fb_deferred_io_init() before initializing info->fix.smem_len. It is set to zero by the framebuffer_alloc() function. It will trigger a WARN_ON() at the start of fb_deferred_io_init() and the function will not do anything. Fixes: 856082f021a2 ("fbdev: defio: fix the pagelist corruption") Signed-off-by: Peter Suti <peter.suti@streamunlimited.com> --- V2 -> V3: Add patch changelog V1 -> V2: Change commit message and base on top of linux-next drivers/staging/fbtft/fbtft-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)