From: Dan Carpenter <dan.carpenter@oracle.com>
To: Peter Suti <peter.suti@streamunlimited.com>,
Chuansheng Liu <chuansheng.liu@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: fbtft: core: set smem_len before fb_deferred_io_init call
Date: Tue, 26 Jul 2022 19:13:48 +0300 [thread overview]
Message-ID: <20220726161347.GR2338@kadam> (raw)
In-Reply-To: <20220726082114.891853-1-peter.suti@streamunlimited.com>
Thanks for the patch.
On Tue, Jul 26, 2022 at 10:21:13AM +0200, Peter Suti wrote:
> fb_deferred_io_init depends on smem_len being filled
> to be able to initialize the virtual page lists since
> commit 856082f021a2 ("fbdev: defio: fix the pagelist corruption")
>
This code has changed since then so the patch needs to be updated.
The patch is still necessary but the bug will look different now
because there was a WARN_ON() added.
Currently the commit message does not say how this bug looks like to the
user. Also the use a Fixes tag. Something like this:
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:
Make sure you CC the original author (Chuansheng Liu) so they can review
the bug fix.
Google used to give good guides for how to send a v2 patch but now the
first page is just useless. :/
regards,
dan carpenter
> Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
> ---
> drivers/staging/fbtft/fbtft-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 9c4d797e7ae4..4137c1a51e1b 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -656,7 +656,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
> fbdefio->delay = HZ / fps;
> fbdefio->sort_pagelist = 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;
> @@ -667,6 +666,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;
> --
> 2.25.1
>
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Peter Suti <peter.suti@streamunlimited.com>,
Chuansheng Liu <chuansheng.liu@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-fbdev@vger.kernel.org, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] staging: fbtft: core: set smem_len before fb_deferred_io_init call
Date: Tue, 26 Jul 2022 19:13:48 +0300 [thread overview]
Message-ID: <20220726161347.GR2338@kadam> (raw)
In-Reply-To: <20220726082114.891853-1-peter.suti@streamunlimited.com>
Thanks for the patch.
On Tue, Jul 26, 2022 at 10:21:13AM +0200, Peter Suti wrote:
> fb_deferred_io_init depends on smem_len being filled
> to be able to initialize the virtual page lists since
> commit 856082f021a2 ("fbdev: defio: fix the pagelist corruption")
>
This code has changed since then so the patch needs to be updated.
The patch is still necessary but the bug will look different now
because there was a WARN_ON() added.
Currently the commit message does not say how this bug looks like to the
user. Also the use a Fixes tag. Something like this:
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:
Make sure you CC the original author (Chuansheng Liu) so they can review
the bug fix.
Google used to give good guides for how to send a v2 patch but now the
first page is just useless. :/
regards,
dan carpenter
> Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
> ---
> drivers/staging/fbtft/fbtft-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
> index 9c4d797e7ae4..4137c1a51e1b 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -656,7 +656,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
> fbdefio->delay = HZ / fps;
> fbdefio->sort_pagelist = 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;
> @@ -667,6 +666,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;
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-07-26 16:14 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-26 8:21 [PATCH] staging: fbtft: core: set smem_len before fb_deferred_io_init call Peter Suti
2022-07-26 8:21 ` Peter Suti
2022-07-26 16:13 ` Dan Carpenter [this message]
2022-07-26 16:13 ` Dan Carpenter
2022-07-27 7:07 ` [PATCH v2] " Peter Suti
2022-07-27 7:07 ` Peter Suti
2022-07-27 7:13 ` Greg Kroah-Hartman
2022-07-27 7:13 ` Greg Kroah-Hartman
2022-07-27 7:35 ` [PATCH v3] " Peter Suti
2022-07-27 7:35 ` Peter Suti
2022-07-28 12:49 ` Liu, Chuansheng
2022-07-28 12:49 ` Liu, Chuansheng
2022-07-28 13:39 ` Dan Carpenter
2022-07-28 13:39 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220726161347.GR2338@kadam \
--to=dan.carpenter@oracle.com \
--cc=chuansheng.liu@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=peter.suti@streamunlimited.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.