| Message ID | 20230425142846.730-5-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm,fbdev: Use fbdev's I/O helpers | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Push the test for info->screen_base from fb_read() and fb_write() into > the implementations of struct fb_ops.{fb_read,fb_write}. In cases where > the driver operates on info->screen_buffer, test this field instead. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn> On 2023/4/25 22:28, Thomas Zimmermann wrote: > Push the test for info->screen_base from fb_read() and fb_write() into > the implementations of struct fb_ops.{fb_read,fb_write}. In cases where > the driver operates on info->screen_buffer, test this field instead. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > --- > drivers/video/fbdev/cobalt_lcdfb.c | 6 ++++++ > drivers/video/fbdev/core/fb_sys_fops.c | 6 ++++++ > drivers/video/fbdev/core/fbmem.c | 10 ++++++++-- > drivers/video/fbdev/sm712fb.c | 4 ++-- > 4 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/cobalt_lcdfb.c b/drivers/video/fbdev/cobalt_lcdfb.c > index 5f8b6324d2e8..26dbd1c78195 100644 > --- a/drivers/video/fbdev/cobalt_lcdfb.c > +++ b/drivers/video/fbdev/cobalt_lcdfb.c > @@ -129,6 +129,9 @@ static ssize_t cobalt_lcdfb_read(struct fb_info *info, char __user *buf, > unsigned long pos; > int len, retval = 0; > > + if (!info->screen_base) > + return -ENODEV; > + > pos = *ppos; > if (pos >= LCD_CHARS_MAX || count == 0) > return 0; > @@ -175,6 +178,9 @@ static ssize_t cobalt_lcdfb_write(struct fb_info *info, const char __user *buf, > unsigned long pos; > int len, retval = 0; > > + if (!info->screen_base) > + return -ENODEV; > + > pos = *ppos; > if (pos >= LCD_CHARS_MAX || count == 0) > return 0; > diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c > index 7dee5d3c7fb1..0cb0989abda6 100644 > --- a/drivers/video/fbdev/core/fb_sys_fops.c > +++ b/drivers/video/fbdev/core/fb_sys_fops.c > @@ -22,6 +22,9 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count, > unsigned long total_size, c; > ssize_t ret; > > + if (!info->screen_buffer) > + return -ENODEV; > + > total_size = info->screen_size; > > if (total_size == 0) > @@ -61,6 +64,9 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf, > unsigned long total_size, c; > size_t ret; > > + if (!info->screen_buffer) > + return -ENODEV; > + > total_size = info->screen_size; > > if (total_size == 0) > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index abc92d79a295..b993bb97058f 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -768,7 +768,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) > int c, cnt = 0, err = 0; > unsigned long total_size; > > - if (!info || ! info->screen_base) > + if (!info) > return -ENODEV; > > if (info->state != FBINFO_STATE_RUNNING) > @@ -777,6 +777,9 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) > if (info->fbops->fb_read) > return info->fbops->fb_read(info, buf, count, ppos); > > + if (!info->screen_base) > + return -ENODEV; > + > total_size = info->screen_size; > > if (total_size == 0) > @@ -833,7 +836,7 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) > int c, cnt = 0, err = 0; > unsigned long total_size; > > - if (!info || !info->screen_base) > + if (!info) > return -ENODEV; > > if (info->state != FBINFO_STATE_RUNNING) > @@ -842,6 +845,9 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) > if (info->fbops->fb_write) > return info->fbops->fb_write(info, buf, count, ppos); > > + if (!info->screen_base) > + return -ENODEV; > + > total_size = info->screen_size; > > if (total_size == 0) > diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c > index 6f852cd756c5..b7ad3c644e13 100644 > --- a/drivers/video/fbdev/sm712fb.c > +++ b/drivers/video/fbdev/sm712fb.c > @@ -1028,7 +1028,7 @@ static ssize_t smtcfb_read(struct fb_info *info, char __user *buf, > int c, i, cnt = 0, err = 0; > unsigned long total_size; > > - if (!info || !info->screen_base) > + if (!info->screen_base) > return -ENODEV; > > total_size = info->screen_size; > @@ -1091,7 +1091,7 @@ static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf, > int c, i, cnt = 0, err = 0; > unsigned long total_size; > > - if (!info || !info->screen_base) > + if (!info->screen_base) > return -ENODEV; > > total_size = info->screen_size;
Hi Thomas, On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: > Push the test for info->screen_base from fb_read() and fb_write() into > the implementations of struct fb_ops.{fb_read,fb_write}. In cases where > the driver operates on info->screen_buffer, test this field instead. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> Thanks for your patch! You forgot to mention why it is a good idea to duplicate this in all the implementations, instead of doing it once in the core? > drivers/video/fbdev/cobalt_lcdfb.c | 6 ++++++ > drivers/video/fbdev/core/fb_sys_fops.c | 6 ++++++ > drivers/video/fbdev/core/fbmem.c | 10 ++++++++-- > drivers/video/fbdev/sm712fb.c | 4 ++-- > 4 files changed, 22 insertions(+), 4 deletions(-) Aren't there more fbdev drivers to fix, before you can move the checks in drivers/video/fbdev/core/fbmem.c? Gr{oetje,eeting}s, Geert
Hi Am 26.04.23 um 16:56 schrieb Geert Uytterhoeven: > Hi Thomas, > > > On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> Push the test for info->screen_base from fb_read() and fb_write() into >> the implementations of struct fb_ops.{fb_read,fb_write}. In cases where >> the driver operates on info->screen_buffer, test this field instead. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > Thanks for your patch! > You forgot to mention why it is a good idea to duplicate this in all > the implementations, instead of doing it once in the core? > >> drivers/video/fbdev/cobalt_lcdfb.c | 6 ++++++ >> drivers/video/fbdev/core/fb_sys_fops.c | 6 ++++++ >> drivers/video/fbdev/core/fbmem.c | 10 ++++++++-- >> drivers/video/fbdev/sm712fb.c | 4 ++-- >> 4 files changed, 22 insertions(+), 4 deletions(-) > > Aren't there more fbdev drivers to fix, before you can move the checks > in drivers/video/fbdev/core/fbmem.c? I've found a few. And I've also found quite a number of drivers that use screen_base when they should use screen_buffer instead. I'll fix them as well. Best regards Thomas > > Gr{oetje,eeting}s, > > Geert >
diff --git a/drivers/video/fbdev/cobalt_lcdfb.c b/drivers/video/fbdev/cobalt_lcdfb.c index 5f8b6324d2e8..26dbd1c78195 100644 --- a/drivers/video/fbdev/cobalt_lcdfb.c +++ b/drivers/video/fbdev/cobalt_lcdfb.c @@ -129,6 +129,9 @@ static ssize_t cobalt_lcdfb_read(struct fb_info *info, char __user *buf, unsigned long pos; int len, retval = 0; + if (!info->screen_base) + return -ENODEV; + pos = *ppos; if (pos >= LCD_CHARS_MAX || count == 0) return 0; @@ -175,6 +178,9 @@ static ssize_t cobalt_lcdfb_write(struct fb_info *info, const char __user *buf, unsigned long pos; int len, retval = 0; + if (!info->screen_base) + return -ENODEV; + pos = *ppos; if (pos >= LCD_CHARS_MAX || count == 0) return 0; diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c index 7dee5d3c7fb1..0cb0989abda6 100644 --- a/drivers/video/fbdev/core/fb_sys_fops.c +++ b/drivers/video/fbdev/core/fb_sys_fops.c @@ -22,6 +22,9 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count, unsigned long total_size, c; ssize_t ret; + if (!info->screen_buffer) + return -ENODEV; + total_size = info->screen_size; if (total_size == 0) @@ -61,6 +64,9 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf, unsigned long total_size, c; size_t ret; + if (!info->screen_buffer) + return -ENODEV; + total_size = info->screen_size; if (total_size == 0) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index abc92d79a295..b993bb97058f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -768,7 +768,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) int c, cnt = 0, err = 0; unsigned long total_size; - if (!info || ! info->screen_base) + if (!info) return -ENODEV; if (info->state != FBINFO_STATE_RUNNING) @@ -777,6 +777,9 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) if (info->fbops->fb_read) return info->fbops->fb_read(info, buf, count, ppos); + if (!info->screen_base) + return -ENODEV; + total_size = info->screen_size; if (total_size == 0) @@ -833,7 +836,7 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) int c, cnt = 0, err = 0; unsigned long total_size; - if (!info || !info->screen_base) + if (!info) return -ENODEV; if (info->state != FBINFO_STATE_RUNNING) @@ -842,6 +845,9 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos) if (info->fbops->fb_write) return info->fbops->fb_write(info, buf, count, ppos); + if (!info->screen_base) + return -ENODEV; + total_size = info->screen_size; if (total_size == 0) diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c index 6f852cd756c5..b7ad3c644e13 100644 --- a/drivers/video/fbdev/sm712fb.c +++ b/drivers/video/fbdev/sm712fb.c @@ -1028,7 +1028,7 @@ static ssize_t smtcfb_read(struct fb_info *info, char __user *buf, int c, i, cnt = 0, err = 0; unsigned long total_size; - if (!info || !info->screen_base) + if (!info->screen_base) return -ENODEV; total_size = info->screen_size; @@ -1091,7 +1091,7 @@ static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf, int c, i, cnt = 0, err = 0; unsigned long total_size; - if (!info || !info->screen_base) + if (!info->screen_base) return -ENODEV; total_size = info->screen_size;
Push the test for info->screen_base from fb_read() and fb_write() into the implementations of struct fb_ops.{fb_read,fb_write}. In cases where the driver operates on info->screen_buffer, test this field instead. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/video/fbdev/cobalt_lcdfb.c | 6 ++++++ drivers/video/fbdev/core/fb_sys_fops.c | 6 ++++++ drivers/video/fbdev/core/fbmem.c | 10 ++++++++-- drivers/video/fbdev/sm712fb.c | 4 ++-- 4 files changed, 22 insertions(+), 4 deletions(-)