[PATCH] MAINTAINERS: mark FRAMEBUFFER LAYER as Orphan
It has been a fun ride since 2017 but unfortunately I don't have enough time to look after it properly anymore. Signed-off-by: Bartlomiej Zolnierkiewicz --- MAINTAINERS |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Index: b/MAINTAINERS === --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6894,10 +6894,9 @@ F: drivers/net/wan/dlci.c F: drivers/net/wan/sdla.c FRAMEBUFFER LAYER -M: Bartlomiej Zolnierkiewicz L: dri-de...@lists.freedesktop.org L: linux-fb...@vger.kernel.org -S: Maintained +S: Orphan Q: http://patchwork.kernel.org/project/linux-fbdev/list/ T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/fb/
[PATCH] MAINTAINERS: remove LIBATA PATA DRIVERS entry
Even though there is not much happening for libata PATA drivers I don't have time to look after them anymore. Since Jens is maintaining the whole libata anyway just remove "LIBATA PATA DRIVERS" entry. Signed-off-by: Bartlomiej Zolnierkiewicz --- MAINTAINERS |9 - 1 file changed, 9 deletions(-) Index: b/MAINTAINERS === --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9912,15 +9912,6 @@ T: git git://git.kernel.org/pub/scm/linu F: drivers/ata/pata_arasan_cf.c F: include/linux/pata_arasan_cf_data.h -LIBATA PATA DRIVERS -M: Bartlomiej Zolnierkiewicz -M: Jens Axboe -L: linux-...@vger.kernel.org -S: Maintained -T: git git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git -F: drivers/ata/ata_generic.c -F: drivers/ata/pata_*.c - LIBATA PATA FARADAY FTIDE010 AND GEMINI SATA BRIDGE DRIVERS M: Linus Walleij L: linux-...@vger.kernel.org
Re: [PATCH] fbdev: sm712fb: handle ioremap() errors in probe
On 7/13/20 10:05 AM, Evgeny Novikov wrote:
> smtcfb_pci_probe() does not handle ioremap() errors for case 0x720. The
> patch fixes that exactly like for case 0x710/2.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Evgeny Novikov
Applied to drm-misc-next tree, thanks and sorry for the delay.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/sm712fb.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
> index 6a1b4a853d9e..0171b23fa211 100644
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -1602,6 +1602,14 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
> sfb->fb->fix.mmio_start = mmio_base;
> sfb->fb->fix.mmio_len = 0x0020;
> sfb->dp_regs = ioremap(mmio_base, 0x0020 + smem_size);
> + if (!sfb->dp_regs) {
> + dev_err(>dev,
> + "%s: unable to map memory mapped IO!\n",
> + sfb->fb->fix.id);
> + err = -ENOMEM;
> + goto failed_fb;
> + }
> +
> sfb->lfb = sfb->dp_regs + 0x0020;
> sfb->mmio = (smtc_regbaseaddress =
> sfb->dp_regs + 0x000c);
>
Re: [PATCH] omapfb: fix spelling mistake "propert" -> "property"
On 8/5/20 12:28 PM, Colin King wrote: > From: Colin Ian King > > There is a spelling mistake in a pr_err message. Fix it. > > Signed-off-by: Colin Ian King Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/omap2/omapfb/dss/venc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c > b/drivers/video/fbdev/omap2/omapfb/dss/venc.c > index 0b0ad20afd63..f560fa4d7786 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c > @@ -787,7 +787,7 @@ static int venc_probe_of(struct platform_device *pdev) > venc.type = OMAP_DSS_VENC_TYPE_SVIDEO; > break; > default: > - dev_err(>dev, "bad channel propert '%d'\n", channels); > + dev_err(>dev, "bad channel property '%d'\n", channels); > r = -EINVAL; > goto err; > } >
Re: [PATCH v3 00/12] video: fbdev: use generic power management
On 8/19/20 8:56 PM, Vaibhav Gupta wrote: > Linux Kernel Mentee: Remove Legacy Power Management. > > The purpose of this patch series is to upgrade power management in video fbdev > drivers. This has been done by upgrading .suspend() and .resume() callbacks. > > The upgrade makes sure that the involvement of PCI Core does not change the > order of operations executed in a driver. Thus, does not change its behavior. > > In general, drivers with legacy PM, .suspend() and .resume() make use of PCI > helper functions like pci_enable/disable_device_mem(), pci_set_power_state(), > pci_save/restore_state(), pci_enable/disable_device(), etc. to complete > their job. > > The conversion requires the removal of those function calls, change the > callbacks' definition accordingly and make use of dev_pm_ops structure. > > All patches are compile-tested only. > > Test tools: > - Compiler: gcc (GCC) 10.1.0 > - allmodconfig build: make -j$(nproc) W=1 all > > Vaibhav Gupta (12): > fbdev: gxfb: use generic power management > fbdev: lxfb: use generic power management > fbdev: via-core: use generic power management > fbdev: aty: use generic power management > fbdev: aty128fb: use generic power management > fbdev: nvidia: use generic power management > fbdev: savagefb: use generic power management > fbdev: cyber2000fb: use generic power management > fbdev: i740fb: use generic power management > fbdev: vt8623fb: use generic power management > fbdev: s3fb: use generic power management > fbdev: arkfb: use generic power management > > drivers/video/fbdev/arkfb.c | 41 ++--- > drivers/video/fbdev/aty/aty128fb.c | 51 ++-- > drivers/video/fbdev/aty/atyfb_base.c | 50 ++- > drivers/video/fbdev/cyber2000fb.c| 13 ++-- > drivers/video/fbdev/geode/gxfb.h | 5 -- > drivers/video/fbdev/geode/gxfb_core.c| 36 ++- > drivers/video/fbdev/geode/lxfb.h | 5 -- > drivers/video/fbdev/geode/lxfb_core.c| 37 +-- > drivers/video/fbdev/geode/lxfb_ops.c | 4 -- > drivers/video/fbdev/geode/suspend_gx.c | 4 -- > drivers/video/fbdev/i740fb.c | 40 +--- > drivers/video/fbdev/nvidia/nvidia.c | 64 +++- > drivers/video/fbdev/s3fb.c | 39 +--- > drivers/video/fbdev/savage/savagefb_driver.c | 52 ++-- > drivers/video/fbdev/via/via-core.c | 39 +--- > drivers/video/fbdev/vt8623fb.c | 41 ++--- > include/linux/via-core.h | 2 - > 17 files changed, 267 insertions(+), 256 deletions(-) Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH v4] video: fbdev: ssd1307fb: Added support to Column offset
[ added dri-devel ML to Cc: ] On 7/27/20 10:40 PM, Rob Herring wrote: > On Fri, 24 Jul 2020 17:22:18 -0300, Rodrigo Alencar wrote: >> This patch provides support for displays like VGM128064B0W10, >> which requires a column offset of 2, i.e., its segments starts >> in SEG2 and ends in SEG129. >> >> Signed-off-by: Rodrigo Alencar <455.rodrigo.alen...@gmail.com> >> --- >> Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 + >> drivers/video/fbdev/ssd1307fb.c | 8 ++-- >> 2 files changed, 7 insertions(+), 2 deletions(-) >> > > Acked-by: Rob Herring Applied to drm-misc-next tree, thanks and sorry for the delay. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH] video: fbdev: fix setting of pixclock because a pass-by-value error
On 7/23/20 7:02 PM, Colin King wrote:
> From: Colin Ian King
>
> The pixclock is being set locally because it is being passed as a
> pass-by-value argument rather than pass-by-reference, so the computed
> pixclock is never being set in var->pixclock. Fix this by passing
> by reference.
>
> [This dates back to 2002, I found the offending commit from the git
> history git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git ]
>
> Addresses-Coverity: ("Unused value")
> Fixes: 115f4504a64a ("Removed currcon and other console related code. Very
> little is now left.")
> Signed-off-by: Colin Ian King
Applied to drm-misc-next tree, thanks and sorry for the delay.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/vga16fb.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
> index a20eeb8308ff..52f273af6cae 100644
> --- a/drivers/video/fbdev/vga16fb.c
> +++ b/drivers/video/fbdev/vga16fb.c
> @@ -243,7 +243,7 @@ static void vga16fb_update_fix(struct fb_info *info)
> }
>
> static void vga16fb_clock_chip(struct vga16fb_par *par,
> -unsigned int pixclock,
> +unsigned int *pixclock,
> const struct fb_info *info,
> int mul, int div)
> {
> @@ -259,14 +259,14 @@ static void vga16fb_clock_chip(struct vga16fb_par *par,
> { 0 /* bad */,0x00, 0x00}};
> int err;
>
> - pixclock = (pixclock * mul) / div;
> + *pixclock = (*pixclock * mul) / div;
> best = vgaclocks;
> - err = pixclock - best->pixclock;
> + err = *pixclock - best->pixclock;
> if (err < 0) err = -err;
> for (ptr = vgaclocks + 1; ptr->pixclock; ptr++) {
> int tmp;
>
> - tmp = pixclock - ptr->pixclock;
> + tmp = *pixclock - ptr->pixclock;
> if (tmp < 0) tmp = -tmp;
> if (tmp < err) {
> err = tmp;
> @@ -275,7 +275,7 @@ static void vga16fb_clock_chip(struct vga16fb_par *par,
> }
> par->misc |= best->misc;
> par->clkdiv = best->seq_clock_mode;
> - pixclock = (best->pixclock * div) / mul;
> + *pixclock = (best->pixclock * div) / mul;
> }
>
> #define FAIL(X) return -EINVAL
> @@ -497,10 +497,10 @@ static int vga16fb_check_var(struct fb_var_screeninfo
> *var,
>
> if (mode & MODE_8BPP)
> /* pixel clock == vga clock / 2 */
> - vga16fb_clock_chip(par, var->pixclock, info, 1, 2);
> + vga16fb_clock_chip(par, >pixclock, info, 1, 2);
> else
> /* pixel clock == vga clock */
> - vga16fb_clock_chip(par, var->pixclock, info, 1, 1);
> + vga16fb_clock_chip(par, >pixclock, info, 1, 1);
>
> var->red.offset = var->green.offset = var->blue.offset =
> var->transp.offset = 0;
>
Re: [PATCH] video: fbdev: sis: fix null ptr dereference
On 8/5/20 4:52 PM, t...@redhat.com wrote:
> From: Tom Rix
>
> Clang static analysis reports this representative error
>
> init.c:2501:18: warning: Array access (from variable 'queuedata') results
> in a null pointer dereference
> templ |= ((queuedata[i] & 0xc0) << 3);
>
> This is the problem block of code
>
>if(ModeNo > 0x13) {
> ...
> if(SiS_Pr->ChipType == SIS_730) {
>queuedata = [0];
> } else {
>queuedata = [0];
> }
>} else {
>
>}
>
> queuedata is not set in the else block
>
> Reviewing the old code, the arrays FQBQData730 and FQBQData were
> used directly.
>
> So hoist the setting of queuedata out of the if-else block.
>
> Fixes: 544393fe584d ("[PATCH] sisfb update")
>
> Signed-off-by: Tom Rix
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/sis/init.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/sis/init.c b/drivers/video/fbdev/sis/init.c
> index dfe3eb769638..fde27feae5d0 100644
> --- a/drivers/video/fbdev/sis/init.c
> +++ b/drivers/video/fbdev/sis/init.c
> @@ -2428,6 +2428,11 @@ SiS_SetCRT1FIFO_630(struct SiS_Private *SiS_Pr,
> unsigned short ModeNo,
>
> i = 0;
>
> + if (SiS_Pr->ChipType == SIS_730)
> + queuedata = [0];
> + else
> + queuedata = [0];
> +
> if(ModeNo > 0x13) {
>
>/* Get VCLK */
> @@ -2445,12 +2450,6 @@ SiS_SetCRT1FIFO_630(struct SiS_Private *SiS_Pr,
> unsigned short ModeNo,
>/* Get half colordepth */
>colorth = colortharray[(SiS_Pr->SiS_ModeType - ModeEGA)];
>
> - if(SiS_Pr->ChipType == SIS_730) {
> - queuedata = [0];
> - } else {
> - queuedata = [0];
> - }
> -
>do {
>templ = SiS_CalcDelay2(SiS_Pr, queuedata[i]) * VCLK * colorth;
>
>
Re: [PATCH] coccinelle: api: fix device_attr_show.cocci warnings
Hi,
On 8/10/20 11:21 AM, kernel test robot wrote:
> From: kernel test robot
>
> drivers/video/fbdev/core/fbcon.c:3509:8-16: WARNING: use scnprintf or sprintf
> drivers/video/fbdev/core/fbcon.c:3484:8-16: WARNING: use scnprintf or sprintf
>
>
> From Documentation/filesystems/sysfs.txt:
> show() must not use snprintf() when formatting the value to be
> returned to user space. If you can guarantee that an overflow
> will never happen you can use sprintf() otherwise you must use
> scnprintf().
>
> Generated by: scripts/coccinelle/api/device_attr_show.cocci
>
> Fixes: abfc19ff202d ("coccinelle: api: add device_attr_show script")
> CC: Denis Efremov
> Signed-off-by: kernel test robot
> ---
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> master
> head: fc80c51fd4b23ec007e88d4c688f2cac1b8648e7
> commit: abfc19ff202d287742483e15fd478ddd6ada2187 coccinelle: api: add
> device_attr_show script
>
> Please take the patch only if it's a positive warning. Thanks!
>
> fbcon.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -3481,7 +3481,7 @@ static ssize_t show_rotate(struct device
> rotate = fbcon_get_rotate(info);
> err:
> console_unlock();
> - return snprintf(buf, PAGE_SIZE, "%d\n", rotate);
> + return scnprintf(buf, PAGE_SIZE, "%d\n", rotate);
buf length is at least PAGE_SIZE and rotate val is an int so
shouldn't this be converted to use sprintf() instead?
> }
>
> static ssize_t show_cursor_blink(struct device *device,
> @@ -3506,7 +3506,7 @@ static ssize_t show_cursor_blink(struct
> blink = (ops->flags & FBCON_FLAGS_CURSOR_TIMER) ? 1 : 0;
> err:
> console_unlock();
> - return snprintf(buf, PAGE_SIZE, "%d\n", blink);
> + return scnprintf(buf, PAGE_SIZE, "%d\n", blink);
ditto for blink val
> }
>
> static ssize_t store_cursor_blink(struct device *device,
>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH] coccinelle: api: fix kobj_to_dev.cocci warnings
On 8/26/20 10:54 PM, Julia Lawall wrote:
> From: kernel test robot
>
> Use kobj_to_dev() instead of container_of()
>
> Generated by: scripts/coccinelle/api/kobj_to_dev.cocci
>
> Fixes: a2fc3718bc22 ("coccinelle: api: add kobj_to_dev.cocci script")
> CC: Denis Efremov
> Signed-off-by: kernel test robot
> Signed-off-by: Julia Lawall
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/jlawall/linux.git
> for-5.10
> head: a2fc3718bc22e85378085568ecc5765fb28cabce
> commit: a2fc3718bc22e85378085568ecc5765fb28cabce [3/3] coccinelle: api: add
> kobj_to_dev.cocci script
> :: branch date: 5 days ago
> :: commit date: 5 days ago
>
> udlfb.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- a/drivers/video/fbdev/udlfb.c
> +++ b/drivers/video/fbdev/udlfb.c
> @@ -1457,7 +1457,7 @@ static ssize_t edid_show(
> struct file *filp,
> struct kobject *kobj, struct bin_attribute *a,
>char *buf, loff_t off, size_t count) {
> - struct device *fbdev = container_of(kobj, struct device, kobj);
> + struct device *fbdev = kobj_to_dev(kobj);
> struct fb_info *fb_info = dev_get_drvdata(fbdev);
> struct dlfb_data *dlfb = fb_info->par;
>
> @@ -1479,7 +1479,7 @@ static ssize_t edid_store(
> struct file *filp,
> struct kobject *kobj, struct bin_attribute *a,
> char *src, loff_t src_off, size_t src_size) {
> - struct device *fbdev = container_of(kobj, struct device, kobj);
> + struct device *fbdev = kobj_to_dev(kobj);
> struct fb_info *fb_info = dev_get_drvdata(fbdev);
> struct dlfb_data *dlfb = fb_info->par;
> int ret;
>
Re: [PATCH] video: fbdev: replace spurious snprintf() with sprintf()
On 8/24/20 7:44 PM, Alex Dewar wrote:
> par->vgapass is a u8, so if we are assuming that buf is at least
> PAGE_SIZE then the extra checking is pointless.
>
> Signed-off-by: Alex Dewar
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/sstfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
> index afe6d1b7c3a0..c05cdabeb11c 100644
> --- a/drivers/video/fbdev/sstfb.c
> +++ b/drivers/video/fbdev/sstfb.c
> @@ -733,7 +733,7 @@ static ssize_t show_vgapass(struct device *device, struct
> device_attribute *attr
> {
> struct fb_info *info = dev_get_drvdata(device);
> struct sstfb_par *par = info->par;
> - return snprintf(buf, PAGE_SIZE, "%d\n", par->vgapass);
> + return sprintf(buf, "%d\n", par->vgapass);
> }
>
> static struct device_attribute device_attrs[] = {
>
Re: [PATCH 22/29] video: fbdev: Avoid comma separated statements
On 8/25/20 6:56 AM, Joe Perches wrote:
> Use semicolons and braces.
>
> Signed-off-by: Joe Perches
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/tgafb.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c
> index e9869135d833..666fbe2f671c 100644
> --- a/drivers/video/fbdev/tgafb.c
> +++ b/drivers/video/fbdev/tgafb.c
> @@ -989,8 +989,10 @@ tgafb_fillrect(struct fb_info *info, const struct
> fb_fillrect *rect)
> /* We can fill 2k pixels per operation. Notice blocks that fit
> the width of the screen so that we can take advantage of this
> and fill more than one line per write. */
> - if (width == line_length)
> - width *= height, height = 1;
> + if (width == line_length) {
> + width *= height;
> + height = 1;
> + }
>
> /* The write into the frame buffer must be aligned to 4 bytes,
> but we are allowed to encode the offset within the word in
> @@ -1171,8 +1173,10 @@ copyarea_8bpp(struct fb_info *info, u32 dx, u32 dy,
> u32 sx, u32 sy,
> More than anything else, these control how we do copies. */
> depos = dy * line_length + dx;
> sepos = sy * line_length + sx;
> - if (backward)
> - depos += width, sepos += width;
> + if (backward) {
> + depos += width;
> + sepos += width;
> + }
>
> /* Next copy full words at a time. */
> n32 = width / 32;
>
Re: [PATCH] docs: fb: Correcting the location of FRAMEBUFFER_CONSOLE option.
[ added linux-fbdev ML to Cc: ] On 8/24/20 4:51 PM, Bilal Wasim wrote: > fbcon doc mentions FRAMEBUFFER_CONSOLE option to be under > Device Drivers->Graphics Support->Frame buffer Devices-> > Console display driver support->Framebuffer Console Support, > while its under Device Drivers->Graphics Support-> > Console display driver support->Framebuffer Console Support. > > Correcting it in the docs. > > Signed-off-by: Bilal Wasim Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > Documentation/fb/fbcon.rst | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/fb/fbcon.rst b/Documentation/fb/fbcon.rst > index e57a3d1d085a..a7b487cba307 100644 > --- a/Documentation/fb/fbcon.rst > +++ b/Documentation/fb/fbcon.rst > @@ -20,8 +20,8 @@ A. Configuration > > > The framebuffer console can be enabled by using your favorite kernel > -configuration tool. It is under Device Drivers->Graphics Support->Frame > -buffer Devices->Console display driver support->Framebuffer Console Support. > +configuration tool. It is under Device Drivers->Graphics Support-> > +Console display driver support->Framebuffer Console Support. > Select 'y' to compile support statically or 'm' for module support. The > module will be fbcon. >
Re: [PATCH v2] lib/fonts: add font 6x8 for OLED display
On 8/20/20 10:42 AM, Greg Kroah-Hartman wrote: > On Thu, Aug 20, 2020 at 10:21:37AM +0200, Sascha Hauer wrote: >> From: Sven Schneider >> >> This font is derived from lib/fonts/font_6x10.c and is useful for small >> OLED displays >> >> Signed-off-by: Sven Schneider >> Signed-off-by: Sascha Hauer > > Reviewed-by: Greg Kroah-Hartman Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH] video: fbdev: remove set but not used 'ulCoreClock'
On 8/27/20 3:00 PM, Jason Yan wrote:
> This addresses the following gcc warning with "make W=1":
>
> drivers/video/fbdev/kyro/STG4000InitDevice.c: In function
> ‘SetCoreClockPLL’:
> drivers/video/fbdev/kyro/STG4000InitDevice.c:247:6: warning: variable
> ‘ulCoreClock’ set but not used [-Wunused-but-set-variable] // yanaijie
> fixed
> 247 | u32 ulCoreClock;
> | ^~~
>
> Reported-by: Hulk Robot
> Signed-off-by: Jason Yan
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/kyro/STG4000InitDevice.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/kyro/STG4000InitDevice.c
> b/drivers/video/fbdev/kyro/STG4000InitDevice.c
> index 1d3f2080aa6f..edaeec2d9590 100644
> --- a/drivers/video/fbdev/kyro/STG4000InitDevice.c
> +++ b/drivers/video/fbdev/kyro/STG4000InitDevice.c
> @@ -244,7 +244,6 @@ int SetCoreClockPLL(volatile STG4000REG __iomem *pSTGReg,
> struct pci_dev *pDev)
> {
> u32 F, R, P;
> u16 core_pll = 0, sub;
> - u32 ulCoreClock;
> u32 tmp;
> u32 ulChipSpeed;
>
> @@ -282,7 +281,7 @@ int SetCoreClockPLL(volatile STG4000REG __iomem *pSTGReg,
> struct pci_dev *pDev)
> if (ulChipSpeed == 0)
> return -EINVAL;
>
> - ulCoreClock = ProgramClock(REF_FREQ, CORE_PLL_FREQ, , , );
> + ProgramClock(REF_FREQ, CORE_PLL_FREQ, , , );
>
> core_pll |= ((P) | ((F - 2) << 2) | ((R - 2) << 11));
>
Re: [PATCH] video: fbdev: remove set but not used 'ulBestVCO'
On 8/27/20 3:00 PM, Jason Yan wrote:
> This addresses the following gcc warning with "make W=1":
>
> drivers/video/fbdev/kyro/STG4000InitDevice.c: In function
> ‘ProgramClock’:
> drivers/video/fbdev/kyro/STG4000InitDevice.c:123:6: warning: variable
> ‘ulBestVCO’ set but not used [-Wunused-but-set-variable]
> 123 | u32 ulBestVCO = 0, ulBestClk = 0, ulBestScore = 0;
> | ^
>
> Reported-by: Hulk Robot
> Signed-off-by: Jason Yan
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/kyro/STG4000InitDevice.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/kyro/STG4000InitDevice.c
> b/drivers/video/fbdev/kyro/STG4000InitDevice.c
> index edaeec2d9590..21875d3c2dc2 100644
> --- a/drivers/video/fbdev/kyro/STG4000InitDevice.c
> +++ b/drivers/video/fbdev/kyro/STG4000InitDevice.c
> @@ -120,7 +120,7 @@ u32 ProgramClock(u32 refClock,
> {
> u32 R = 0, F = 0, OD = 0, ODIndex = 0;
> u32 ulBestR = 0, ulBestF = 0, ulBestOD = 0;
> - u32 ulBestVCO = 0, ulBestClk = 0, ulBestScore = 0;
> + u32 ulBestClk = 0, ulBestScore = 0;
> u32 ulScore, ulPhaseScore, ulVcoScore;
> u32 ulTmp = 0, ulVCO;
> u32 ulScaleClockReq, ulMinClock, ulMaxClock;
> @@ -189,7 +189,6 @@ u32 ProgramClock(u32 refClock,
> ulScore = ulPhaseScore +
> ulVcoScore;
>
> if (!ulBestScore) {
> - ulBestVCO = ulVCO;
> ulBestOD = OD;
> ulBestF = F;
> ulBestR = R;
> @@ -206,7 +205,6 @@ u32 ProgramClock(u32 refClock,
>but we shall keep this code in case new
> restrictions come into play
>
> --*/
> if ((ulScore >= ulBestScore) &&
> (OD > 0)) {
> - ulBestVCO = ulVCO;
> ulBestOD = OD;
> ulBestF = F;
> ulBestR = R;
>
Re: [PATCH v1 0/2] video: fbdev: radeonfb: PCI PM framework upgrade and fix-ups.
On 9/7/20 9:02 AM, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
>
> The original goal of the patch series is to upgrade the power management
> framework of radeonfb fbdev driver. This has been done by upgrading .suspend()
> and .resume() callbacks.
>
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
>
> During this process, it was found that "#if defined(CONFIG_PM)" at line 1434
> is
> redundant. This was introduced in the commit
> 42ddb453a0cd ("radeon: Conditionally compile PM code").
>
>
>
> Before 42ddb453a0cd:
> $ git show 65122f7e80b5:drivers/video/aty/radeon_pm.c | grep -n
> "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
>
> Based on output in terminal:
>
> 547:#ifdef CONFIG_PM
>|-- 959:#ifdef CONFIG_PPC_PMAC
>|-- 972:#endif
>|-- 1291:#ifdef CONFIG_PPC_OF
>|-- 1301:#endif /* CONFIG_PPC_OF */
>|-- 1943:#ifdef CONFIG_PPC_OF
>|-- 2206:#if 0 /* Not ready yet */
>|-- 2508:#endif /* 0 */
>|-- 2510:#endif /* CONFIG_PPC_OF */
>|-- 2648:#ifdef CONFIG_PPC_PMAC
>|-- 2654:#endif /* CONFIG_PPC_PMAC */
>|-- 2768:#ifdef CONFIG_PPC_PMAC
>|-- 2774:#endif /* CONFIG_PPC_PMAC */
>|-- 2791:#ifdef CONFIG_PPC_OF__disabled
>|-- 2801:#endif /* CONFIG_PPC_OF */
> 2803:#endif /* CONFIG_PM */
>
>
>
> After 42ddb453a0cd:
> $ git show 42ddb453a0cd:drivers/video/aty/radeon_pm.c | grep -n
> "#ifdef\|#if\|#else\|#endif\|#elif\|#ifndef"
>
> Based on output in terminal:
>
> 547:#ifdef CONFIG_PM
>|-- 959:#ifdef CONFIG_PPC_PMAC
>|-- 972:#endif
>|-- 1291:#ifdef CONFIG_PPC_OF
>|-- 1301:#endif /* CONFIG_PPC_OF */
>|-- 1430:#if defined(CONFIG_PM)
>|-- 1431:#if defined(CONFIG_X86) ||
> defined(CONFIG_PPC_PMAC)
>|-- 1944:#endif
>|-- 1946:#ifdef CONFIG_PPC_OF
>|-- 1947:#ifdef CONFIG_PPC_PMAC
>|-- 2208:#endif
>|-- 2209:#endif
>|-- 2211:#if 0 /* Not ready yet */
>|-- 2513:#endif /* 0 */
>|-- 2515:#endif /* CONFIG_PPC_OF */
>|-- 2653:#ifdef CONFIG_PPC_PMAC
>|-- 2659:#endif /* CONFIG_PPC_PMAC */
>|-- 2773:#ifdef CONFIG_PPC_PMAC
>|-- 2779:#endif /* CONFIG_PPC_PMAC */
>|-- 2796:#ifdef CONFIG_PPC_OF__disabled
>|-- 2806:#endif /* CONFIG_PPC_OF */
> 2808:#endif /* CONFIG_PM */
>
>
>
> This also affected the CONFIG_PPC_OF container (line 1943 at commit
> 65122f7e80b5)
>
> The patch-series fixes it along with PM upgrade.
>
> All patches are compile-tested only.
>
> Test tools:
> - Compiler: gcc (GCC) 10.1.0
> - allmodconfig build: make -j$(nproc) W=1 all
>
> Vaibhav Gupta (2):
> video: fbdev: aty: radeon_pm: remove redundant CONFIG_PM container
> fbdev: radeonfb:use generic power management
>
> drivers/video/fbdev/aty/radeon_base.c | 10 ---
> drivers/video/fbdev/aty/radeon_pm.c | 38 ---
> drivers/video/fbdev/aty/radeonfb.h| 3 +--
> 3 files changed, 35 insertions(+), 16 deletions(-)
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH] fbdev: remove mbx framebuffer driver
On 8/30/20 1:55 PM, Mike Rapoport wrote:
> From: Mike Rapoport
>
> The only in-tree user for mbx driver for Intel 2700G graphics chip was
> cm-x270 platform. Since this platform was removed by the commit
> 9d3239147d6d ("ARM: pxa: remove Compulab pxa2xx boards") there is no
> point to keep the obsolete framebuffer driver.
>
> Signed-off-by: Mike Rapoport
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> .../userspace-api/ioctl/ioctl-number.rst |2 -
> drivers/video/fbdev/Kconfig | 19 -
> drivers/video/fbdev/Makefile |1 -
> drivers/video/fbdev/mbx/Makefile |4 -
> drivers/video/fbdev/mbx/mbxdebugfs.c | 232
> drivers/video/fbdev/mbx/mbxfb.c | 1053 -
> drivers/video/fbdev/mbx/reg_bits.h| 614 --
> drivers/video/fbdev/mbx/regs.h| 196 ---
> include/video/mbxfb.h | 99 --
> 9 files changed, 2220 deletions(-)
> delete mode 100644 drivers/video/fbdev/mbx/Makefile
> delete mode 100644 drivers/video/fbdev/mbx/mbxdebugfs.c
> delete mode 100644 drivers/video/fbdev/mbx/mbxfb.c
> delete mode 100644 drivers/video/fbdev/mbx/reg_bits.h
> delete mode 100644 drivers/video/fbdev/mbx/regs.h
> delete mode 100644 include/video/mbxfb.h
>
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 2a198838fca9..a20102f7db69 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -356,8 +356,6 @@ Code Seq#Include File
>Comments
> 0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h
> ChromeOS EC driver
> 0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb
> (in development)
>
> <mailto:tho...@winischhofer.net>
> -0xF4 00-1F video/mbxfb.h mbxfb
> -
> <mailto:r...@8d.com>
> 0xF6 allLTTng
> Linux Trace Toolkit Next Generation
>
> <mailto:mathieu.desnoy...@efficios.com>
> 0xFD alllinux/dm-ioctl.h
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index b2c9dd4f0cb5..e36578258b5b 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -1775,25 +1775,6 @@ config PXA3XX_GCU
>
> If you compile this as a module, it will be called pxa3xx_gcu.
>
> -config FB_MBX
> - tristate "2700G LCD framebuffer support"
> - depends on FB && ARCH_PXA
> - select FB_CFB_FILLRECT
> - select FB_CFB_COPYAREA
> - select FB_CFB_IMAGEBLIT
> - help
> - Framebuffer driver for the Intel 2700G (Marathon) Graphics
> - Accelerator
> -
> -config FB_MBX_DEBUG
> - bool "Enable debugging info via debugfs"
> - depends on FB_MBX && DEBUG_FS
> - help
> - Enable this if you want debugging information using the debug
> - filesystem (debugfs)
> -
> - If unsure, say N.
> -
> config FB_FSL_DIU
> tristate "Freescale DIU framebuffer support"
> depends on FB && FSL_SOC
> diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
> index cad4fb64442a..2ff8849ffde6 100644
> --- a/drivers/video/fbdev/Makefile
> +++ b/drivers/video/fbdev/Makefile
> @@ -31,7 +31,6 @@ obj-$(CONFIG_FB_VIA) += via/
> obj-$(CONFIG_FB_KYRO) += kyro/
> obj-$(CONFIG_FB_SAVAGE)+= savage/
> obj-$(CONFIG_FB_GEODE) += geode/
> -obj-$(CONFIG_FB_MBX) += mbx/
> obj-$(CONFIG_FB_NEOMAGIC) += neofb.o
> obj-$(CONFIG_FB_3DFX) += tdfxfb.o
> obj-$(CONFIG_FB_CONTROL) += controlfb.o
> diff --git a/drivers/video/fbdev/mbx/Makefile
> b/drivers/video/fbdev/mbx/Makefile
> deleted file mode 100644
> index 3e8e7ff41f18..
> --- a/drivers/video/fbdev/mbx/Makefile
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -# Makefile for the 2700G controller driver.
> -
> -obj-y+= mbxfb.o
> diff --git a/drivers/video/fbdev/mbx/mbxdebugfs.c
> b/drivers/video/fbdev/mbx/mbxdebugfs.c
> deleted file mode 100644
>
Re: [PATCH] video: fbdev: radeon: Fix memleak in radeonfb_pci_register
On 8/25/20 9:47 AM, Mathieu Malaterre wrote:
> On Tue, Aug 25, 2020 at 9:27 AM Dinghao Liu wrote:
>>
>> When radeon_kick_out_firmware_fb() fails, info should be
>> freed just like the subsequent error paths.
>>
>> Fixes: 069ee21a82344 ("fbdev: Fix loading of module radeonfb on PowerMac")
>> Signed-off-by: Dinghao Liu
>> ---
>> drivers/video/fbdev/aty/radeon_base.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/aty/radeon_base.c
>> b/drivers/video/fbdev/aty/radeon_base.c
>> index 3fe509cb9b87..13bd2bd5c043 100644
>> --- a/drivers/video/fbdev/aty/radeon_base.c
>> +++ b/drivers/video/fbdev/aty/radeon_base.c
>> @@ -2307,7 +2307,7 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
>>
>> ret = radeon_kick_out_firmware_fb(pdev);
>> if (ret)
>> - return ret;
>> + goto err_release_fb;
>
> Good catch ! Thanks
>
> Reviewed-by: Mathieu Malaterre
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
>> /* request the mem regions */
>> ret = pci_request_region(pdev, 0, "radeonfb framebuffer");
>> --
>> 2.17.1
Re: [PATCH v2 00/41] spi / fbdev / cpufreq / usb / mmc / hwmon / ARM: Prepare for multiplatform S3C
[ trimmed Cc: list ] On 8/20/20 5:59 PM, Krzysztof Kozlowski wrote: > On Thu, Aug 06, 2020 at 08:19:32PM +0200, Krzysztof Kozlowski wrote: >> Hi All, >> >> Shortly >> === >> This is a continuation of Arnd's work from 2019 [1]. The goal is to >> cleanup, merge and finally make the Samsung S3C24xx and S3C64xx >> architectures multiplatform. The multiplatform did not happen yet >> here... just cleaning up and merging into one mach-s3c. >> >> I intend to take it through Samsung SoC tree so all Acks are welcomed. >> >> Changes since v1 >> >> 1. Rebased, >> 2. Addressed comments (including mine), >> 3. Few new patches. >> >> Please see individual changelogs (per patch). >> >> [1] >> https://patchwork.kernel.org/project/linux-samsung-soc/list/?series=185855=* >> > > Hi All, Hi Krzysztof, > I applied second part of the set which finishes this step of S3C > cleanup. Thanks to Arnd for the work! Thank you (& Arnd) for working on this! PS I've checked fbdev related changes now and they all look good to me. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > Best regards, > Krzysztof
Re: [PATCH] video: fbdev: controlfb: Fix build for COMPILE_TEST=y && PPC_PMAC=n
On 8/21/20 12:49 PM, Michael Ellerman wrote:
> The build is currently broken, if COMPILE_TEST=y and PPC_PMAC=n:
>
> linux/drivers/video/fbdev/controlfb.c: In function ‘control_set_hardware’:
> linux/drivers/video/fbdev/controlfb.c:276:2: error: implicit declaration of
> function ‘btext_update_display’
> 276 | btext_update_display(p->frame_buffer_phys + CTRLFB_OFF,
> | ^~~~
>
> Fix it by including btext.h whenever CONFIG_BOOTX_TEXT is enabled.
>
> Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
> Signed-off-by: Michael Ellerman
Acked-by: Bartlomiej Zolnierkiewicz
Thanks for fixing this.
> ---
> drivers/video/fbdev/controlfb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> Does anyone mind if I apply this via the powerpc tree for v5.9?
>
> It would be nice to get the build clean.
No objections from my side.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> cheers
>
> diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
> index 9c4f1be856ec..547abeb39f87 100644
> --- a/drivers/video/fbdev/controlfb.c
> +++ b/drivers/video/fbdev/controlfb.c
> @@ -49,6 +49,8 @@
> #include
> #ifdef CONFIG_PPC_PMAC
> #include
> +#endif
> +#ifdef CONFIG_BOOTX_TEXT
> #include
> #endif
>
Re: [PATCH] MAINTAINERS: enlist Greg formally for console stuff
On 8/3/20 4:11 PM, Daniel Vetter wrote: > I did a few greps for main console data structures, and there's a few > places outside of drivers/video/console: > - a braille driver > - a sisusbvga driver > - fbcon, but I think that's fine if we leave that officially under > fbdev maintainership > - lots of stuff in drivers/tty/vt, which is already under Greg's > maintainership. > > So I think this match gives reasonably useful Cc: lists for the files > and places I've tested. > > Cc: Bartlomiej Zolnierkiewicz > Cc: Greg KH > Cc: dri-de...@lists.freedesktop.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Daniel Vetter Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > MAINTAINERS | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index ab94723c0cae..8084d118892c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4343,6 +4343,12 @@ L: net...@vger.kernel.org > S: Maintained > F: drivers/connector/ > > +CONSOLE SUBSYSTEM > +M: Greg Kroah-Hartman > +S: Supported > +F: drivers/video/console/ > +F: include/linux/console* > + > CONTROL GROUP (CGROUP) > M: Tejun Heo > M: Li Zefan >
Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer
On 8/3/20 11:47 AM, Greg KH wrote: > On Mon, Aug 03, 2020 at 10:45:07AM +0200, Daniel Vetter wrote: >> On Mon, Aug 3, 2020 at 10:26 AM Greg KH wrote: >>> >>> On Mon, Aug 03, 2020 at 10:08:43AM +0200, Jiri Slaby wrote: >>>> Hi, >>>> >>>> On 31. 07. 20, 7:22, 张云海 wrote: >>>>> Remove whitespace at EOL >>>> >>>> I am fine with the patch. However it should be sent properly (inline >>>> mail, having a PATCH subject etc. -- see >>>> Documentation/process/submitting-patches.rst). git send-email after git >>>> format-patch handles most of it. >>>> >>>> There is also question who is willing to take it? >>>> >>>> Bart? Greg? Should we route it via akpm, or will you Linus directly? (I >>>> can sign off and resend the patch which was attached to the mail I am >>>> replying to too, if need be.) >>> >>> I can take it, if Bart can't, just let me know. >> >> Yeah vt stuff and console drivers != fbcon go through Greg's tree past >> few years ... >> >> Greg, should we maybe add a MAINTAINERS entry that matches on all >> things console? With tty/vt you definitely have the other side of that >> coin already :-) > > Sure, that would be good as things do fall through the cracks at times. Since taking over fbdev in 2017 I've tried to act as the last resort Maintainer for console stuff (AFAIK there are no "lost" patches) but it really deserves its own entry. Also most console patches make it through you nowadays anyway: $ git log --pretty=fuller --since=2017 drivers/video/console/|grep "Commit\:"|sort|uniq -cd 2 Commit: Arnd Bergmann 11 Commit: Bartlomiej Zolnierkiewicz 2 Commit: Daniel Vetter 3 Commit: Dave Airlie 12 Commit: Greg Kroah-Hartman 7 Commit: Linus Torvalds 2 Commit: Martin Schwidefsky > If you write the patch, I'll merge it :) ACK from me. :) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer
Hi Jiri,
On 7/29/20 9:02 AM, Jiri Slaby wrote:
> The current vgacon's scroll up implementation uses a circural buffer
> in vgacon_scrollback_cur. It always advances tail to prepare it for the
> next write and caps it to zero if the next ->vc_size_row bytes won't fit.
>
> But when we change the VT size (e.g. by VT_RESIZE) in the meantime, the new
> line might not fit to the end of the scrollback buffer in the next
> attempt to scroll. This leads to various crashes as
> vgacon_scrollback_update writes out of the buffer:
> BUG: unable to handle page fault for address: c91752a0
> #PF: supervisor write access in kernel mode
> #PF: error_code(0x0002) - not-present page
> RIP: 0010:mutex_unlock+0x13/0x30
> ...
> Call Trace:
> n_tty_write+0x1a0/0x4d0
> tty_write+0x1a0/0x2e0
>
> Or to KASAN reports:
> BUG: KASAN: slab-out-of-bounds in vgacon_scroll+0x57a/0x8ed
>
> So check whether the line fits in the buffer and wrap if needed. Do it
> before the loop as console_sem is held and ->vc_size_row cannot change
> during the execution of vgacon_scrollback_cur. If it does change, we
> need to ensure it does not change elsewhere, not here.
>
> Also, we do not split the write of a line into chunks as that would
> break the consumers of the buffer. They expect ->cnt, ->tail and ->size
> to be in harmony and advanced by ->vc_size_row.
>
> I found few reports of this in the past, some with patches included,
> some even 2 years old:
> https://lore.kernel.org/lkml/CAEAjamsJnG-=TSOwgRbbb3B9Z-PA63oWmNPoKYWQ=Z=+x49...@mail.gmail.com/
Sorry but I don't work on fixing fbdev/console KASAN/syzbot/etc.
reports (-ENORESOURCES). This has been made official in the past.
I'm happy to review/apply patches though.
> https://lore.kernel.org/lkml/1589336932-35508-1-git-send-email-yangyingli...@huawei.com/
This was the first time the patch for issue was submitted.
I tried to apply it to drm-misc but then I have noticed that
it has not been posted to linux-fbdev / dri-devel MLs (so it
was not possible to merge it using dim tool) and thus I've
requested the author to resend it:
https://lore.kernel.org/lkml/62544bd9-e47d-e7f9-92f2-49b8dbb13...@samsung.com/
which he did:
https://lore.kernel.org/lkml/20200713105730.550334-1-yangyingli...@huawei.com/
and the patch is currently under review period (to give people
chance to comment on it) and in my "to apply if no objections"
folder.
I see that your/Yunhai patch addresses the root source of
the issue so I'll be happy to apply/ACK it instead of Yang's
patch once the final version is posted.
Thank you for working on this.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> This fixes CVE-2020-14331.
>
> Big thanks to guys mentioned in the Reported-and-debugged-by lines below
> who actually found the root cause.
>
> Signed-off-by: Jiri Slaby
> Reported-and-debugged-by: 张云海
> Reported-and-debugged-by: Yang Yingliang
> Reported-by: Kyungtae Kim
> Fixes: 15bdab959c9b ([PATCH] vgacon: Add support for soft scrollback)
> Cc: Linus Torvalds
> Cc: Greg KH
> Cc: Solar Designer
> Cc: "Srivatsa S. Bhat"
> Cc: Anthony Liguori
> Cc: Security Officers
> Cc: linux-dist...@vs.openwall.org
> Cc: Yang Yingliang
> Cc: Bartlomiej Zolnierkiewicz
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-fb...@vger.kernel.org
> ---
> drivers/video/console/vgacon.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index f0f3d573f848..13194bb246f8 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -250,6 +250,11 @@ static void vgacon_scrollback_update(struct vc_data *c,
> int t, int count)
>
> p = (void *) (c->vc_origin + t * c->vc_size_row);
>
> + /* vc_size_row might have changed by VT_RESIZE in the meantime */
> + if ((vgacon_scrollback_cur->tail + c->vc_size_row) >=
> + vgacon_scrollback_cur->size)
> + vgacon_scrollback_cur->tail = 0;
> +
> while (count--) {
> scr_memcpyw(vgacon_scrollback_cur->data +
> vgacon_scrollback_cur->tail,
>
Re: [PATCH v2] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.
On 7/23/20 4:21 PM, Greg Kroah-Hartman wrote:
> On Wed, Jul 22, 2020 at 10:07:06AM +0200, Daniel Vetter wrote:
>> On Tue, Jul 21, 2020 at 6:08 PM Greg Kroah-Hartman
>> wrote:
>>>
>>> On Thu, Jul 16, 2020 at 08:27:21PM +0900, Tetsuo Handa wrote:
>>>> On 2020/07/16 19:00, Daniel Vetter wrote:
>>>>> On Thu, Jul 16, 2020 at 12:29:00AM +0900, Tetsuo Handa wrote:
>>>>>> On 2020/07/16 0:12, Dan Carpenter wrote:
>>>>>>> I've complained about integer overflows in fbdev for a long time...
>>>>>>>
>>>>>>> What I'd like to see is something like the following maybe. I don't
>>>>>>> know how to get the vc_data in fbmem.c so it doesn't include your checks
>>>>>>> for negative.
>>>>>>
>>>>>> Yes. Like I said "Thus, I consider that we need more sanity/constraints
>>>>>> checks." at
>>>>>> https://lore.kernel.org/lkml/b1e7dd6a-fc22-bba8-0abb-d3e779329...@i-love.sakura.ne.jp/
>>>>>> ,
>>>>>> we want basic checks. That's a task for fbdev people who should be
>>>>>> familiar with
>>>>>> necessary constraints.
>>>>>
>>>>> I think the worldwide supply of people who understand fbdev and willing to
>>>>> work on it is roughly 0. So if someone wants to fix this mess properly
>>>>> (which likely means adding tons of over/underflow checks at entry points,
>>>>> since you're never going to catch the driver bugs, there's too many and
>>>>> not enough people who care) they need to fix this themselves.
>>>>
>>>> But I think we can enforce reasonable constraint which is much stricter
>>>> than Dan's basic_checks()
>>>> (which used INT_MAX). For example, do we need to accept var->{xres,yres}
>>>> >= 1048576, for
>>>> "32768 rows or cols" * "32 pixels per character" = 1045876 and
>>>> vc_do_resize() accepts only
>>>> rows and cols < 32768 ?
>>>>
>>>>>
>>>>> Just to avoid confusion here.
>>>>>
>>>>>> Anyway, my two patches are small and low cost; can we apply these
>>>>>> patches regardless
>>>>>> of basic checks?
>>>>>
>>>>> Which two patches where?
>>>>
>>>> [PATCH v3] vt: Reject zero-sized screen buffer size.
>>>> from
>>>> https://lkml.kernel.org/r/20200712111013.11881-1-penguin-ker...@i-love.sakura.ne.jp
>>>
>>> This is now in my tree.
>>>
>>>> [PATCH v2] fbdev: Detect integer underflow at "struct
>>>> fbcon_ops"->clear_margins.
>>>> from
>>>> https://lkml.kernel.org/r/20200715015102.3814-1-penguin-ker...@i-love.sakura.ne.jp
>>>
>>> That should be taken by the fbdev maintainer, but I can take it too if
>>> people want.
>>
>> Just missed this weeks pull request train and feeling like not worth
>> making this an exception (it's been broken forever after all), so
>> maybe best if you just add this to vt.
>>
>> Acked-by: Daniel Vetter
>>
>> Also this avoids the impression I know what's going on in fbdev code,
>> maybe with sufficient abandon from my side someone will pop up who
>> cares an fixes the bazillion of syzkaller issues we seem to have
>> around console/vt and everything related.
>
> Great, will go queue it up now, thanks!
Fine with me, thanks!
PS I'll later queue the patch from George to drm-misc-next (after
reading both fbdev patches in detail it seems that both are needed).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
On 7/17/20 1:53 PM, Lukasz Luba wrote: > > > On 7/14/20 10:01 AM, Lukasz Luba wrote: >> Hi Bartek, >> >> On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote: >>> >>> Hi, >>> >>> On 7/10/20 9:11 PM, Lukasz Luba wrote: >>>> The driver can operate in two modes relaying on devfreq monitoring >>>> mechanism which periodically checks the device status or it can use >>>> interrupts when they are provided by loaded Device Tree. The newly >>>> introduced module parameter can be used to choose between devfreq >>>> monitoring and internal interrupts without modifying the Device Tree. >>>> It also sets devfreq monitoring as default when the parameter is not set >>>> (also the case for default when the driver is not built as a module). >>> >>> Could you please explain why should we leave the IRQ mode >>> support in the dmc driver? >> >> I am still experimenting with the IRQ mode in DMC, but have limited time >> for it and no TRM. >> The IRQ mode in memory controller or bus controller has one major >> advantage: is more interactive. In polling we have fixed period, i.e. >> 100ms - that's a lot when we have a sudden, latency sensitive workload. >> There might be no check of the device load for i.e. 99ms, but the tasks >> with such workload started running. That's a long period of a few frames >> which are likely to be junked. Should we adjust polling interval to i.e. >> 10ms, I don't think so. There is no easy way to address all of the >> scenarios. >> >>> >>> What are the advantages over the polling mode? >> >> As described above: more reactive to sudden workload, which might be >> latency sensitive and cause junk frames. >> Drawback: not best in benchmarks which are randomly jumping >> over the data set, causing low traffic on memory. >> It could be mitigated as Sylwester described with not only one type >> of interrupt, but another, which could 'observe' also other information >> type in the counters and fire. >> >>> >>> In what scenarios it should be used? >> >> System like Android with GUI, when there is this sudden workload >> quite often. >> >> I think the interconnect could help here and would adjust the DMC >> freq upfront. Although I don't know if interconnect on Exynos5422 is in >> your scope in near future. Of course the interconnect will not cover >> all scenarios either. >> >> >>> >>> [ If this is only for documentation purposes then it should be >>> removed as it would stay in (easily accessible) git history >>> anyway.. ] >> >> The current interrupt mode is definitely not perfect and switching >> to devfreq monitoring mode has more sense. On the other hand, it >> still has potential, until there is no interconnect for this SoC. >> I will continue experimenting with irq mode, so I would like to >> still have the code in the driver. >> >> Regards, >> Lukasz >> >>> >>> Best regards, >>> -- >>> Bartlomiej Zolnierkiewicz >>> Samsung R Institute Poland >>> Samsung Electronics >>> > > Bartek, do you have some objections to the patches or you think > they can be taken via devfreq-next? No objections from me, thank you for the IRQ mode explanation. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH v3 0/3] drivers: ide: use generic power management
On 7/14/20 9:52 AM, Vaibhav Gupta wrote: > On Tue, Jul 14, 2020 at 09:32:56AM +0200, Bartlomiej Zolnierkiewicz wrote: >> >> Hi, >> >> On 7/13/20 7:36 PM, Vaibhav Gupta wrote: >>> Linux Kernel Mentee: Remove Legacy Power Management. >>> >>> The purpose of this patch series is to remove legacy power management >>> callbacks >>> from ide drivers. >>> >>> The suspend() and resume() callbacks operations are still invoking >>> pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(), >>> etc. and handling the power management themselves, which is not recommended. >>> >>> The conversion requires the removal of the those function calls and change >>> the >>> callback definition accordingly and make use of dev_pm_ops structure. >> >> IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata >> subsystem (drivers/ata/). >> >> libata drivers have the same issue so please concentrate on fixing them >> first. Later (if desirable) changes can be back-ported to drivers/ide/. >> > Hello, (drivers/ide) and (drivers/ata) are the two major families, I am > working > on, for generic PM upgradation. I was bit unaware about priority, and also in > the last, both ide and ata drivers have to be upgraded. Well, drivers/ide/ is scheduled for removal in 2021 (it even prints the warning during initialization of every host driver).. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics >>> All patches are compile-tested only. >> >> This patchset needs (at least) some basic testing. It should be easier with >> libata subsystem as it also support SATA controllers and devices. > To upgrade PM in (drivers/ide) I have made .suspend() and .resume() static. > Then > bind them in "struct dev_pm_ops" variable (ide_pci_pm_ops) and expose it using > EXPORT_SYMBOL_GPL(). This has affected 30 drivers. I was hoping if ide changes > can be tested/verified, specially [PATCH 1/3]. As then, I will be sure about > similar change in ata, as it also requires similar alteration. > > Thanks > Vaibhav Gupta > >> >> Best regards, >> -- >> Bartlomiej Zolnierkiewicz >> Samsung R Institute Poland >> Samsung Electronics >> >>> v3: >>> - Modpost error for undefined reference by Kbuild in v1. >>> - Another approach to disable PM in drivers/ide/triflex.c suggested by >>> Bjorn Helgaas in v2. >>> >>> Test tools: >>> - Compiler: gcc (GCC) 10.1.0 >>> - allmodconfig build: make -j$(nproc) W=1 all >>> >>> Vaibhav Gupta (3): >>> ide: use generic power management >>> ide: sc1200: use generic power management >>> ide: delkin_cb: use generic power management >>> >>> drivers/ide/aec62xx.c | 3 +-- >>> drivers/ide/alim15x3.c| 3 +-- >>> drivers/ide/amd74xx.c | 3 +-- >>> drivers/ide/atiixp.c | 3 +-- >>> drivers/ide/cmd64x.c | 3 +-- >>> drivers/ide/cs5520.c | 3 +-- >>> drivers/ide/cs5530.c | 3 +-- >>> drivers/ide/cs5535.c | 3 +-- >>> drivers/ide/cs5536.c | 3 +-- >>> drivers/ide/cy82c693.c| 3 +-- >>> drivers/ide/delkin_cb.c | 32 +- >>> drivers/ide/hpt366.c | 3 +-- >>> drivers/ide/ide-pci-generic.c | 3 +-- >>> drivers/ide/it8172.c | 3 +-- >>> drivers/ide/it8213.c | 3 +-- >>> drivers/ide/it821x.c | 3 +-- >>> drivers/ide/jmicron.c | 3 +-- >>> drivers/ide/ns87415.c | 3 +-- >>> drivers/ide/opti621.c | 3 +-- >>> drivers/ide/pdc202xx_new.c| 3 +-- >>> drivers/ide/pdc202xx_old.c| 3 +-- >>> drivers/ide/piix.c| 3 +-- >>> drivers/ide/sc1200.c | 43 --- >>> drivers/ide/serverworks.c | 3 +-- >>> drivers/ide/setup-pci.c | 29 +-- >>> drivers/ide/siimage.c | 3 +-- >>> drivers/ide/sis5513.c | 3 +-- >>> drivers/ide/sl82c105.c| 3 +-- >>> drivers/ide/slc90e66.c| 3 +-- >>> drivers/ide/triflex.c | 24 +++ >>> drivers/ide/via82cxxx.c | 3 +-- >>> include/linux/ide.h | 8 +-- >>> 32 files changed, 62 insertions(+), 155 deletions(-) >>> >> > >
Re: [PATCH v2] efi: avoid error message when booting under Xen
On 7/10/20 4:22 PM, Juergen Gross wrote:
> efifb_probe() will issue an error message in case the kernel is booted
> as Xen dom0 from UEFI as EFI_MEMMAP won't be set in this case. Avoid
> that message by calling efi_mem_desc_lookup() only if EFI_MEMMAP is set.
>
> Fixes: 38ac0287b7f4 ("fbdev/efifb: Honour UEFI memory map attributes when
> mapping the FB")
> Signed-off-by: Juergen Gross
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/efifb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 65491ae74808..e57c00824965 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -453,7 +453,7 @@ static int efifb_probe(struct platform_device *dev)
> info->apertures->ranges[0].base = efifb_fix.smem_start;
> info->apertures->ranges[0].size = size_remap;
>
> - if (efi_enabled(EFI_BOOT) &&
> + if (efi_enabled(EFI_MEMMAP) &&
> !efi_mem_desc_lookup(efifb_fix.smem_start, )) {
> if ((efifb_fix.smem_start + efifb_fix.smem_len) >
> (md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT))) {
>
Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
Hi,
On 7/10/20 9:11 PM, Lukasz Luba wrote:
> The driver can operate in two modes relaying on devfreq monitoring
> mechanism which periodically checks the device status or it can use
> interrupts when they are provided by loaded Device Tree. The newly
> introduced module parameter can be used to choose between devfreq
> monitoring and internal interrupts without modifying the Device Tree.
> It also sets devfreq monitoring as default when the parameter is not set
> (also the case for default when the driver is not built as a module).
Could you please explain why should we leave the IRQ mode
support in the dmc driver?
What are the advantages over the polling mode?
In what scenarios it should be used?
[ If this is only for documentation purposes then it should be
removed as it would stay in (easily accessible) git history
anyway.. ]
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Reported-by: Willy Wolff
> Signed-off-by: Lukasz Luba
> ---
> drivers/memory/samsung/exynos5422-dmc.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/samsung/exynos5422-dmc.c
> b/drivers/memory/samsung/exynos5422-dmc.c
> index e03ee35f0ab5..53bfe6b7b703 100644
> --- a/drivers/memory/samsung/exynos5422-dmc.c
> +++ b/drivers/memory/samsung/exynos5422-dmc.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -21,6 +22,10 @@
> #include "../jedec_ddr.h"
> #include "../of_memory.h"
>
> +static int irqmode;
> +module_param(irqmode, int, 0644);
> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
> +
> #define EXYNOS5_DREXI_TIMINGAREF (0x0030)
> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
> #define EXYNOS5_DREXI_TIMINGDATA0(0x0038)
> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device
> *pdev)
> /* There is two modes in which the driver works: polling or IRQ */
> irq[0] = platform_get_irq_byname(pdev, "drex_0");
> irq[1] = platform_get_irq_byname(pdev, "drex_1");
> - if (irq[0] > 0 && irq[1] > 0) {
> + if (irq[0] > 0 && irq[1] > 0 && irqmode) {
> ret = devm_request_threaded_irq(dev, irq[0], NULL,
> dmc_irq_thread, IRQF_ONESHOT,
> dev_name(dev), dmc);
> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device
> *pdev)
> if (dmc->in_irq_mode)
> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
>
> - dev_info(dev, "DMC initialized\n");
> + dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode);
>
> return 0;
>
>
Re: [PATCH v3 0/3] drivers: ide: use generic power management
Hi, On 7/13/20 7:36 PM, Vaibhav Gupta wrote: > Linux Kernel Mentee: Remove Legacy Power Management. > > The purpose of this patch series is to remove legacy power management > callbacks > from ide drivers. > > The suspend() and resume() callbacks operations are still invoking > pci_save/restore_state(), pci_set_power_state(), pci_enable/disable_state(), > etc. and handling the power management themselves, which is not recommended. > > The conversion requires the removal of the those function calls and change the > callback definition accordingly and make use of dev_pm_ops structure. IDE subsystem (drivers/ide/) is deprecated and has been superseded by libata subsystem (drivers/ata/). libata drivers have the same issue so please concentrate on fixing them first. Later (if desirable) changes can be back-ported to drivers/ide/. > All patches are compile-tested only. This patchset needs (at least) some basic testing. It should be easier with libata subsystem as it also support SATA controllers and devices. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > v3: > - Modpost error for undefined reference by Kbuild in v1. > - Another approach to disable PM in drivers/ide/triflex.c suggested by > Bjorn Helgaas in v2. > > Test tools: > - Compiler: gcc (GCC) 10.1.0 > - allmodconfig build: make -j$(nproc) W=1 all > > Vaibhav Gupta (3): > ide: use generic power management > ide: sc1200: use generic power management > ide: delkin_cb: use generic power management > > drivers/ide/aec62xx.c | 3 +-- > drivers/ide/alim15x3.c| 3 +-- > drivers/ide/amd74xx.c | 3 +-- > drivers/ide/atiixp.c | 3 +-- > drivers/ide/cmd64x.c | 3 +-- > drivers/ide/cs5520.c | 3 +-- > drivers/ide/cs5530.c | 3 +-- > drivers/ide/cs5535.c | 3 +-- > drivers/ide/cs5536.c | 3 +-- > drivers/ide/cy82c693.c| 3 +-- > drivers/ide/delkin_cb.c | 32 +- > drivers/ide/hpt366.c | 3 +-- > drivers/ide/ide-pci-generic.c | 3 +-- > drivers/ide/it8172.c | 3 +-- > drivers/ide/it8213.c | 3 +-- > drivers/ide/it821x.c | 3 +-- > drivers/ide/jmicron.c | 3 +-- > drivers/ide/ns87415.c | 3 +-- > drivers/ide/opti621.c | 3 +-- > drivers/ide/pdc202xx_new.c| 3 +-- > drivers/ide/pdc202xx_old.c| 3 +-- > drivers/ide/piix.c| 3 +-- > drivers/ide/sc1200.c | 43 --- > drivers/ide/serverworks.c | 3 +-- > drivers/ide/setup-pci.c | 29 +-- > drivers/ide/siimage.c | 3 +-- > drivers/ide/sis5513.c | 3 +-- > drivers/ide/sl82c105.c| 3 +-- > drivers/ide/slc90e66.c| 3 +-- > drivers/ide/triflex.c | 24 +++ > drivers/ide/via82cxxx.c | 3 +-- > include/linux/ide.h | 8 +-- > 32 files changed, 62 insertions(+), 155 deletions(-) >
Re: [PATCH] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.
[ Please Cc: fbdev Maintainer (happens to be me :) on fbdev patches, thanks. ]
Hi,
On 7/12/20 1:10 PM, Tetsuo Handa wrote:
> I found that
>
> const int fd = open("/dev/fb0", O_ACCMODE);
> struct fb_var_screeninfo var = { };
> ioctl(fd, FBIOGET_VSCREENINFO, );
> var.xres = var.yres = 1;
> ioctl(fd, FBIOPUT_VSCREENINFO, );
>
> causes general protection fault in bitfill_aligned(), for vc_do_resize()
> updates vc->vc_{cols,rows} only when vc_do_resize() will return 0.
>
> [ 20.10] BUG: unable to handle page fault for address: b80500d7b000
> [ 20.102225] #PF: supervisor write access in kernel mode
> [ 20.102226] #PF: error_code(0x0002) - not-present page
> [ 20.102227] PGD 13a48c067 P4D 13a48c067 PUD 13a48d067 PMD 132525067 PTE 0
> [ 20.102230] Oops: 0002 [#1] SMP
> [ 20.102232] CPU: 3 PID: 2786 Comm: a.out Not tainted 5.8.0-rc4+ #749
> [ 20.102233] Hardware name: VMware, Inc. VMware Virtual Platform/440BX
> Desktop Reference Platform, BIOS 6.00 02/27/2020
> [ 20.102237] RIP: 0010:bitfill_aligned+0x87/0x120 [cfbfillrect]
> [ 20.102277] Call Trace:
> [ 20.102281] cfb_fillrect+0x159/0x340 [cfbfillrect]
> [ 20.102747] vmw_fb_fillrect+0x12/0x30 [vmwgfx]
> [ 20.102755] bit_clear_margins+0x92/0xf0 [fb]
> [ 20.102760] fbcon_clear_margins+0x4c/0x50 [fb]
> [ 20.102763] fbcon_switch+0x321/0x570 [fb]
> [ 20.102771] redraw_screen+0xe0/0x250
> [ 20.102775] fbcon_modechanged+0x164/0x1b0 [fb]
> [ 20.102779] fbcon_update_vcs+0x15/0x20 [fb]
> [ 20.102781] fb_set_var+0x364/0x3c0 [fb]
> [ 20.102817] do_fb_ioctl+0x2ff/0x3f0 [fb]
> [ 20.103139] fb_ioctl+0x2e/0x40 [fb]
> [ 20.103141] ksys_ioctl+0x86/0xc0
> [ 20.103146] __x64_sys_ioctl+0x15/0x20
> [ 20.103148] do_syscall_64+0x54/0xa0
> [ 20.103151] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> If vc_do_resize() fails (e.g. kzalloc() failure) when var.xres or var.yres
> is going to shrink, bit_clear_margins() hits integer underflow bug due to
> info->var.xres < (vc->vc_cols * cw) or info->var.yres < (vc->vc_rows * ch).
> Unexpectedly large rw or bh will try to overrun the __iomem region and
> causes general protection fault.
>
> This crash is easily reproducible by calling vc_do_resize(vc, 0, 0)
> which the reproducer above will do. Since fbcon_modechanged() is doing
>
> cols = FBCON_SWAP(ops->rotate, info->var.xres, info->var.yres);
> rows = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres);
> cols /= vc->vc_font.width;
> rows /= vc->vc_font.height;
> vc_resize(vc, cols, rows);
> (...snipped...)
> update_screen(vc);
>
> , var.xres < vc->vc_font.width makes cols = 0 and var.yres <
> vc->vc_font.height
> makes rows = 0. But vc_do_resize() does not set vc->vc_cols = vc->vc_rows = 0
> due to
>
> new_cols = (cols ? cols : vc->vc_cols);
> new_rows = (lines ? lines : vc->vc_rows);
>
> exception.
>
> Of course, the root problem is that callers of do_vc_resize() are not
> handling vc_do_resize() failures, but it might not be easy to handle
> them under complicated dependency. Therefore, as a band-aid workaround,
> this patch checks integer underflow in "struct fbcon_ops"->clear_margins
> call, assuming that vc->vc_cols * vc->vc_font.width and
> vc->vc_rows * vc->vc_font.heigh do not cause integer overflow.
>
> I hope that we can survive even if info->var.{xres,yres} were increased
> but vc->vc_{cols,rows} were not increased due to kzalloc() failure, for
> the __iomem memory for cfb_fillrect() seems to be allocated upon driver
> load.
>
> By the way, syzbot has several reports which are stalling inside filling
> functions. Although reproducer for [1] is not found yet, it has tried
>
> r0 = openat$fb0(0xff9c, &(0x7f000180)='/dev/fb0\x00', 0x0,
> 0x0)
> ioctl$FBIOPUT_VSCREENINFO(r0, 0x4601, &(0x7f00)={0x0, 0x0, 0x0,
> 0x500, 0x0, 0x0, 0x4})
>
> which corresponds to
>
> const int fd = open("/dev/fb0", O_ACCMODE);
> struct fb_var_screeninfo var = { };
> var.yres_virtual = 0x500;
> var.bits_per_pixel = 4;
> ioctl(fd, FBIOPUT_VSCREENINFO, );
>
> and somehow hit unexpectedly long bit_clear_margins() loops. I don't know
> why syzbot does not hit general protection fault, but it would depend on
> environment because in my VMware environment ioctl(FBIOPUT_VSCREENINFO)
> returns -EINVAL if var.xres == var.yres == 0.
>
> [1]
> https://syzkaller.appspot.com/bug?id=91ecc3bf32ab1a551c33a39dab7fc0c8cd7b7e16
>
> Signed-off-by: Tetsuo Handa
How does this patch relate to:
https://marc.info/?l=linux-fbdev=159
Re: [PATCH] video: fbdev: vt8623fb: Constify static vga_regsets
On 7/1/20 11:02 PM, Rikard Falkeborn wrote:
> These are not modified so make them const to allow the compiler to put
> them in read-only memory.
>
> Before:
>textdata bss dec hex filename
> 255097928 64 3350182dd drivers/video/fbdev/vt8623fb.o
>
> After:
>textdata bss dec hex filename
> 265336904 64 3350182dd drivers/video/fbdev/vt8623fb.o
>
> Signed-off-by: Rikard Falkeborn
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/vt8623fb.c | 36 +-
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
> index 7b3eef1b893f..98ff8235c9e9 100644
> --- a/drivers/video/fbdev/vt8623fb.c
> +++ b/drivers/video/fbdev/vt8623fb.c
> @@ -62,24 +62,24 @@ static const struct svga_pll vt8623_pll = {2, 127, 2, 7,
> 0, 3,
>
> /* CRT timing register sets */
>
> -static struct vga_regset vt8623_h_total_regs[] = {{0x00, 0, 7}, {0x36,
> 3, 3}, VGA_REGSET_END};
> -static struct vga_regset vt8623_h_display_regs[] = {{0x01, 0, 7},
> VGA_REGSET_END};
> -static struct vga_regset vt8623_h_blank_start_regs[] = {{0x02, 0, 7},
> VGA_REGSET_END};
> -static struct vga_regset vt8623_h_blank_end_regs[] = {{0x03, 0, 4}, {0x05,
> 7, 7}, {0x33, 5, 5}, VGA_REGSET_END};
> -static struct vga_regset vt8623_h_sync_start_regs[] = {{0x04, 0, 7}, {0x33,
> 4, 4}, VGA_REGSET_END};
> -static struct vga_regset vt8623_h_sync_end_regs[]= {{0x05, 0, 4},
> VGA_REGSET_END};
> -
> -static struct vga_regset vt8623_v_total_regs[] = {{0x06, 0, 7}, {0x07,
> 0, 0}, {0x07, 5, 5}, {0x35, 0, 0}, VGA_REGSET_END};
> -static struct vga_regset vt8623_v_display_regs[] = {{0x12, 0, 7}, {0x07,
> 1, 1}, {0x07, 6, 6}, {0x35, 2, 2}, VGA_REGSET_END};
> -static struct vga_regset vt8623_v_blank_start_regs[] = {{0x15, 0, 7}, {0x07,
> 3, 3}, {0x09, 5, 5}, {0x35, 3, 3}, VGA_REGSET_END};
> -static struct vga_regset vt8623_v_blank_end_regs[] = {{0x16, 0, 7},
> VGA_REGSET_END};
> -static struct vga_regset vt8623_v_sync_start_regs[] = {{0x10, 0, 7}, {0x07,
> 2, 2}, {0x07, 7, 7}, {0x35, 1, 1}, VGA_REGSET_END};
> -static struct vga_regset vt8623_v_sync_end_regs[]= {{0x11, 0, 3},
> VGA_REGSET_END};
> -
> -static struct vga_regset vt8623_offset_regs[]= {{0x13, 0, 7}, {0x35,
> 5, 7}, VGA_REGSET_END};
> -static struct vga_regset vt8623_line_compare_regs[] = {{0x18, 0, 7}, {0x07,
> 4, 4}, {0x09, 6, 6}, {0x33, 0, 2}, {0x35, 4, 4}, VGA_REGSET_END};
> -static struct vga_regset vt8623_fetch_count_regs[] = {{0x1C, 0, 7}, {0x1D,
> 0, 1}, VGA_REGSET_END};
> -static struct vga_regset vt8623_start_address_regs[] = {{0x0d, 0, 7}, {0x0c,
> 0, 7}, {0x34, 0, 7}, {0x48, 0, 1}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_total_regs[] = {{0x00, 0, 7},
> {0x36, 3, 3}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_display_regs[] = {{0x01, 0, 7},
> VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_blank_start_regs[] = {{0x02, 0, 7},
> VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_blank_end_regs[] = {{0x03, 0, 4},
> {0x05, 7, 7}, {0x33, 5, 5}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_sync_start_regs[] = {{0x04, 0, 7},
> {0x33, 4, 4}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_h_sync_end_regs[]= {{0x05, 0, 4},
> VGA_REGSET_END};
> +
> +static const struct vga_regset vt8623_v_total_regs[] = {{0x06, 0, 7},
> {0x07, 0, 0}, {0x07, 5, 5}, {0x35, 0, 0}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_v_display_regs[] = {{0x12, 0, 7},
> {0x07, 1, 1}, {0x07, 6, 6}, {0x35, 2, 2}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_v_blank_start_regs[] = {{0x15, 0, 7},
> {0x07, 3, 3}, {0x09, 5, 5}, {0x35, 3, 3}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_v_blank_end_regs[] = {{0x16, 0, 7},
> VGA_REGSET_END};
> +static const struct vga_regset vt8623_v_sync_start_regs[] = {{0x10, 0, 7},
> {0x07, 2, 2}, {0x07, 7, 7}, {0x35, 1, 1}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_v_sync_end_regs[]= {{0x11, 0, 3},
> VGA_REGSET_END};
> +
> +static const struct vga_regset vt8623_offset_regs[]= {{0x13, 0, 7},
> {0x35, 5, 7}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_line_compare_regs[] = {{0x18, 0, 7},
> {0x07, 4, 4}, {0x09, 6, 6}, {0x33, 0, 2}, {0x35, 4, 4}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_fetch_count_regs[] = {{0x1C, 0, 7},
> {0x1D, 0, 1}, VGA_REGSET_END};
> +static const struct vga_regset vt8623_start_address_regs[] = {{0x0d, 0, 7},
> {0x0c, 0, 7}, {0x34, 0, 7}, {0x48, 0, 1}, VGA_REGSET_END};
>
> static const struct svga_timing_regs vt8623_timing_regs = {
> vt8623_h_total_regs, vt8623_h_display_regs, vt8623_h_blank_start_regs,
>
Re: [PATCH] fbdev: sm712fb: set error code in probe
On 7/6/20 5:53 PM, Evgeny Novikov wrote: > If smtcfb_pci_probe() does not detect a valid chip it cleans up > everything and returns 0. This can result in various bad things later. > The patch sets the error code on the corresponding path. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/sm712fb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c > index 6a1b4a853d9e..fbe97340b8e0 100644 > --- a/drivers/video/fbdev/sm712fb.c > +++ b/drivers/video/fbdev/sm712fb.c > @@ -1614,7 +1614,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev, > default: > dev_err(>dev, > "No valid Silicon Motion display chip was detected!\n"); > - > + err = -ENODEV; > goto failed_fb; > } > >
Re: [PATCH] video: fbdev: neofb: fix memory leak in neo_scan_monitor()
On 6/30/20 9:54 PM, Evgeny Novikov wrote: > neofb_probe() calls neo_scan_monitor() that can successfully allocate a > memory for info->monspecs.modedb and proceed to case 0x03. There it does > not free the memory and returns -1. neofb_probe() goes to label > err_scan_monitor, thus, it does not free this memory through calling > fb_destroy_modedb() as well. We can not go to label err_init_hw since > neo_scan_monitor() can fail during memory allocation. So, the patch frees > the memory directly for case 0x03. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/neofb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c > index f5a676bfd67a..09a20d4ab35f 100644 > --- a/drivers/video/fbdev/neofb.c > +++ b/drivers/video/fbdev/neofb.c > @@ -1819,6 +1819,7 @@ static int neo_scan_monitor(struct fb_info *info) > #else > printk(KERN_ERR > "neofb: Only 640x480, 800x600/480 and 1024x768 panels > are currently supported\n"); > + kfree(info->monspecs.modedb); > return -1; > #endif > default: >
Re: [PATCH] omapfb: fix multiple reference count leaks due to pm_runtime_get_sync
On 6/14/20 5:05 AM, Aditya Pakki wrote:
> On calling pm_runtime_get_sync() the reference count of the device
> is incremented. In case of failure, decrement the
> reference count before returning the error.
>
> Signed-off-by: Aditya Pakki
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 7 +--
> drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 7 +--
> drivers/video/fbdev/omap2/omapfb/dss/dss.c | 7 +--
> drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 5 +++--
> drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c | 5 +++--
> drivers/video/fbdev/omap2/omapfb/dss/venc.c | 7 +--
> 6 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> index 4a16798b2ecd..e2b572761bf6 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -520,8 +520,11 @@ int dispc_runtime_get(void)
> DSSDBG("dispc_runtime_get\n");
>
> r = pm_runtime_get_sync(>dev);
> - WARN_ON(r < 0);
> - return r < 0 ? r : 0;
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>dev);
> + return r;
> + }
> + return 0;
> }
> EXPORT_SYMBOL(dispc_runtime_get);
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> index d620376216e1..6f9c25fec994 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> @@ -1137,8 +1137,11 @@ static int dsi_runtime_get(struct platform_device
> *dsidev)
> DSSDBG("dsi_runtime_get\n");
>
> r = pm_runtime_get_sync(>pdev->dev);
> - WARN_ON(r < 0);
> - return r < 0 ? r : 0;
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>pdev->dev);
> + return r;
> + }
> + return 0;
> }
>
> static void dsi_runtime_put(struct platform_device *dsidev)
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> index 7252d22dd117..3586579c838f 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> @@ -768,8 +768,11 @@ int dss_runtime_get(void)
> DSSDBG("dss_runtime_get\n");
>
> r = pm_runtime_get_sync(>dev);
> - WARN_ON(r < 0);
> - return r < 0 ? r : 0;
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>dev);
> + return r;
> + }
> + return 0;
> }
>
> void dss_runtime_put(void)
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> index 7060ae56c062..4804aab34298 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> @@ -39,9 +39,10 @@ static int hdmi_runtime_get(void)
> DSSDBG("hdmi_runtime_get\n");
>
> r = pm_runtime_get_sync(>dev);
> - WARN_ON(r < 0);
> - if (r < 0)
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>dev);
> return r;
> + }
>
> return 0;
> }
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
> b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
> index ac49531e4732..a06b6f1355bd 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c
> @@ -43,9 +43,10 @@ static int hdmi_runtime_get(void)
> DSSDBG("hdmi_runtime_get\n");
>
> r = pm_runtime_get_sync(>dev);
> - WARN_ON(r < 0);
> - if (r < 0)
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>dev);
> return r;
> + }
>
> return 0;
> }
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c
> b/drivers/video/fbdev/omap2/omapfb/dss/venc.c
> index d5404d56c922..0b0ad20afd63 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c
> @@ -348,8 +348,11 @@ static int venc_runtime_get(void)
> DSSDBG("venc_runtime_get\n");
>
> r = pm_runtime_get_sync(>dev);
> - WARN_ON(r < 0);
> - return r < 0 ? r : 0;
> + if (WARN_ON(r < 0)) {
> + pm_runtime_put_sync(>dev);
> + return r;
> + }
> + return 0;
> }
>
> static void venc_runtime_put(void)
>
Re: [PATCH] fbdev: da8xx-fb: go to proper label on error handling paths in probe
On 7/2/20 6:05 PM, Evgeny Novikov wrote:
> fb_probe() can successfully allocate a new frame buffer, but then fail
> to perform some operations with regulator. In these cases fb_probe()
> goes to label err_pm_runtime_disable where the frame buffer is not
> released. The patch makes fb_probe() to go to label err_release_fb on
> corresponding error handling paths.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Evgeny Novikov
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/da8xx-fb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
> index 73c3c4c8cc12..e38c0e3f9c61 100644
> --- a/drivers/video/fbdev/da8xx-fb.c
> +++ b/drivers/video/fbdev/da8xx-fb.c
> @@ -1402,14 +1402,14 @@ static int fb_probe(struct platform_device *device)
> if (IS_ERR(par->lcd_supply)) {
> if (PTR_ERR(par->lcd_supply) == -EPROBE_DEFER) {
> ret = -EPROBE_DEFER;
> - goto err_pm_runtime_disable;
> + goto err_release_fb;
> }
>
> par->lcd_supply = NULL;
> } else {
> ret = regulator_enable(par->lcd_supply);
> if (ret)
> - goto err_pm_runtime_disable;
> + goto err_release_fb;
> }
>
> fb_videomode_to_var(_fb_var, lcdc_info);
>
Re: [PATCH] omapfb: dss: Fix max fclk divider for omap36xx
On 6/30/20 8:26 PM, Adam Ford wrote:
> The drm/omap driver was fixed to correct an issue where using a
> divider of 32 breaks the DSS despite the TRM stating 32 is a valid
> number. Through experimentation, it appears that 31 works, and
> it is consistent with the value used by the drm/omap driver.
>
> This patch fixes the divider for fbdev driver instead of the drm.
>
> Fixes: f76ee892a99e ("omapfb: copy omapdss & displays for omapfb")
>
> Cc: #4.9+
> Signed-off-by: Adam Ford
Applied to drm-misc-next tree, thanks.
(I marked patch as applicable to stable 4.5+ while merging)
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> Linux 4.4 will need a similar patch, but it doesn't apply cleanly.
>
> The DRM version of this same fix is:
> e2c4ed148cf3 ("drm/omap: fix max fclk divider for omap36xx")
>
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> index 7252d22dd117..bfc5c4c5a26a 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c
> @@ -833,7 +833,7 @@ static const struct dss_features omap34xx_dss_feats = {
> };
>
> static const struct dss_features omap3630_dss_feats = {
> - .fck_div_max= 32,
> + .fck_div_max= 31,
> .dss_fck_multiplier = 1,
> .parent_clk_name= "dpll4_ck",
> .dpi_select_source = _dpi_select_source_omap2_omap3,
>
Re: [PATCH] video: fbdev: savage: fix memory leak on error handling path in probe
On 6/19/20 6:21 PM, Evgeny Novikov wrote: > savagefb_probe() calls savage_init_fb_info() that can successfully > allocate memory for info->pixmap.addr but then fail when > fb_alloc_cmap() fails. savagefb_probe() goes to label failed_init and > does not free allocated memory. It is not valid to go to label > failed_mmio since savage_init_fb_info() can fail during memory > allocation as well. So, the patch free allocated memory on the error > handling path in savage_init_fb_info() itself. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/savage/savagefb_driver.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/video/fbdev/savage/savagefb_driver.c > b/drivers/video/fbdev/savage/savagefb_driver.c > index 3c8ae87f0ea7..3fd87aeb6c79 100644 > --- a/drivers/video/fbdev/savage/savagefb_driver.c > +++ b/drivers/video/fbdev/savage/savagefb_driver.c > @@ -2157,6 +2157,8 @@ static int savage_init_fb_info(struct fb_info *info, > struct pci_dev *dev, > info->flags |= FBINFO_HWACCEL_COPYAREA | > FBINFO_HWACCEL_FILLRECT | > FBINFO_HWACCEL_IMAGEBLIT; > + else > + kfree(info->pixmap.addr); > } > #endif > return err; >
Re: [PATCH][next] fbdev/fb.h: Use struct_size() helper in kzalloc()
On 6/20/20 1:27 PM, Sam Ravnborg wrote:
> Hi Gustavo.
>
> On Wed, Jun 17, 2020 at 12:56:47PM -0500, Gustavo A. R. Silva wrote:
>> Make use of the struct_size() helper instead of an open-coded version
>> in order to avoid any potential type mistakes.
>>
>> This code was detected with the help of Coccinelle and, audited and
>> fixed manually.
>>
>> Signed-off-by: Gustavo A. R. Silva
>
> struct_size is defined in overflow.h - which is not included by fs.h.
> So we rely on overflow.h being pulled in by some other header - maybe
> slab.h in this case.
> Seems fragile, should this patch add an include of overflow.h?
$ git grep struct_size drivers/|wc -l
697
$ git grep overflow\\.h drivers/|wc -l
8
$ git grep overflow\\.h include/linux/
include/linux/device.h:#include
include/linux/mm.h:#include
include/linux/slab.h:#include
include/linux/vmalloc.h:#include
so I've applied the patch as it is (hoping that the issue is so
widespread that no-one tries to remove overflow.h from slab.h
without fixing drivers at the same time)..
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Sam
>
>> ---
>> include/linux/fb.h | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> index 3b4b2f0c6994..2b530e6d86e4 100644
>> --- a/include/linux/fb.h
>> +++ b/include/linux/fb.h
>> @@ -506,8 +506,9 @@ struct fb_info {
>> };
>>
>> static inline struct apertures_struct *alloc_apertures(unsigned int
>> max_num) {
>> -struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
>> -+ max_num * sizeof(struct aperture), GFP_KERNEL);
>> +struct apertures_struct *a;
>> +
>> +a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
>> if (!a)
>> return NULL;
>> a->count = max_num;
>> --
>> 2.27.0
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://protect2.fireeye.com/url?k=7bae4d09-26604cda-7bafc646-000babff317b-7eab3a2caa4b8b73=1=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
Re: [PATCH][next] fbdev/fb.h: Use struct_size() helper in kzalloc()
On 6/17/20 7:56 PM, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes.
>
> This code was detected with the help of Coccinelle and, audited and
> fixed manually.
>
> Signed-off-by: Gustavo A. R. Silva
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> include/linux/fb.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 3b4b2f0c6994..2b530e6d86e4 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -506,8 +506,9 @@ struct fb_info {
> };
>
> static inline struct apertures_struct *alloc_apertures(unsigned int max_num)
> {
> - struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
> - + max_num * sizeof(struct aperture), GFP_KERNEL);
> + struct apertures_struct *a;
> +
> + a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
> if (!a)
> return NULL;
> a->count = max_num;
>
Re: [PATCH v2 2/2] video: fbdev: amifb: add FIXMEs about {put,get}_user() failures
On 6/2/20 2:03 PM, Geert Uytterhoeven wrote: > On Tue, Jun 2, 2020 at 1:52 PM Bartlomiej Zolnierkiewicz > wrote: >> Since we lack the hardware (or proper emulator setup) for >> testing needed changes add FIXMEs to document the issues >> (so at least they are not forgotten). >> >> Cc: Al Viro >> Cc: Geert Uytterhoeven >> Signed-off-by: Bartlomiej Zolnierkiewicz > > Reviewed-by: Geert Uytterhoeven Applied to drm-misc-next tree. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH][next] fbcon: Use array3_size() helper in scr_memcpyw()
On 6/16/20 1:15 AM, Gustavo A. R. Silva wrote:
> Use array3_size() helper instead of the open-coded version in scr_memcpyw()
> and scr_memsetw(). These sorts of multiplication factors need to be wrapped
> in array3_size().
>
> This issue was found with the help of Coccinelle and, audited and fixed
> manually.
>
> Addresses-KSPP-ID: https://github.com/KSPP/linux/issues/83
> Signed-off-by: Gustavo A. R. Silva
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/core/fbcon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c
> b/drivers/video/fbdev/core/fbcon.c
> index 9d28a8e3328f..6af2734f2a7b 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -639,7 +639,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct
> fb_info *info,
> GFP_KERNEL);
> if (save) {
> int i = cols < new_cols ? cols : new_cols;
> - scr_memsetw(save, erase, logo_lines * new_cols * 2);
> + scr_memsetw(save, erase, array3_size(logo_lines,
> new_cols, 2));
> r = q - step;
> for (cnt = 0; cnt < logo_lines; cnt++, r += i)
> scr_memcpyw(save + cnt * new_cols, r, 2 * i);
> @@ -676,7 +676,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct
> fb_info *info,
> q = (unsigned short *) (vc->vc_origin +
> vc->vc_size_row *
> rows);
> - scr_memcpyw(q, save, logo_lines * new_cols * 2);
> + scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2));
> vc->vc_y += logo_lines;
> vc->vc_pos += logo_lines * vc->vc_size_row;
> kfree(save);
>
Re: [PATCH v3] video: fbdev: ssd1307fb: Added support to Column offset
[ added dri-devel ML to Cc: ]
Hi,
Sorry for the delay.
On 5/13/20 8:48 PM, Rodrigo Alencar wrote:
> From: Rodrigo Rolim Mendes de Alencar
>
> This patch provides support for displays like VGM128064B0W10,
> which requires a column offset of 2, i.e., its segments starts
> in SEG2 and ends in SEG129.
>
> Signed-off-by: Rodrigo Alencar <455.rodrigo.alen...@gmail.com>
Please resend with "From:" & "Signed-off-by:" tags fixed to match so
I can merge the patch.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 +
> drivers/video/fbdev/ssd1307fb.c | 8 ++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt
> b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> index 27333b9551b3..2dcb6d12d137 100644
> --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
> +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
> @@ -19,6 +19,7 @@ Optional properties:
>- vbat-supply: The supply for VBAT
>- solomon,segment-no-remap: Display needs normal (non-inverted) data column
>to segment mapping
> + - solomon,col-offset: Offset of columns (COL/SEG) that the screen is
> mapped to.
>- solomon,com-seq: Display uses sequential COM pin configuration
>- solomon,com-lrremap: Display uses left-right COM pin remap
>- solomon,com-invdir: Display uses inverted COM pin scan direction
> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 8e06ba912d60..102f941104c0 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -74,6 +74,7 @@ struct ssd1307fb_par {
> struct fb_info *info;
> u8 lookup_table[4];
> u32 page_offset;
> + u32 col_offset;
> u32 prechargep1;
> u32 prechargep2;
> struct pwm_device *pwm;
> @@ -458,11 +459,11 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
> if (ret < 0)
> return ret;
>
> - ret = ssd1307fb_write_cmd(par->client, 0x0);
> + ret = ssd1307fb_write_cmd(par->client, par->col_offset);
> if (ret < 0)
> return ret;
>
> - ret = ssd1307fb_write_cmd(par->client, par->width - 1);
> + ret = ssd1307fb_write_cmd(par->client, par->col_offset + par->width -
> 1);
> if (ret < 0)
> return ret;
>
> @@ -626,6 +627,9 @@ static int ssd1307fb_probe(struct i2c_client *client)
> if (device_property_read_u32(dev, "solomon,page-offset",
> >page_offset))
> par->page_offset = 1;
>
> + if (device_property_read_u32(dev, "solomon,col-offset",
> >col_offset))
> + par->col_offset = 0;
> +
> if (device_property_read_u32(dev, "solomon,com-offset",
> >com_offset))
> par->com_offset = 0;
>
>
Re: [PATCH v2 1/2] video: fbdev: amifb: add FIXME about dead APUS support
On 6/2/20 2:03 PM, Geert Uytterhoeven wrote: > On Tue, Jun 2, 2020 at 1:50 PM Bartlomiej Zolnierkiewicz > wrote: >> On 5/14/20 10:21 PM, Geert Uytterhoeven wrote: >>> These #ifdefs are relics from APUS (Amiga Power-Up System), which >>> added a PPC board. APUS support was killed off a long time ago, >>> when arch/ppc/ was still king, but these #ifdefs were missed, because >>> they didn't test for CONFIG_APUS. >> >> Add FIXME about using the C code variants (APUS ones) in the future. >> >> Reported-by: Al Viro >> Reported-by: Geert Uytterhoeven >> Signed-off-by: Bartlomiej Zolnierkiewicz > > Reviewed-by: Geert Uytterhoeven Applied to drm-misc-next tree. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
On 7/10/20 2:56 PM, Lukasz Luba wrote: > > > On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote: >> On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote: >>> Hi Chanwoo, >>> >>> On 7/9/20 5:08 AM, Chanwoo Choi wrote: >>>> Hi Lukasz, >>>> >>>> On 7/9/20 12:34 AM, Lukasz Luba wrote: >>>>> In order to react faster and make better decisions under some workloads, >>>>> benchmarking the memory subsystem behavior, adjust the polling interval >>>>> and upthreshold value used by the simple_ondemand governor. >>>>> >>>>> Reported-by: Willy Wolff >>>>> Signed-off-by: Lukasz Luba >>>>> --- >>>>> drivers/memory/samsung/exynos5422-dmc.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/memory/samsung/exynos5422-dmc.c >>>>> b/drivers/memory/samsung/exynos5422-dmc.c >>>>> index 93e9c2429c0d..e03ee35f0ab5 100644 >>>>> --- a/drivers/memory/samsung/exynos5422-dmc.c >>>>> +++ b/drivers/memory/samsung/exynos5422-dmc.c >>>>> @@ -1466,10 +1466,10 @@ static int exynos5_dmc_probe(struct >>>>> platform_device *pdev) >>>>> * Setup default thresholds for the devfreq governor. >>>>> * The values are chosen based on experiments. >>>>> */ >>>>> - dmc->gov_data.upthreshold = 30; >>>>> + dmc->gov_data.upthreshold = 10; >>>>> dmc->gov_data.downdifferential = 5; >>>>> - exynos5_dmc_df_profile.polling_ms = 500; >>>>> + exynos5_dmc_df_profile.polling_ms = 100; >>>>> } >>>>> >>>> >>>> Reviewed-by: Chanwoo Choi >>>> >>> >>> Thank you for the review. Do you think this patch could go through >>> your tree together with your patches? >>> >>> I don't know Krzysztof's opinion about the patch 2/2, but >>> I would expect, assuming the patch itself is correct, he would >>> like to take it into his next/dt branch. >> >> In the cover letter you mentioned that this is a follow up for the >> Chanwoo's patchset. But are these patches really depending on it? Can >> they be picked up independently? > > > They are not heavily dependent on Chanwoo's patches. > Yes, they can be picked up independently. Hmmm, are you sure? Sure, they will apply fine but without Chanwoo's patches won't they cause the dmc driver to use using polling mode with deferred timer (unintended/bad behavior) instead of IRQs (current behavior) or polling mode with delayed timer (future behavior)? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > I just wanted to mention that the patch 1/2 was produced on the > code base which had already applied Chanwoo's patch for DMC. > If you like to take both 1/2 and 2/2 into your tree, it's good. > > Thank you for having a look on this. > > Regards, > Lukasz > > >> >> The DTS patch must go through arm soc, so I will take it. If it really >> depends on driver changes, then it has to wait for next release. >> >> Best regards, >> Krzysztof >>
Re: [PATCH] vgacon: fix a UAF in do_update_region()
Hi, Please re-send adding the correct mailing lists: * dri-de...@lists.freedesktop.org * linux-fb...@vger.kernel.org to Cc: so the patch can be reviewed/merged. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics On 6/4/20 4:38 PM, Yang Yingliang wrote: > I got a UAF report in do_update_region() when I doing fuzz test. > > [ 51.161905] BUG: KASAN: use-after-free in do_update_region+0x579/0x600 > [ 51.161918] Read of size 2 at addr 88800010 by task test/295 > > [ 51.161957] CPU: 2 PID: 295 Comm: test Not tainted 5.7.0+ #975 > [ 51.161969] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 > [ 51.161976] Call Trace: > [ 51.162001] dump_stack+0xc6/0x11e > [ 51.162019] ? do_update_region+0x579/0x600 > [ 51.162047] print_address_description.constprop.6+0x1a/0x220 > [ 51.162083] ? vprintk_func+0x66/0xed > [ 51.162100] ? do_update_region+0x579/0x600 > [ 51.162112] ? do_update_region+0x579/0x600 > [ 51.162128] kasan_report.cold.9+0x37/0x7c > [ 51.162151] ? do_update_region+0x579/0x600 > [ 51.162173] do_update_region+0x579/0x600 > [ 51.162207] ? con_get_trans_old+0x230/0x230 > [ 51.162229] ? retint_kernel+0x10/0x10 > [ 51.162278] csi_J+0x557/0xa00 > [ 51.162307] do_con_trol+0x49af/0x5cc0 > [ 51.162330] ? lock_downgrade+0x720/0x720 > [ 51.162347] ? reset_palette+0x1b0/0x1b0 > [ 51.162369] ? lockdep_hardirqs_on_prepare+0x379/0x540 > [ 51.162393] ? notifier_call_chain+0x11b/0x160 > [ 51.162438] do_con_write.part.24+0xb0a/0x1a30 > [ 51.162501] ? do_con_trol+0x5cc0/0x5cc0 > [ 51.162522] ? console_unlock+0x7b8/0xb00 > [ 51.162555] ? __mutex_unlock_slowpath+0xd4/0x670 > [ 51.162574] ? this_tty+0xe0/0xe0 > [ 51.162589] ? console_unlock+0x559/0xb00 > [ 51.162605] ? wait_for_completion+0x260/0x260 > [ 51.162638] con_write+0x31/0xb0 > [ 51.162658] n_tty_write+0x4fa/0xd40 > [ 51.162710] ? n_tty_read+0x1800/0x1800 > [ 51.162730] ? prepare_to_wait_exclusive+0x270/0x270 > [ 51.162754] ? __might_fault+0x175/0x1b0 > [ 51.162783] tty_write+0x42b/0x8d0 > [ 51.162795] ? n_tty_read+0x1800/0x1800 > [ 51.162825] ? tty_lookup_driver+0x450/0x450 > [ 51.162848] __vfs_write+0x7c/0x100 > [ 51.162875] vfs_write+0x1c9/0x510 > [ 51.162901] ksys_write+0xff/0x200 > [ 51.162918] ? __ia32_sys_read+0xb0/0xb0 > [ 51.162940] ? do_syscall_64+0x1a/0x520 > [ 51.162957] ? lockdep_hardirqs_on_prepare+0x379/0x540 > [ 51.162984] do_syscall_64+0xa1/0x520 > [ 51.163008] entry_SYSCALL_64_after_hwframe+0x49/0xb3 > > After vgacon_set_origin() is called in set_origin(), the vc_origin is > set to vga_vram_base, the vc_pos should between vga_vram_base and > vga_vram_end. But we still use vc_screenbuf_size, if the vga_vram_size > is smaller than vc_screenbuf_size, vc_pos may be out of bound, using it > will cause a use-after-free(or out-of-bounds). Fix this by calling > vc_resize() if vga_vram_size is smaller than vc_screenbuf_size. > > Signed-off-by: Yang Yingliang > --- > drivers/video/console/vgacon.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c > index 998b0de..2ee3d62 100644 > --- a/drivers/video/console/vgacon.c > +++ b/drivers/video/console/vgacon.c > @@ -1336,6 +1336,9 @@ static int vgacon_set_origin(struct vc_data *c) > if (vga_is_gfx || /* We don't play origin tricks in graphic modes > */ > (console_blanked && !vga_palette_blanked)) /* Nor we write to > blanked screens */ > return 0; > + > + if (c->vc_screenbuf_size > vga_vram_size) > + vc_resize(c, screen_info.orig_video_cols, > screen_info.orig_video_lines); > c->vc_origin = c->vc_visible_origin = vga_vram_base; > vga_set_mem_top(c); > vga_rolled_over = 0; >
Re: [PATCH -next] vgacon: Fix an out-of-bounds in vgacon_scrollback_update()
Hi, Please re-send adding the correct mailing lists: * dri-de...@lists.freedesktop.org * linux-fb...@vger.kernel.org to Cc: so the patch can be reviewed/merged. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics On 5/13/20 4:28 AM, Yang Yingliang wrote: > I got a slab-out-of-bounds report when I doing fuzz test. > > [ 334.989515] > == > [ 334.989577] BUG: KASAN: slab-out-of-bounds in vgacon_scroll+0x57a/0x8ed > [ 334.989588] Write of size 1766 at addr 8883de69ff3e by task test/2658 > [ 334.989593] > [ 334.989608] CPU: 3 PID: 2658 Comm: test Not tainted > 5.7.0-rc5-5-g152036d1379f #789 > [ 334.989617] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > [ 334.989624] Call Trace: > [ 334.989646] dump_stack+0xe4/0x14e > [ 334.989676] print_address_description.constprop.5+0x3f/0x60 > [ 334.989699] ? vgacon_scroll+0x57a/0x8ed > [ 334.989710] __kasan_report.cold.8+0x92/0xaf > [ 334.989735] ? vgacon_scroll+0x57a/0x8ed > [ 334.989761] kasan_report+0x37/0x50 > [ 334.989789] check_memory_region+0x1c1/0x1e0 > [ 334.989806] memcpy+0x38/0x60 > [ 334.989824] vgacon_scroll+0x57a/0x8ed > [ 334.989876] con_scroll+0x4ef/0x5e0 > [ 334.989904] ? lockdep_hardirqs_on+0x5e0/0x5e0 > [ 334.989934] lf+0x24f/0x2a0 > [ 334.989951] ? con_scroll+0x5e0/0x5e0 > [ 334.989975] ? find_held_lock+0x33/0x1c0 > [ 334.990005] do_con_trol+0x313/0x5ff0 > [ 334.990027] ? lock_downgrade+0x730/0x730 > [ 334.990045] ? reset_palette+0x440/0x440 > [ 334.990070] ? _raw_spin_unlock_irqrestore+0x4b/0x60 > [ 334.990095] ? notifier_call_chain+0x120/0x170 > [ 334.990132] ? __atomic_notifier_call_chain+0xf0/0x180 > [ 334.990160] do_con_write.part.16+0xb2b/0x1b20 > [ 334.990238] ? do_con_trol+0x5ff0/0x5ff0 > [ 334.990258] ? mutex_lock_io_nested+0x1280/0x1280 > [ 334.990269] ? rcu_read_unlock+0x50/0x50 > [ 334.990315] ? __mutex_unlock_slowpath+0xd9/0x670 > [ 334.990340] ? lockdep_hardirqs_on+0x3a2/0x5e0 > [ 334.990368] con_write+0x36/0xc0 > [ 334.990389] do_output_char+0x561/0x780 > [ 334.990414] n_tty_write+0x58e/0xd30 > [ 334.990478] ? n_tty_read+0x1800/0x1800 > [ 334.990500] ? prepare_to_wait_exclusive+0x300/0x300 > [ 334.990525] ? __might_fault+0x17a/0x1c0 > [ 334.990557] tty_write+0x430/0x960 > [ 334.990568] ? n_tty_read+0x1800/0x1800 > [ 334.990600] ? tty_release+0x1280/0x1280 > [ 334.990622] __vfs_write+0x81/0x100 > [ 334.990648] vfs_write+0x1ce/0x510 > [ 334.990676] ksys_write+0x104/0x200 > [ 334.990691] ? __ia32_sys_read+0xb0/0xb0 > [ 334.990708] ? trace_hardirqs_on_thunk+0x1a/0x1c > [ 334.990725] ? trace_hardirqs_off_caller+0x40/0x1a0 > [ 334.990744] ? do_syscall_64+0x3b/0x5e0 > [ 334.990775] do_syscall_64+0xc8/0x5e0 > [ 334.990798] entry_SYSCALL_64_after_hwframe+0x49/0xb3 > [ 334.990811] RIP: 0033:0x44f369 > [ 334.990827] Code: 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 > f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48 > [ 334.990834] RSP: 002b:7ffe9ace0968 EFLAGS: 0246 ORIG_RAX: > 0001 > [ 334.990848] RAX: ffda RBX: 00400418 RCX: > 0044f369 > [ 334.990856] RDX: 0381 RSI: 20003500 RDI: > 0003 > [ 334.990865] RBP: 7ffe9ace0980 R08: 20003530 R09: > 7ffe9ace0980 > [ 334.990873] R10: 0001 R11: 0246 R12: > 00402110 > [ 334.990881] R13: R14: 006bf018 R15: > > [ 334.990937] > [ 334.990943] The buggy address belongs to the page: > [ 334.990962] page:ea000f79a400 refcount:1 mapcount:0 > mapping:2bff47b3 index:0x0 head:ea000f79a400 order:4 > compound_mapcount:0 compound_pincount:0 > [ 334.990973] flags: 0x2f8001(head) > [ 334.990992] raw: 002f8001 dead0100 dead0122 > > [ 334.991006] raw: 0001 > > [ 334.991013] page dumped because: kasan: bad access detected > [ 334.991017] > [ 334.991023] Memory state around the buggy address: > [ 334.991034] 8883de6a: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 > [ 334.991044] 8883de6a0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 > [ 334.991054] >8883de6a0100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > fc fc > [ 334.991061] ^ > [ 334.9910
Re: [PATCH] efi: avoid error message when booting under Xen
[ added EFI Maintainer & ML to Cc: ]
Hi,
On 7/9/20 11:17 AM, Jürgen Groß wrote:
> On 28.06.20 10:50, Jürgen Groß wrote:
>> Ping?
>>
>> On 10.06.20 16:10, Juergen Gross wrote:
>>> efifb_probe() will issue an error message in case the kernel is booted
>>> as Xen dom0 from UEFI as EFI_MEMMAP won't be set in this case. Avoid
>>> that message by calling efi_mem_desc_lookup() only if EFI_PARAVIRT
>>> isn't set.
>>>
>>> Fixes: 38ac0287b7f4 ("fbdev/efifb: Honour UEFI memory map attributes when
>>> mapping the FB")
>>> Signed-off-by: Juergen Gross
>>> ---
>>> drivers/video/fbdev/efifb.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
>>> index 65491ae74808..f5eccd1373e9 100644
>>> --- a/drivers/video/fbdev/efifb.c
>>> +++ b/drivers/video/fbdev/efifb.c
>>> @@ -453,7 +453,7 @@ static int efifb_probe(struct platform_device *dev)
>>> info->apertures->ranges[0].base = efifb_fix.smem_start;
>>> info->apertures->ranges[0].size = size_remap;
>>> - if (efi_enabled(EFI_BOOT) &&
>>> + if (efi_enabled(EFI_BOOT) && !efi_enabled(EFI_PARAVIRT) &&
>>> !efi_mem_desc_lookup(efifb_fix.smem_start, )) {
>>> if ((efifb_fix.smem_start + efifb_fix.smem_len) >
>>> (md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT))) {
>>>
>>
>
> In case I see no reaction from the maintainer for another week I'll take
> this patch through the Xen tree.
>From fbdev POV this change looks fine to me and I'm OK with merging it
through Xen or EFI tree:
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [RFC PATCH 0/2] PM / devfreq: Add delayed timer for polling
Hi Chanwoo, On 7/3/20 8:26 AM, Chanwoo Choi wrote: > Add the delayed timer to devfreq framework in order to support > the periodical polling mode without stop caused by CPU idle state. Thank you, this patchset looks fine to me and is a step in the right direction: Reviewed-by: Bartlomiej Zolnierkiewicz > Some Non-CPU device must need to monitor the device status like > utilization regardless of CPU state. This is probably true for all devfreq devices using simple_ondemand governor by default: drivers/devfreq/exynos-bus.c drivers/devfreq/rk3399_dmc.c drivers/devfreq/tegra20-devfreq.c drivers/gpu/drm/lima/lima_devfreq.c drivers/gpu/drm/msm/msm_gpu.c drivers/gpu/drm/panfrost/panfrost_devfreq.c drivers/memory/samsung/exynos5422-dmc.c drivers/scsi/ufs/ufshcd.c With devfreq device polling being "coupled" to CPU idle state the devfreq subsystem behavior is completely unpredictable and unreliable. It affects both performance (device opp change up happening too late) and power consumption (device opp change down happening too late). It also causes hardware usage counters support to report too high values (because of CPU idle "coupling" the real polling period becomes larger than maximum period supported by the counter and the counter becomes fully "saturated") which negatively affects power consumption (as has been observed when using Odroid XU3/4). [ The only upside of using such "coupling" is lowered CPU power usage (in some situations) but at the (unacceptable IMHO) cost of the correctness of operations of devfreq subsystem. ] Unfortunately this patchset currently fixes only exynos5422-dmc devfreq driver. To fix problems for Exynos platforms we need to also fix exynos-bus devfreq driver. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > - patch1 explains the detailed reason why the delayed timer is required. > - patch2 initializes that exynos5422-dmc device use delayed timer as default > instead of deferrable timer. > > Chanwoo Choi (2): > PM / devfreq: Add support delayed timer for polling mode > memory: samsung: exynos5422-dmc: Use delayed timer as default > > Documentation/ABI/testing/sysfs-class-devfreq | 12 +++ > drivers/devfreq/devfreq.c | 83 ++- > drivers/memory/samsung/exynos5422-dmc.c | 1 + > include/linux/devfreq.h | 9 ++ > 4 files changed, 104 insertions(+), 1 deletion(-)
Re: brocken devfreq simple_ondemand for Odroid XU3/4?
to monitor the devices (or stop if they are suspended). >> That should work nicely with the governors, which try to predict the >> next best frequency. From my experience the more fluctuating intervals >> the governors are called, the more odd decisions they make. >> That's why I think having a predictable interval i.e. 100ms is something >> desirable. Tuning the governors is easier in this case, statistics >> are easier to trace and interpret, solution is not to platform specific, >> etc. >> >> Kamil do you have plans to refresh and push your next version of the >> workqueue solution? > > I do not, as Bartek takes over my work, > +CC Bartek Hi Lukasz, As you remember in January Chanwoo has proposed another idea (to allow selecting workqueue type by devfreq device driver): "I'm developing the RFC patch and then I'll send it as soon as possible." (https://lore.kernel.org/linux-pm/6107fa2b-81ad-060d-89a2-d8941ac4d...@samsung.com/) "After posting my suggestion, we can discuss it" (https://lore.kernel.org/linux-pm/f5c5cd64-b72c-2802-f6ea-ab3d28483...@samsung.com/) so we have been waiting on the patch to be posted.. Similarly we have been waiting on (any) feedback for exynos-bus/nocp fixes for Exynos5422 support (which have been posted by Kamil also in January): https://lore.kernel.org/linux-pm/8f82d8d5-927b-afb4-272f-45c16b5a2...@samsung.com/ Considering the above and how hard it has been to push the changes through review/merge process last year we are near giving up when it comes to upstream devfreq contributions. Sylwester is still working on exynos-bus & interconnect integration (continuation of Artur Swigon's work from last year) & related issues (IRQ support for PPMU) but I'm seriously considering putting it all on-hold.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: include/linux/libata.h:1366:2: note: in expansion of macro '__ATA_BASE_SHT'
platform.c:40:2: note: in expansion of macro ‘AHCI_SHT’
AHCI_SHT(DRV_NAME),
^
CC drivers/ata/libahci.o
CC drivers/ata/libahci_platform.o
CC drivers/ata/sata_dwc_460ex.o
CC drivers/ata/ahci_ceva.o
In file included from drivers/ata/ahci_ceva.c:15:0:
drivers/ata/ahci.h:384:16: warning: initialized field overwritten
[-Woverride-init]
.can_queue = AHCI_MAX_CMDS, \
^
drivers/ata/ahci_ceva.c:187:2: note: in expansion of macro ‘AHCI_SHT’
AHCI_SHT(DRV_NAME),
^
drivers/ata/ahci.h:384:16: note: (near initialization for
‘ahci_platform_sht.can_queue’)
.can_queue = AHCI_MAX_CMDS, \
^
drivers/ata/ahci_ceva.c:187:2: note: in expansion of macro ‘AHCI_SHT’
AHCI_SHT(DRV_NAME),
^
drivers/ata/ahci.h:388:17: warning: initialized field overwritten
[-Woverride-init]
.sdev_attrs = ahci_sdev_attrs
^
drivers/ata/ahci_ceva.c:187:2: note: in expansion of macro ‘AHCI_SHT’
AHCI_SHT(DRV_NAME),
^
drivers/ata/ahci.h:388:17: note: (near initialization for
‘ahci_platform_sht.sdev_attrs’)
.sdev_attrs = ahci_sdev_attrs
^
drivers/ata/ahci_ceva.c:187:2: note: in expansion of macro ‘AHCI_SHT’
AHCI_SHT(DRV_NAME),
^
Also it turns out that this is an old issue and not a regression
introduced by commit 6f09eae3b5d974ef845e56690d6bc2b8f2a70acd
("ata: expose ncq_enable_prio sysfs attribute only on NCQ capable
hosts").
Anyway, it should be fixed by:
https://lore.kernel.org/linux-ide/0d803e72-b15e-4673-4858-4741f2772...@samsung.com/
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
[PATCH] ata: fix AHCI_SHT() macro definition
Fix AHCI_SHT() macro definition to not reinitialize .can_queue and .sdev_attrs fields. This removes gcc warnings from W=1 builds such as: CC drivers/ata/ahci_platform.o In file included from drivers/ata/ahci_platform.c:21:0: drivers/ata/ahci.h:384:16: warning: initialized field overwritten [-Woverride-init] .can_queue = AHCI_MAX_CMDS, \ ^ drivers/ata/ahci_platform.c:40:2: note: in expansion of macro ‘AHCI_SHT’ AHCI_SHT(DRV_NAME), ^ drivers/ata/ahci.h:384:16: note: (near initialization for ‘ahci_platform_sht.can_queue’) .can_queue = AHCI_MAX_CMDS, \ ^ drivers/ata/ahci_platform.c:40:2: note: in expansion of macro ‘AHCI_SHT’ AHCI_SHT(DRV_NAME), ^ drivers/ata/ahci.h:388:17: warning: initialized field overwritten [-Woverride-init] .sdev_attrs = ahci_sdev_attrs ^ drivers/ata/ahci_platform.c:40:2: note: in expansion of macro ‘AHCI_SHT’ AHCI_SHT(DRV_NAME), ^ drivers/ata/ahci.h:388:17: note: (near initialization for ‘ahci_platform_sht.sdev_attrs’) .sdev_attrs = ahci_sdev_attrs ^ drivers/ata/ahci_platform.c:40:2: note: in expansion of macro ‘AHCI_SHT’ AHCI_SHT(DRV_NAME), ^ Reported-by: kernel test robot Cc: Christoph Hellwig Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ata/ahci.h |7 --- include/linux/libata.h |3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) Index: b/drivers/ata/ahci.h === --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -378,15 +378,16 @@ extern struct device_attribute *ahci_sde /* * This must be instantiated by the edge drivers. Read the comments - * for ATA_BASE_SHT + * for __ATA_BASE_SHT */ #define AHCI_SHT(drv_name) \ - ATA_NCQ_SHT(drv_name), \ + __ATA_BASE_SHT(drv_name), \ .can_queue = AHCI_MAX_CMDS,\ .sg_tablesize = AHCI_MAX_SG, \ .dma_boundary = AHCI_DMA_BOUNDARY,\ .shost_attrs= ahci_shost_attrs, \ - .sdev_attrs = ahci_sdev_attrs + .sdev_attrs = ahci_sdev_attrs, \ + .change_queue_depth = ata_scsi_change_queue_depth extern struct ata_port_operations ahci_ops; extern struct ata_port_operations ahci_platform_ops; Index: b/include/linux/libata.h === --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1389,7 +1389,6 @@ extern struct device_attribute *ata_comm ATA_SCSI_COMPAT_IOCTL \ .queuecommand = ata_scsi_queuecmd,\ .dma_need_drain = ata_scsi_dma_need_drain, \ - .can_queue = ATA_DEF_QUEUE,\ .tag_alloc_policy = BLK_TAG_ALLOC_RR, \ .this_id= ATA_SHT_THIS_ID, \ .emulated = ATA_SHT_EMULATED, \ @@ -1401,6 +1400,7 @@ extern struct device_attribute *ata_comm #define ATA_BASE_SHT(drv_name) \ __ATA_BASE_SHT(drv_name), \ + .can_queue = ATA_DEF_QUEUE,\ .sdev_attrs = ata_common_sdev_attrs #ifdef CONFIG_SATA_HOST @@ -1408,6 +1408,7 @@ extern struct device_attribute *ata_ncq_ #define ATA_NCQ_SHT(drv_name) \ __ATA_BASE_SHT(drv_name), \ + .can_queue = ATA_DEF_QUEUE,\ .sdev_attrs = ata_ncq_sdev_attrs, \ .change_queue_depth = ata_scsi_change_queue_depth #endif
Re: [PATCH v2] fbdev: geode: Add the missed pci_disable_device() in gx1fb_map_video_memory()
Hi,
On 6/5/20 6:14 PM, Chuhong Yuan wrote:
> Although gx1fb_probe() has handled the failure of gx1fb_map_video_memory()
> partly, it does not call pci_disable_device() as gx1fb_map_video_memory()
> calls pci_enable_device().
> Add the missed function call to fix the bug.
>
> Fixes: 53eed4ec8bcd ("[PATCH] fbdev: geode updates]")
This doesn't seem to be a matching commit.
The proper commit seems to be:
commit a06630f3e7fb29f2524e1d7b009eb8b5a278ba23
Author: Antonino A. Daplas
Date: Mon Jun 26 00:27:04 2006 -0700
[PATCH] Detaching fbcon: remove calls to pci_disable_device()
Detaching fbcon allows individual drivers to be unloaded. However several
drivers call pci_disable_device() upon exit. This function will disable the
BAR's which will kill VGA text mode and/or affect X/DRM.
To prevent this, remove calls to pci_disable_device() from several drivers.
...
which removed pci_disable_device() calls from:
drivers/video/aty/radeon_base.c
drivers/video/cirrusfb.c
drivers/video/geode/gx1fb_core.c
drivers/video/geode/gxfb_core.c
drivers/video/i810/i810_main.c
drivers/video/nvidia/nvidia.c
drivers/video/riva/fbdev.c
In order to bring back pci_disable_device() calls to gx1fb
driver (and other affected ones) you should verify that
the issue described in the above commit is no longer present
(preferably with testing modified driver on a real hardware).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Signed-off-by: Chuhong Yuan
> ---
> Changes in v2:
> - Fix the typo in the subject.
> - Modify the label of error handler.
> - Refactor the code.
>
> drivers/video/fbdev/geode/gx1fb_core.c | 24 +---
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/fbdev/geode/gx1fb_core.c
> b/drivers/video/fbdev/geode/gx1fb_core.c
> index 5d34d89fb665..15645244e4d0 100644
> --- a/drivers/video/fbdev/geode/gx1fb_core.c
> +++ b/drivers/video/fbdev/geode/gx1fb_core.c
> @@ -208,29 +208,39 @@ static int gx1fb_map_video_memory(struct fb_info *info,
> struct pci_dev *dev)
>
> ret = pci_request_region(dev, 0, "gx1fb (video)");
> if (ret < 0)
> - return ret;
> + goto err_disable_device;
> par->vid_regs = pci_ioremap_bar(dev, 0);
> if (!par->vid_regs)
> - return -ENOMEM;
> + goto err_nomem;
>
> - if (!request_mem_region(gx_base + 0x8300, 0x100, "gx1fb (display
> controller)"))
> - return -EBUSY;
> + if (!request_mem_region(gx_base + 0x8300, 0x100,
> + "gx1fb (display controller)")) {
> + ret = -EBUSY;
> + goto err_disable_device;
> + }
> par->dc_regs = ioremap(gx_base + 0x8300, 0x100);
> if (!par->dc_regs)
> - return -ENOMEM;
> + goto err_nomem;
>
> if ((fb_len = gx1_frame_buffer_size()) < 0)
> - return -ENOMEM;
> + goto err_nomem;
> +
> info->fix.smem_start = gx_base + 0x80;
> info->fix.smem_len = fb_len;
> info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
> if (!info->screen_base)
> - return -ENOMEM;
> + goto err_nomem;
>
> dev_info(>dev, "%d Kibyte of video memory at 0x%lx\n",
>info->fix.smem_len / 1024, info->fix.smem_start);
>
> return 0;
> +
> +err_nomem:
> + ret = -ENOMEM;
> +err_disable_device:
> + pci_disable_device(dev);
> + return ret;
> }
>
> static int parse_panel_option(struct fb_info *info)
>
[PATCH v2] video: fbdev: uvesafb: fix "noblank" option handling
Fix the recent regression.
Fixes: dbc7ece12a38 ("video: uvesafb: use true,false for bool variables")
Cc: Jason Yan
Reviewed-by: Sam Ravnborg
Signed-off-by: Bartlomiej Zolnierkiewicz
---
v2:
- added Reviewed-by tag from Sam
- removed no longer working Michal's email address from Cc:
drivers/video/fbdev/uvesafb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: b/drivers/video/fbdev/uvesafb.c
===
--- a/drivers/video/fbdev/uvesafb.c
+++ b/drivers/video/fbdev/uvesafb.c
@@ -1836,7 +1836,7 @@ static int uvesafb_setup(char *options)
else if (!strcmp(this_opt, "noedid"))
noedid = true;
else if (!strcmp(this_opt, "noblank"))
- blank = true;
+ blank = false;
else if (!strncmp(this_opt, "vtotal:", 7))
vram_total = simple_strtoul(this_opt + 7, NULL, 0);
else if (!strncmp(this_opt, "vremap:", 7))
[PATCH v2 2/2] video: fbdev: amifb: add FIXMEs about {put,get}_user() failures
Since we lack the hardware (or proper emulator setup) for
testing needed changes add FIXMEs to document the issues
(so at least they are not forgotten).
Cc: Al Viro
Cc: Geert Uytterhoeven
Signed-off-by: Bartlomiej Zolnierkiewicz
---
v2:
- rebased on top of updated patch #1/2
drivers/video/fbdev/amifb.c |2 ++
1 file changed, 2 insertions(+)
Index: b/drivers/video/fbdev/amifb.c
===
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -1892,6 +1892,7 @@ static int ami_get_var_cursorinfo(struct
| ((datawords >> 15) & 1));
datawords <<= 1;
#endif
+ /* FIXME: check the return value + test the change */
put_user(color, data++);
}
if (bits > 0) {
@@ -1962,6 +1963,7 @@ static int ami_set_var_cursorinfo(struct
bits = 16; words = delta; datawords = 0;
for (width = (short)var->width - 1; width >= 0; width--) {
unsigned long tdata = 0;
+ /* FIXME: check the return value + test the change */
get_user(tdata, data);
data++;
#ifdef __mc68000__
[PATCH v2 1/2] video: fbdev: amifb: add FIXME about dead APUS support
On 5/14/20 10:21 PM, Geert Uytterhoeven wrote:
> These #ifdefs are relics from APUS (Amiga Power-Up System), which
> added a PPC board. APUS support was killed off a long time ago,
> when arch/ppc/ was still king, but these #ifdefs were missed, because
> they didn't test for CONFIG_APUS.
Add FIXME about using the C code variants (APUS ones) in the future.
Reported-by: Al Viro
Reported-by: Geert Uytterhoeven
Signed-off-by: Bartlomiej Zolnierkiewicz
---
v2:
- added FIXME comment instead of removing the C code variants
drivers/video/fbdev/amifb.c |6 ++
1 file changed, 6 insertions(+)
Index: b/drivers/video/fbdev/amifb.c
===
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -575,6 +575,12 @@ static u_short maxfmode, chipset;
#define downx(x, v)((v) & -(x))
#define modx(x, v) ((v) & ((x) - 1))
+/*
+ * FIXME: Use C variants of the code marked with #ifdef __mc68000__
+ * in the driver. It shouldn't negatively affect the performance and
+ * is required for APUS support (once it is re-added to the kernel).
+ * Needs to be tested on the hardware though..
+ */
/* if x1 is not a constant, this macro won't make real sense :-) */
#ifdef __mc68000__
#define DIVUL(x1, x2) ({int res; asm("divul %1,%2,%3": "=d" (res): \
Re: [PATCH 1/2] video: fbdev: amifb: remove dead APUS support
On 6/2/20 1:07 PM, John Paul Adrian Glaubitz wrote: > Hi! > > On 6/2/20 1:04 PM, Geert Uytterhoeven wrote: >>> What do you mean with the sentence "when arch/ppc/ was still king"? >> >> Ah, Bartl copied that from my email ;-) >> >> There used to be APUS support under arch/ppc/. >> Later, 32-bit arch/ppc/ and 64-bit arch/ppc64/ were merged in a new\ >> architecture port under arch/powerpc/, and the old ones were dropped. >> APUS was never converted, and thus dropped. > > Ah, yes. Similar to the merge with x86. > >>> Does that mean - in the case we would re-add APUS support in the future, >>> that >>> these particular changes would not be necessary? >> >> They would still be necessary, as PowerPC doesn't grok m68k instructions. >> Alternatively, we could just drop the m68k inline asm, and retain the C >> version instead? I have no idea how big of a difference that would make >> on m68k, using a more modern compiler than when the code was written >> originally. > > Hmm, no idea. I would keep the assembly for the time being. This was just > a question out of curiosity. We could still consider such a change if > someone should consider working on APUS support again. > >> Note that all of this is used only for cursor handling, which I doubt is >> actually used by any user space application. The only exception is the >> DIVUL() macro, which is used once during initialization, thus also not >> performance critical. > I see, thanks. Since the code in question is not performance critical it indeed seems to be good idea to use C version. However it still would need be tested on the hardware (or emulator at least) so for the time being I think that we should just add another FIXME comment instead of doing real code changes.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
[PATCH 2/2] video: fbdev: amifb: add FIXMEs about {put,get}_user() failures
Since we lack the hardware (or proper emulator setup) for
testing needed changes add FIXMEs to document the issues
(so at least they are not forgotten).
Cc: Al Viro
Cc: Geert Uytterhoeven
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/video/fbdev/amifb.c |2 ++
1 file changed, 2 insertions(+)
Index: b/drivers/video/fbdev/amifb.c
===
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -1866,6 +1866,7 @@ static int ami_get_var_cursorinfo(struct
"clrb %0 ; swap %1 ; lslw #1,%1 ; roxlb #1,%0 ;
"
"swap %1 ; lslw #1,%1 ; roxlb #1,%0"
: "=d" (color), "=d" (datawords) : "1"
(datawords));
+ /* FIXME: check the return value + test the change */
put_user(color, data++);
}
if (bits > 0) {
@@ -1923,6 +1924,7 @@ static int ami_set_var_cursorinfo(struct
bits = 16; words = delta; datawords = 0;
for (width = (short)var->width - 1; width >= 0; width--) {
unsigned long tdata = 0;
+ /* FIXME: check the return value + test the change */
get_user(tdata, data);
data++;
asm volatile (
[PATCH 1/2] video: fbdev: amifb: remove dead APUS support
On 5/14/20 10:21 PM, Geert Uytterhoeven wrote:
> These #ifdefs are relics from APUS (Amiga Power-Up System), which
> added a PPC board. APUS support was killed off a long time ago,
> when arch/ppc/ was still king, but these #ifdefs were missed, because
> they didn't test for CONFIG_APUS.
Reported-by: Al Viro
Reported-by: Geert Uytterhoeven
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/video/fbdev/amifb.c | 63
1 file changed, 63 deletions(-)
Index: b/drivers/video/fbdev/amifb.c
===
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -576,14 +576,8 @@ static u_short maxfmode, chipset;
#define modx(x, v) ((v) & ((x) - 1))
/* if x1 is not a constant, this macro won't make real sense :-) */
-#ifdef __mc68000__
#define DIVUL(x1, x2) ({int res; asm("divul %1,%2,%3": "=d" (res): \
"d" (x2), "d" ((long)((x1) / 0x1ULL)), "0" ((long)(x1))); res;})
-#else
-/* We know a bit about the numbers, so we can do it this way */
-#define DIVUL(x1, x2) long)((unsigned long long)x1 >> 8) / x2) << 8) + \
- long)((unsigned long long)x1 >> 8) % x2) << 8) / x2))
-#endif
#define highw(x) ((u_long)(x)>>16 & 0x)
#define loww(x)((u_long)(x) & 0x)
@@ -1837,11 +1831,7 @@ static int ami_get_var_cursorinfo(struct
const struct amifb_par *par)
{
register u_short *lspr, *sspr;
-#ifdef __mc68000__
register u_long datawords asm ("d2");
-#else
- register u_long datawords;
-#endif
register short delta;
register u_char color;
short height, width, bits, words;
@@ -1868,24 +1858,14 @@ static int ami_get_var_cursorinfo(struct
for (width = (short)var->width - 1; width >= 0; width--) {
if (bits == 0) {
bits = 16; --words;
-#ifdef __mc68000__
asm volatile ("movew %1@(%3:w:2),%0 ; swap %0 ;
movew %1@+,%0"
: "=d" (datawords), "=a" (lspr) : "1"
(lspr), "d" (delta));
-#else
- datawords = (*(lspr + delta) << 16) | (*lspr++);
-#endif
}
--bits;
-#ifdef __mc68000__
asm volatile (
"clrb %0 ; swap %1 ; lslw #1,%1 ; roxlb #1,%0 ;
"
"swap %1 ; lslw #1,%1 ; roxlb #1,%0"
: "=d" (color), "=d" (datawords) : "1"
(datawords));
-#else
- color = (((datawords >> 30) & 2)
-| ((datawords >> 15) & 1));
- datawords <<= 1;
-#endif
put_user(color, data++);
}
if (bits > 0) {
@@ -1893,17 +1873,8 @@ static int ami_get_var_cursorinfo(struct
}
while (--words >= 0)
++lspr;
-#ifdef __mc68000__
asm volatile ("lea %0@(%4:w:2),%0 ; tstl %1 ; jeq 1f ; exg
%0,%1\n1:"
: "=a" (lspr), "=a" (sspr) : "0" (lspr), "1" (sspr),
"d" (delta));
-#else
- lspr += delta;
- if (sspr) {
- u_short *tmp = lspr;
- lspr = sspr;
- sspr = tmp;
- }
-#endif
}
return 0;
}
@@ -1912,11 +1883,7 @@ static int ami_set_var_cursorinfo(struct
u_char __user *data, struct amifb_par *par)
{
register u_short *lspr, *sspr;
-#ifdef __mc68000__
register u_long datawords asm ("d2");
-#else
- register u_long datawords;
-#endif
register short delta;
u_short fmode;
short height, width, bits, words;
@@ -1958,60 +1925,30 @@ static int ami_set_var_cursorinfo(struct
unsigned long tdata = 0;
get_user(tdata, data);
data++;
-#ifdef __mc68000__
asm volatile (
"lsrb #1,%2 ; roxlw #1,%0 ; swap %0 ; "
"lsrb #1,%2 ; roxlw #1,%0 ; swap %0"
: "=d" (datawords)
: "0" (datawords), "d" (tdata));
-#else
- datawords = ((datawords << 1) & 0xfffefffe);
- datawords |= tdata & 1;
- datawords |= (tdata & 2) << (1
Re: [PATCH] ata: return true in ata_is_host_link()
On 5/7/20 1:06 PM, Jason Yan wrote:
> Fix the following coccicheck warning:
>
> include/linux/libata.h:1446:8-9: WARNING: return of 0/1 in function
> 'ata_is_host_link' with return type bool
>
> Signed-off-by: Jason Yan
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> include/linux/libata.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 8bf5e59a7859..e05a8ed2e31e 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1443,7 +1443,7 @@ static inline bool sata_pmp_attached(struct ata_port
> *ap)
>
> static inline bool ata_is_host_link(const struct ata_link *link)
> {
> - return 1;
> + return true;
> }
> #endif /* CONFIG_SATA_PMP */
>
Re: [PATCH] video: fbdev: pxafb: Use correct return value for pxafb_probe()
On 5/25/20 9:11 AM, Tiezhu Yang wrote:
> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
> to check the return value and return PTR_ERR() if failed.
>
> Signed-off-by: Tiezhu Yang
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/pxafb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
> index 00b96a7..423331c 100644
> --- a/drivers/video/fbdev/pxafb.c
> +++ b/drivers/video/fbdev/pxafb.c
> @@ -2305,7 +2305,7 @@ static int pxafb_probe(struct platform_device *dev)
> fbi->mmio_base = devm_platform_ioremap_resource(dev, 0);
> if (IS_ERR(fbi->mmio_base)) {
> dev_err(>dev, "failed to get I/O memory\n");
> - ret = -EBUSY;
> + ret = PTR_ERR(fbi->mmio_base);
> goto failed;
> }
>
>
Re: [PATCH] omapfb/dss: fix comparison to bool warning
On 4/22/20 9:19 AM, Jason Yan wrote:
> Fix the following coccicheck warning:
>
> drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c:461:15-32: WARNING:
> Comparison to bool
> drivers/video/fbdev/omap2/omapfb/dss/dispc.c:891:5-35: WARNING:
> Comparison of 0/1 to bool variable
>
> Signed-off-by: Jason Yan
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 2 +-
> drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 4 +---
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> index 4a16798b2ecd..3bb951eb29c7 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -888,7 +888,7 @@ static void dispc_ovl_set_color_mode(enum omap_plane
> plane,
> static void dispc_ovl_configure_burst_type(enum omap_plane plane,
> enum omap_dss_rotation_type rotation_type)
> {
> - if (dss_has_feature(FEAT_BURST_2D) == 0)
> + if (!dss_has_feature(FEAT_BURST_2D))
> return;
>
> if (rotation_type == OMAP_DSS_ROT_TILER)
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> index 7060ae56c062..ef659c89ba58 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c
> @@ -455,11 +455,9 @@ static void hdmi_disconnect(struct omap_dss_device
> *dssdev,
> static int hdmi_read_edid(struct omap_dss_device *dssdev,
> u8 *edid, int len)
> {
> - bool need_enable;
> + bool need_enable = !hdmi.core_enabled;
> int r;
>
> - need_enable = hdmi.core_enabled == false;
> -
> if (need_enable) {
> r = hdmi_core_enable(dssdev);
> if (r)
>
Re: [PATCH] video: pxafb: Fix the function used to balance a 'dma_alloc_coherent()' call
On 4/29/20 10:45 AM, Christophe JAILLET wrote:
> 'dma_alloc_coherent()' must be balanced by a call to 'dma_free_coherent()'
> not 'dma_free_wc()'.
> The correct dma_free_ function is already used in the error handling path
> of the probe function.
>
> Fixes: 77e196752bdd ("[ARM] pxafb: allow video memory size to be
> configurable")
> Signed-off-by: Christophe JAILLET
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/pxafb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
> index 00b96a78676e..6f972bed410a 100644
> --- a/drivers/video/fbdev/pxafb.c
> +++ b/drivers/video/fbdev/pxafb.c
> @@ -2417,8 +2417,8 @@ static int pxafb_remove(struct platform_device *dev)
>
> free_pages_exact(fbi->video_mem, fbi->video_mem_size);
>
> - dma_free_wc(>dev, fbi->dma_buff_size, fbi->dma_buff,
> - fbi->dma_buff_phys);
> + dma_free_coherent(>dev, fbi->dma_buff_size, fbi->dma_buff,
> + fbi->dma_buff_phys);
>
> return 0;
> }
>
Re: [trivial PATCH] video: fbdev: Use IS_BUILTIN
On 5/5/20 1:29 AM, Joe Perches wrote: > IS_BUILTIN can be use to replace various initializations > like #if CONFIG_ int val = 1; #else int val = 0; #endif > so do so. > > Signed-off-by: Joe Perches Applied to drm-misc-next tree (patch should show up in v5.9), thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/aty/aty128fb.c | 6 +- > drivers/video/fbdev/aty/atyfb_base.c | 7 +-- > drivers/video/fbdev/aty/radeon_base.c | 6 +- > drivers/video/fbdev/nvidia/nvidia.c| 6 +- > drivers/video/fbdev/omap/omapfb_main.c | 6 +- > drivers/video/fbdev/riva/fbdev.c | 6 +- > drivers/video/fbdev/s3c2410fb.c| 6 +- > 7 files changed, 7 insertions(+), 36 deletions(-) > > diff --git a/drivers/video/fbdev/aty/aty128fb.c > b/drivers/video/fbdev/aty/aty128fb.c > index d05d4195acad..6fae6ad6cb77 100644 > --- a/drivers/video/fbdev/aty/aty128fb.c > +++ b/drivers/video/fbdev/aty/aty128fb.c > @@ -384,11 +384,7 @@ static int default_lcd_on = 1; > static bool mtrr = true; > > #ifdef CONFIG_FB_ATY128_BACKLIGHT > -#ifdef CONFIG_PMAC_BACKLIGHT > -static int backlight = 1; > -#else > -static int backlight = 0; > -#endif > +static int backlight = IS_BUILTIN(CONFIG_PMAC_BACKLIGHT); > #endif > > /* PLL constants */ > diff --git a/drivers/video/fbdev/aty/atyfb_base.c > b/drivers/video/fbdev/aty/atyfb_base.c > index 49d192869cf5..23a29d61c2a2 100644 > --- a/drivers/video/fbdev/aty/atyfb_base.c > +++ b/drivers/video/fbdev/aty/atyfb_base.c > @@ -317,12 +317,7 @@ static int mclk; > static int xclk; > static int comp_sync = -1; > static char *mode; > - > -#ifdef CONFIG_PMAC_BACKLIGHT > -static int backlight = 1; > -#else > -static int backlight = 0; > -#endif > +static int backlight = IS_BUILTIN(CONFIG_PMAC_BACKLIGHT); > > #ifdef CONFIG_PPC > static int default_vmode = VMODE_CHOOSE; > diff --git a/drivers/video/fbdev/aty/radeon_base.c > b/drivers/video/fbdev/aty/radeon_base.c > index e116a3f9ad56..3fe509cb9b87 100644 > --- a/drivers/video/fbdev/aty/radeon_base.c > +++ b/drivers/video/fbdev/aty/radeon_base.c > @@ -269,11 +269,7 @@ static bool force_measure_pll = 0; > static bool nomtrr = 0; > static bool force_sleep; > static bool ignore_devlist; > -#ifdef CONFIG_PMAC_BACKLIGHT > -static int backlight = 1; > -#else > -static int backlight = 0; > -#endif > +static int backlight = IS_BUILTIN(CONFIG_PMAC_BACKLIGHT); > > /* Note about this function: we have some rare cases where we must not > schedule, > * this typically happen with our special "wake up early" hook which allows > us to > diff --git a/drivers/video/fbdev/nvidia/nvidia.c > b/drivers/video/fbdev/nvidia/nvidia.c > index c24de9107958..c6820e21875d 100644 > --- a/drivers/video/fbdev/nvidia/nvidia.c > +++ b/drivers/video/fbdev/nvidia/nvidia.c > @@ -74,11 +74,7 @@ static int vram = 0; > static int bpp = 8; > static int reverse_i2c; > static bool nomtrr = false; > -#ifdef CONFIG_PMAC_BACKLIGHT > -static int backlight = 1; > -#else > -static int backlight = 0; > -#endif > +static int backlight = IS_BUILTIN(CONFIG_PMAC_BACKLIGHT); > > static char *mode_option = NULL; > > diff --git a/drivers/video/fbdev/omap/omapfb_main.c > b/drivers/video/fbdev/omap/omapfb_main.c > index 1a9d6242916e..0cbcc74fa943 100644 > --- a/drivers/video/fbdev/omap/omapfb_main.c > +++ b/drivers/video/fbdev/omap/omapfb_main.c > @@ -34,11 +34,7 @@ static unsigned long def_vyres; > static unsigned int def_rotate; > static unsigned int def_mirror; > > -#ifdef CONFIG_FB_OMAP_MANUAL_UPDATE > -static bool manual_update = 1; > -#else > -static bool manual_update; > -#endif > +static bool manual_update = IS_BUILTIN(CONFIG_FB_OMAP_MANUAL_UPDATE); > > static struct platform_device*fbdev_pdev; > static struct lcd_panel *fbdev_panel; > diff --git a/drivers/video/fbdev/riva/fbdev.c > b/drivers/video/fbdev/riva/fbdev.c > index 764ec3285e62..9b3493846f4d 100644 > --- a/drivers/video/fbdev/riva/fbdev.c > +++ b/drivers/video/fbdev/riva/fbdev.c > @@ -202,11 +202,7 @@ static int flatpanel = -1; /* Autodetect later */ > static int forceCRTC = -1; > static bool noaccel = 0; > static bool nomtrr = 0; > -#ifdef CONFIG_PMAC_BACKLIGHT > -static int backlight = 1; > -#else > -static int backlight = 0; > -#endif > +static int backlight = IS_BUILTIN(CONFIG_PMAC_BACKLIGHT); > > static char *mode_option = NULL; > static bool strictmode = 0; > diff --git a/drivers/video/fbdev/s3c2410fb.c b/drivers/video/fbdev/s3c2410
Re: [PATCH v3] console: newport_con: fix an issue about leak related system resources
On 4/23/20 6:42 PM, Dejin Zheng wrote:
> A call of the function do_take_over_console() can fail here.
> The corresponding system resources were not released then.
> Thus add a call of iounmap() and release_mem_region()
> together with the check of a failure predicate. and also
> add release_mem_region() on device removal.
>
> Fixes: e86bb8acc0fdc ("[PATCH] VT binding: Make newport_con support binding")
> Cc: Andy Shevchenko
> Suggested-by: Bartlomiej Zolnierkiewicz
> Signed-off-by: Dejin Zheng
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> v2 -> v3:
> - modify commit tag CC to Cc by Andy's suggestion.
> - modify Subject 'console: console:' to 'console: newport_con:'
> by Bartlomiej's suggestion.
> - add missing release_mem_region() on error and on device removal
> by Bartlomiej's suggestion.
> - add correct fixes commit, before this patch, add a wrong 'Fixes:
> e84de0c6190503 ("MIPS: GIO bus support for SGI IP22/28")'
> thanks Bartlomiej again!
>
> v1 -> v2:
> - modify the commit comments. The commit comments have some more
> appropriate instructions by Markus'suggestion. here is my first
> version commit comments:
>
> if do_take_over_console() return an error in the newport_probe(),
> due to the io virtual address is not released, it will cause a
> leak.
>
> drivers/video/console/newport_con.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/console/newport_con.c
> b/drivers/video/console/newport_con.c
> index 00dddf6e08b0..2d2ee17052e8 100644
> --- a/drivers/video/console/newport_con.c
> +++ b/drivers/video/console/newport_con.c
> @@ -32,6 +32,8 @@
> #include
> #include
>
> +#define NEWPORT_LEN 0x1
> +
> #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
>
> /* borrowed from fbcon.c */
> @@ -43,6 +45,7 @@
> static unsigned char *font_data[MAX_NR_CONSOLES];
>
> static struct newport_regs *npregs;
> +static unsigned long newport_addr;
>
> static int logo_active;
> static int topscan;
> @@ -702,7 +705,6 @@ const struct consw newport_con = {
> static int newport_probe(struct gio_device *dev,
>const struct gio_device_id *id)
> {
> - unsigned long newport_addr;
> int err;
>
> if (!dev->resource.start)
> @@ -712,7 +714,7 @@ static int newport_probe(struct gio_device *dev,
> return -EBUSY; /* we only support one Newport as console */
>
> newport_addr = dev->resource.start + 0xF;
> - if (!request_mem_region(newport_addr, 0x1, "Newport"))
> + if (!request_mem_region(newport_addr, NEWPORT_LEN, "Newport"))
> return -ENODEV;
>
> npregs = (struct newport_regs *)/* ioremap cannot fail */
> @@ -720,6 +722,11 @@ static int newport_probe(struct gio_device *dev,
> console_lock();
> err = do_take_over_console(_con, 0, MAX_NR_CONSOLES - 1, 1);
> console_unlock();
> +
> + if (err) {
> + iounmap((void *)npregs);
> + release_mem_region(newport_addr, NEWPORT_LEN);
> + }
> return err;
> }
>
> @@ -727,6 +734,7 @@ static void newport_remove(struct gio_device *dev)
> {
> give_up_console(_con);
> iounmap((void *)npregs);
> + release_mem_region(newport_addr, NEWPORT_LEN);
> }
>
> static struct gio_device_id newport_ids[] = {
>
Re: [PATCH v1] video: fbdev: sm712fb: fix an issue about iounmap for a wrong address
On 4/22/20 6:07 PM, Dejin Zheng wrote:
> the sfb->fb->screen_base is not save the value get by iounmap() when
> the chip id is 0x720. so iounmap() for address sfb->fb->screen_base
> is not right.
>
> Fixes: 1461d6672864854 ("staging: sm7xxfb: merge sm712fb with fbdev")
> CC: Andy Shevchenko
> Signed-off-by: Dejin Zheng
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/fbdev/sm712fb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
> index 6a1b4a853d9e..8cd655d6d628 100644
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -1429,6 +1429,8 @@ static int smtc_map_smem(struct smtcfb_info *sfb,
> static void smtc_unmap_smem(struct smtcfb_info *sfb)
> {
> if (sfb && sfb->fb->screen_base) {
> + if (sfb->chip_id == 0x720)
> + sfb->fb->screen_base -= 0x0020;
> iounmap(sfb->fb->screen_base);
> sfb->fb->screen_base = NULL;
> }
>
Re: [PATCH] drivers/video: cleanup coding style in video a bit
On 4/27/20 10:05 AM, Bernard Zhao wrote:
> Eliminate the magic numbers, add vender infoframe size macro
> like other hdmi modules.
>
> Signed-off-by: Bernard Zhao
Applied to drm-misc-next tree (patch should show up in v5.9), thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/video/hdmi.c | 2 +-
> include/linux/hdmi.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index 856a8c4e84a2..f693076f2e5f 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -495,7 +495,7 @@ int hdmi_vendor_infoframe_init(struct
> hdmi_vendor_infoframe *frame)
>* value
>*/
> frame->s3d_struct = HDMI_3D_STRUCTURE_INVALID;
> - frame->length = 4;
> + frame->length = HDMI_VENDOR_INFOFRAME_SIZE;
>
> return 0;
> }
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 9613d796cfb1..ff25aeb95ee4 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -57,6 +57,7 @@ enum hdmi_infoframe_type {
> #define HDMI_SPD_INFOFRAME_SIZE25
> #define HDMI_AUDIO_INFOFRAME_SIZE 10
> #define HDMI_DRM_INFOFRAME_SIZE26
> +#define HDMI_VENDOR_INFOFRAME_SIZE 4
>
> #define HDMI_INFOFRAME_SIZE(type)\
> (HDMI_INFOFRAME_HEADER_SIZE + HDMI_ ## type ## _INFOFRAME_SIZE)
>
Re: [PATCH] video: uvesafb: use true,false for bool variables
Hi,
On 4/22/20 9:18 AM, Jason Yan wrote:
> Fix the following coccicheck warning:
>
> drivers/video/fbdev/uvesafb.c:48:12-17: WARNING: Assignment of 0/1 to
> bool variable
> drivers/video/fbdev/uvesafb.c:1827:3-13: WARNING: Assignment of 0/1 to
> bool variable
> drivers/video/fbdev/uvesafb.c:1829:3-13: WARNING: Assignment of 0/1 to
> bool variable
> drivers/video/fbdev/uvesafb.c:1835:3-9: WARNING: Assignment of 0/1 to
> bool variable
> drivers/video/fbdev/uvesafb.c:1837:3-9: WARNING: Assignment of 0/1 to
> bool variable
> drivers/video/fbdev/uvesafb.c:1839:3-8: WARNING: Assignment of 0/1 to
> bool variable
>
> Signed-off-by: Jason Yan
> ---
> drivers/video/fbdev/uvesafb.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c
> index 1b385cf76110..bee29aadc646 100644
> --- a/drivers/video/fbdev/uvesafb.c
> +++ b/drivers/video/fbdev/uvesafb.c
> @@ -45,7 +45,7 @@ static const struct fb_fix_screeninfo uvesafb_fix = {
> };
>
> static int mtrr = 3;/* enable mtrr by default */
> -static bool blank= 1;/* enable blanking by default */
> +static bool blank= true; /* enable blanking by default */
> static int ypan = 1;/* 0: scroll, 1: ypan, 2: ywrap */
> static bool pmi_setpal = true; /* use PMI for palette changes */
> static bool nocrtc; /* ignore CRTC settings */
> @@ -1824,19 +1824,19 @@ static int uvesafb_setup(char *options)
> else if (!strcmp(this_opt, "ywrap"))
> ypan = 2;
> else if (!strcmp(this_opt, "vgapal"))
> - pmi_setpal = 0;
> + pmi_setpal = false;
> else if (!strcmp(this_opt, "pmipal"))
> - pmi_setpal = 1;
> + pmi_setpal = true;
> else if (!strncmp(this_opt, "mtrr:", 5))
> mtrr = simple_strtoul(this_opt+5, NULL, 0);
> else if (!strcmp(this_opt, "nomtrr"))
> mtrr = 0;
> else if (!strcmp(this_opt, "nocrtc"))
> - nocrtc = 1;
> + nocrtc = true;
> else if (!strcmp(this_opt, "noedid"))
> - noedid = 1;
> + noedid = true;
> else if (!strcmp(this_opt, "noblank"))
> - blank = 0;
> + blank = true;
The above conversion is incorrect.
The follow-up fix is included below (the original patch has been
already applied).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
From: Bartlomiej Zolnierkiewicz
Subject: [PATCH] video: fbdev: uvesafb: fix "noblank" option handling
Fix the recent regression.
Fixes: dbc7ece12a38 ("video: uvesafb: use true,false for bool variables")
Cc: Jason Yan
Cc: Sam Ravnborg
Cc: Michal Januszewski
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/video/fbdev/uvesafb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: b/drivers/video/fbdev/uvesafb.c
===
--- a/drivers/video/fbdev/uvesafb.c
+++ b/drivers/video/fbdev/uvesafb.c
@@ -1836,7 +1836,7 @@ static int uvesafb_setup(char *options)
else if (!strcmp(this_opt, "noedid"))
noedid = true;
else if (!strcmp(this_opt, "noblank"))
- blank = true;
+ blank = false;
else if (!strncmp(this_opt, "vtotal:", 7))
vram_total = simple_strtoul(this_opt + 7, NULL, 0);
else if (!strncmp(this_opt, "vremap:", 7))
Re: [PATCH] ata: omit superfluous error message
Hi,
On 4/20/20 3:53 PM, Tang Bin wrote:
> In the probe function, when get irq failed, the function
> platform_get_irq() logs an error message, so remove
> redundant message here.
platform_get_irq() doesn't log an error message for -EPROBE_DEFER
case so the conversion shouldn't be done automatically for device
drivers which don't support deferred probing (i.e. pata_rb532_cf &
sata_highbank).
Unless there is a proof that -PROBE_DEFER can't happen for these
two drivers this patch shouldn't be applied.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Signed-off-by: Tang Bin
> ---
> drivers/ata/pata_rb532_cf.c | 4 +---
> drivers/ata/sata_highbank.c | 4 +---
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ata/pata_rb532_cf.c b/drivers/ata/pata_rb532_cf.c
> index 479c4b29b..dcde84f57 100644
> --- a/drivers/ata/pata_rb532_cf.c
> +++ b/drivers/ata/pata_rb532_cf.c
> @@ -115,10 +115,8 @@ static int rb532_pata_driver_probe(struct
> platform_device *pdev)
> }
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - dev_err(>dev, "no IRQ resource found\n");
> + if (irq <= 0)
> return -ENOENT;
> - }
>
> gpiod = devm_gpiod_get(>dev, NULL, GPIOD_IN);
> if (IS_ERR(gpiod)) {
> diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
> index ad3893c62..efd1925a9 100644
> --- a/drivers/ata/sata_highbank.c
> +++ b/drivers/ata/sata_highbank.c
> @@ -469,10 +469,8 @@ static int ahci_highbank_probe(struct platform_device
> *pdev)
> }
>
> irq = platform_get_irq(pdev, 0);
> - if (irq <= 0) {
> - dev_err(dev, "no irq\n");
> + if (irq <= 0)
> return -EINVAL;
> - }
>
> hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
> if (!hpriv) {
>
Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
On 5/14/20 4:07 PM, Al Viro wrote:
> On Thu, May 14, 2020 at 03:45:09PM +0200, Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi Al,
>>
>> On 5/10/20 1:45 AM, Al Viro wrote:
>>> From: Al Viro
>>>
>>> addresses passed only to get_user() and put_user()
>>
>> This driver lacks checks for {get,put}_user() return values so it will
>> now return 0 ("success") even if {get,put}_user() fails.
>>
>> Am I missing something?
>
> "now" is interesting, considering
> /* We let the MMU do all checking */
> static inline int access_ok(const void __user *addr,
> unsigned long size)
> {
> return 1;
> }
> in arch/m68k/include/asm/uaccess_mm.h
>
> Again, access_ok() is *NOT* about checking if memory is
> readable/writable/there
> in the first place. All it does is a static check that address is in
> "userland" range - on architectures that have kernel and userland sharing the
> address space. On architectures where we have separate ASI or equivalents
> thereof for kernel and for userland the fscker is always true.
>
> If MMU will prevent access to kernel memory by uaccess insns for given address
> range, access_ok() is fine with it. It does not do anything else.
>
> And yes, get_user()/put_user() callers should handle the fact that those can
> fail. Which they bloody well can _after_ _success_ of access_ok(). And
> without any races whatsoever.
>
> IOW, the lack of such checks is a bug, but it's quite independent from the
> bogus access_ok() call. On any architecture. mmap() something, munmap()
> it and pass the address where it used to be to that ioctl(). Failing
> get_user()/put_user() is guaranteed, so's succeeding access_ok().
>
> And that code is built only on amiga, so access_ok() always succeeds, anyway.
Thank you for in-detail explanations, for this patch:
Acked-by: Bartlomiej Zolnierkiewicz
Could you also please take care of adding missing checks for {get,put}_user()
failures later?
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
Hi Al,
On 5/10/20 1:45 AM, Al Viro wrote:
> From: Al Viro
>
> addresses passed only to get_user() and put_user()
This driver lacks checks for {get,put}_user() return values so it will
now return 0 ("success") even if {get,put}_user() fails.
Am I missing something?
> Signed-off-by: Al Viro
> ---
> drivers/video/fbdev/amifb.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c
> index 20e03e00b66d..6062104f3afb 100644
> --- a/drivers/video/fbdev/amifb.c
> +++ b/drivers/video/fbdev/amifb.c
> @@ -1855,8 +1855,6 @@ static int ami_get_var_cursorinfo(struct
> fb_var_cursorinfo *var,
> var->yspot = par->crsr.spot_y;
> if (size > var->height * var->width)
> return -ENAMETOOLONG;
> - if (!access_ok(data, size))
> - return -EFAULT;
> delta = 1 << par->crsr.fmode;
> lspr = lofsprite + (delta << 1);
> if (par->bplcon0 & BPC0_LACE)
> @@ -1935,8 +1933,6 @@ static int ami_set_var_cursorinfo(struct
> fb_var_cursorinfo *var,
> return -EINVAL;
> if (!var->height)
> return -EINVAL;
> - if (!access_ok(data, var->width * var->height))
> - return -EFAULT;
> delta = 1 << fmode;
> lofsprite = shfsprite = (u_short *)spritememory;
> lspr = lofsprite + (delta << 1);
>
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 12/20] omapfb: get rid of pointless access_ok() calls
On 5/10/20 1:45 AM, Al Viro wrote: > From: Al Viro > > address is passed only to copy_to_user() > > Signed-off-by: Al Viro Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > index 56995f44e76d..f40be68d5aac 100644 > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c > @@ -482,9 +482,6 @@ static int omapfb_memory_read(struct fb_info *fbi, > if (!display || !display->driver->memory_read) > return -ENOENT; > > - if (!access_ok(mr->buffer, mr->buffer_size)) > - return -EFAULT; > - > if (mr->w > 4096 || mr->h > 4096) > return -EINVAL; > >
Re: [PATCH v10 3/3] tty: samsung_tty: 32-bit access for TX/RX hold registers
Hi!
On 5/6/20 10:02 AM, Hyunki Koo wrote:
> Support 32-bit access for the TX/RX hold registers UTXH and URXH.
>
> This is required for some newer SoCs.
Krzysztof has asked this previously but I couldn't find the answer in
previous mails:
Do you plan to upstream support for these newer SoCs?
If not (i.e. this code is only to support Android GKI) then the code
you are adding now may be removed at any time later during cleanups
(due to lack of the in-kernel users).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Signed-off-by: Hyunki Koo
> Reviewed-by: Krzysztof Kozlowski
> Tested on Odroid HC1 (Exynos5422):
> Tested-by: Krzysztof Kozlowski
> ---
> drivers/tty/serial/samsung_tty.c | 62
>
> 1 file changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/samsung_tty.c
> b/drivers/tty/serial/samsung_tty.c
> index 326b0164609c..6ef614d8648c 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -154,10 +154,33 @@ struct s3c24xx_uart_port {
> #define portaddrl(port, reg) \
> ((unsigned long *)(unsigned long)((port)->membase + (reg)))
>
> -#define rd_reg(port, reg) (readb_relaxed(portaddr(port, reg)))
> +static u32 rd_reg(struct uart_port *port, u32 reg)
> +{
> + switch (port->iotype) {
> + case UPIO_MEM:
> + return readb_relaxed(portaddr(port, reg));
> + case UPIO_MEM32:
> + return readl_relaxed(portaddr(port, reg));
> + default:
> + return 0;
> + }
> + return 0;
> +}
> +
> #define rd_regl(port, reg) (readl_relaxed(portaddr(port, reg)))
>
> -#define wr_reg(port, reg, val) writeb_relaxed(val, portaddr(port, reg))
> +static void wr_reg(struct uart_port *port, u32 reg, u32 val)
> +{
> + switch (port->iotype) {
> + case UPIO_MEM:
> + writeb_relaxed(val, portaddr(port, reg));
> + break;
> + case UPIO_MEM32:
> + writel_relaxed(val, portaddr(port, reg));
> + break;
> + }
> +}
> +
> #define wr_regl(port, reg, val) writel_relaxed(val, portaddr(port, reg))
>
> /* Byte-order aware bit setting/clearing functions. */
> @@ -1974,7 +1997,7 @@ static int s3c24xx_serial_probe(struct platform_device
> *pdev)
> struct device_node *np = pdev->dev.of_node;
> struct s3c24xx_uart_port *ourport;
> int index = probe_index;
> - int ret;
> + int ret, prop = 0;
>
> if (np) {
> ret = of_alias_get_id(np, "serial");
> @@ -2000,10 +2023,27 @@ static int s3c24xx_serial_probe(struct
> platform_device *pdev)
> dev_get_platdata(>dev) :
> ourport->drv_data->def_cfg;
>
> - if (np)
> + if (np) {
> of_property_read_u32(np,
> "samsung,uart-fifosize", >port.fifosize);
>
> + if (of_property_read_u32(np, "reg-io-width", ) == 0) {
> + switch (prop) {
> + case 1:
> + ourport->port.iotype = UPIO_MEM;
> + break;
> + case 4:
> + ourport->port.iotype = UPIO_MEM32;
> + break;
> + default:
> + dev_warn(>dev, "unsupported reg-io-width
> (%d)\n",
> + prop);
> + ret = -EINVAL;
> + break;
> + }
> + }
> + }
> +
> if (ourport->drv_data->fifosize[index])
> ourport->port.fifosize = ourport->drv_data->fifosize[index];
> else if (ourport->info->fifosize)
> @@ -2587,6 +2627,18 @@ module_platform_driver(samsung_serial_driver);
> * Early console.
> */
>
> +static void wr_reg_barrier(struct uart_port *port, u32 reg, u32 val)
> +{
> + switch (port->iotype) {
> + case UPIO_MEM:
> + writeb(val, portaddr(port, reg));
> + break;
> + case UPIO_MEM32:
> + writel(val, portaddr(port, reg));
> + break;
> + }
> +}
> +
> struct samsung_early_console_data {
> u32 txfull_mask;
> };
> @@ -2612,7 +2664,7 @@ static void samsung_early_putc(struct uart_port *port,
> int c)
> else
> samsung_early_busyuart(port);
>
> - writeb(c, port->membase + S3C2410_UTXH);
> + wr_reg_barrier(port, S3C2410_UTXH, c);
> }
>
> static void samsung_early_write(struct console *con, const char *s,
>
Re: [PATCH v2] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y && PPC32=n
On 4/29/20 2:51 PM, Christoph Hellwig wrote: > Why do we even bother allocing the driver to compile for !ppc32 > given that it clearly needs ppc-specific infrastructure? The whole > idea of needing magic stubs for the COMPILE_TEST case seems rather > counterproduction. Not a perfect solution but at the cost of 2 ifdefs it allows controlfb driver to be compile tested on any arch. Being able to compile test fbdev device drivers is really useful for me and saves me a lot of time when doing fbdev maintainer duties. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH v2] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y && PPC32=n
On 4/29/20 1:54 PM, Sam Ravnborg wrote:
> Hi Bartlomiej.
>
> On Wed, Apr 29, 2020 at 12:48:24PM +0200, Bartlomiej Zolnierkiewicz wrote:
>>
>> powerpc allyesconfig fails like this:
>>
>> drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
>> drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of
>> function 'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
>> [-Werror=implicit-function-declaration]
>> 756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
>> | ^~~
>> | pgprot_cached
>> drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
>> assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
>>
>> Fix it by adding missing PPC32 dependency.
>
> Is this really the right fix?
Yes, ifdef in the code should match driver dependencies in Kconfig:
config FB_CONTROL
bool "Apple \"control\" display support"
depends on (FB = y) && ((PPC_PMAC && PPC32) || COMPILE_TEST)
> Short term I htink it is OK, but I think there should be a common way
> to do the same for all archtectures so no conditional compilation is
> needed. In other words the use of pgprot_cached_wthru looks like we
> need a better abstraction.
This would be of course nice to have but won't be enough to remove
the ifdef in this particular driver.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> Added Christoph to the mail as he has a good overview of the area.
>
> Sam
>
>
>>
>> Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
>> Reported-by: Stephen Rothwell
>> Reported-by: kbuild test robot
>> Cc: Sam Ravnborg
>> Cc: Daniel Vetter
>> Signed-off-by: Bartlomiej Zolnierkiewicz
>> ---
>> v2: fix implicit btext_update_display() function declaration error
>>
>> drivers/video/fbdev/controlfb.c |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: b/drivers/video/fbdev/controlfb.c
>> ===
>> --- a/drivers/video/fbdev/controlfb.c
>> +++ b/drivers/video/fbdev/controlfb.c
>> @@ -55,7 +55,7 @@
>> #include "macmodes.h"
>> #include "controlfb.h"
>>
>> -#ifndef CONFIG_PPC_PMAC
>> +#if !defined(CONFIG_PPC_PMAC) || !defined(CONFIG_PPC32)
>> #define invalid_vram_cache(addr)
>> #undef in_8
>> #undef out_8
>
>
Re: linux-next: build failure after merge of the drm-misc tree
On 4/29/20 10:09 AM, Bartlomiej Zolnierkiewicz wrote:
>
> Hi Stephen,
>
> On 4/29/20 12:33 AM, Stephen Rothwell wrote:
>> Hi all,
>>
>> On Mon, 20 Apr 2020 13:01:18 +1000 Stephen Rothwell
>> wrote:
>>>
>>> After merging the drm-misc tree, today's linux-next build (powerpc
>>> allyesconfig) failed like this:
>>>
>>> drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
>>> drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of
>>> function 'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
>>> [-Werror=implicit-function-declaration]
>>> 756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
>>> | ^~~
>>> | pgprot_cached
>>> drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
>>> assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
>>>
>>> Presumably exposed by commit
>>>
>>> a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
>>>
>>> I just turned off COMPILE_TEST again for today. Please let me know when
>>> this is fixed.
>>
>> This still appears to have not been addressed.
>
> Sorry for the delay, I've just posted a patch (also included below):
>
> "[PATCH] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y
> && PPC32=n"
>
> which should fix it.
>
> Please verify it, thank you!
I have tested it with powerpc allyesconfig now and it adds one dependency too
much,
fixed in v2:
https://lore.kernel.org/lkml/fe520316-3863-e6c4-9581-5d709f49e...@samsung.com/
Sam, could you please review / merge it to drm-misc-next?
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
[PATCH v2] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y && PPC32=n
powerpc allyesconfig fails like this:
drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of function
'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
[-Werror=implicit-function-declaration]
756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
| ^~~
| pgprot_cached
drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
Fix it by adding missing PPC32 dependency.
Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
Reported-by: Stephen Rothwell
Reported-by: kbuild test robot
Cc: Sam Ravnborg
Cc: Daniel Vetter
Signed-off-by: Bartlomiej Zolnierkiewicz
---
v2: fix implicit btext_update_display() function declaration error
drivers/video/fbdev/controlfb.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: b/drivers/video/fbdev/controlfb.c
===
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -55,7 +55,7 @@
#include "macmodes.h"
#include "controlfb.h"
-#ifndef CONFIG_PPC_PMAC
+#if !defined(CONFIG_PPC_PMAC) || !defined(CONFIG_PPC32)
#define invalid_vram_cache(addr)
#undef in_8
#undef out_8
Re: linux-next: build failure after merge of the drm-misc tree
Hi Stephen,
On 4/29/20 12:33 AM, Stephen Rothwell wrote:
> Hi all,
>
> On Mon, 20 Apr 2020 13:01:18 +1000 Stephen Rothwell
> wrote:
>>
>> After merging the drm-misc tree, today's linux-next build (powerpc
>> allyesconfig) failed like this:
>>
>> drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
>> drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of
>> function 'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
>> [-Werror=implicit-function-declaration]
>> 756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
>> | ^~~
>> | pgprot_cached
>> drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
>> assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
>>
>> Presumably exposed by commit
>>
>> a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
>>
>> I just turned off COMPILE_TEST again for today. Please let me know when
>> this is fixed.
>
> This still appears to have not been addressed.
Sorry for the delay, I've just posted a patch (also included below):
"[PATCH] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y &&
PPC32=n"
which should fix it.
Please verify it, thank you!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
From: Bartlomiej Zolnierkiewicz
Subject: [PATCH] video: fbdev: controlfb: fix build for COMPILE_TEST=y &&
PPC_PMAC=y && PPC32=n
powerpc allyesconfig fails like this:
drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of function
'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
[-Werror=implicit-function-declaration]
756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
| ^~~
| pgprot_cached
drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
Fix it by adding missing PPC32 dependency.
Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
Reported-by: Stephen Rothwell
Reported-by: kbuild test robot
Cc: Sam Ravnborg
Cc: Daniel Vetter
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/video/fbdev/controlfb.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: b/drivers/video/fbdev/controlfb.c
===
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -47,7 +47,7 @@
#include
#include
#include
-#ifdef CONFIG_PPC_PMAC
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
#include
#include
#endif
@@ -55,7 +55,7 @@
#include "macmodes.h"
#include "controlfb.h"
-#ifndef CONFIG_PPC_PMAC
+#if !defined(CONFIG_PPC_PMAC) || !defined(CONFIG_PPC32)
#define invalid_vram_cache(addr)
#undef in_8
#undef out_8
[PATCH] video: fbdev: controlfb: fix build for COMPILE_TEST=y && PPC_PMAC=y && PPC32=n
powerpc allyesconfig fails like this:
drivers/video/fbdev/controlfb.c: In function 'controlfb_mmap':
drivers/video/fbdev/controlfb.c:756:23: error: implicit declaration of function
'pgprot_cached_wthru'; did you mean 'pgprot_cached'?
[-Werror=implicit-function-declaration]
756 | vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
| ^~~
| pgprot_cached
drivers/video/fbdev/controlfb.c:756:23: error: incompatible types when
assigning to type 'pgprot_t' {aka 'struct '} from type 'int'
Fix it by adding missing PPC32 dependency.
Fixes: a07a63b0e24d ("video: fbdev: controlfb: add COMPILE_TEST support")
Reported-by: Stephen Rothwell
Reported-by: kbuild test robot
Cc: Sam Ravnborg
Cc: Daniel Vetter
Signed-off-by: Bartlomiej Zolnierkiewicz
---
drivers/video/fbdev/controlfb.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: b/drivers/video/fbdev/controlfb.c
===
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -47,7 +47,7 @@
#include
#include
#include
-#ifdef CONFIG_PPC_PMAC
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32)
#include
#include
#endif
@@ -55,7 +55,7 @@
#include "macmodes.h"
#include "controlfb.h"
-#ifndef CONFIG_PPC_PMAC
+#if !defined(CONFIG_PPC_PMAC) || !defined(CONFIG_PPC32)
#define invalid_vram_cache(addr)
#undef in_8
#undef out_8
Re: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator_enable()
On 10/8/19 5:48 PM, Mark Brown wrote:
> On Tue, Oct 08, 2019 at 03:24:17PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
>> Commit 7f93ff73f7c8 ("opp: core: add regulators enable and disable")
>> currently can be safely reverted as all affected users use always-on
>> regulators. However IMHO it should be possible to enable always-on
>> regulator without side-effects.
>
> With coupled regulators you might have something kicking in because a
> change was made on a completely different regulator... If we don't take
> account of coupling requirements we'd doubtless have issues with that at
> some point.
OK, I have not considered this.
>> When it comes to setting regulator constraints before doing enable
>> operation, it also seems to be possible solution but would require
>> splitting regulator_set_voltage() operation on two functions:
>
>> - one for setting constraints (before regulator_enable() operation)
>
>> - the other one actually setting voltage (after enable operation)
>
> I don't follow? What would a "constraint" be in this context and how
> would it be different to the voltage range you'd set in normal operation?
The constraint here would be just the voltage range. I just wanted to
point out that the actual voltage set operation (on the hardware itself
not the internal subsystem bookkeeping) shouldn't be done before enable
operation (especially in context of non-coupled regulators).
Taking into account your remark about enable operation on coupled
regulators and Dmitry's mail about cpufreq issue I think now that just
dropping opp change is the most straightforward fix.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH] regulator: core: Skip balancing of the enabled regulators in regulator_enable()
On 10/8/19 2:47 PM, Mark Brown wrote:
> On Tue, Oct 08, 2019 at 02:38:55PM +0200, Marek Szyprowski wrote:
>
>> Then if I get it right, the issue is caused by the commit 7f93ff73f7c8
>> ("opp: core: add regulators enable and disable"). I've checked and
>> indeed reverting it fixes Peach Pi to boot properly. The question is if
>> this is desired behavior or not?
>
> That doesn't seem ideal - either it's redundant for regulators that need
> to be marked as always-on anyway or it's going to force the regulators
> on when a device could do runtime PM (eg, if the same code can run on
> something like a GPU which can be turned off while the screen is off or
> is displaying a static image).
Commit 7f93ff73f7c8 ("opp: core: add regulators enable and disable")
currently can be safely reverted as all affected users use always-on
regulators. However IMHO it should be possible to enable always-on
regulator without side-effects.
When it comes to setting regulator constraints before doing enable
operation, it also seems to be possible solution but would require
splitting regulator_set_voltage() operation on two functions:
- one for setting constraints (before regulator_enable() operation)
- the other one actually setting voltage (after enable operation)
Unfortunately this is much bigger task and doesn't seem to be -rc
time material so I'm in favor of just applying Marek's fix as it is
for now.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v7] ata/pata_buddha: Probe via modalias instead of initcall
On 8/23/19 12:49 PM, Max Staudt wrote:
> Up until now, the pata_buddha driver would only check for cards on
> initcall time. Now, the kernel will call its probe function as soon
> as a compatible card is detected.
>
> v7: Removed suppress_bind_attrs that slipped in
>
> v6: Only do the drvdata workaround for X-Surf (remove breaks otherwise)
> Style
>
> v5: Remove module_exit(): There's no good way to handle the X-Surf hack.
> Also include a workaround to save X-Surf's drvdata in case zorro8390
> is active.
>
> v4: Clean up pata_buddha_probe() by using ent->driver_data.
> Support X-Surf via late_initcall()
>
> v3: Clean up devm_*, implement device removal.
>
> v2: Rename 'zdev' to 'z' to make the patch easy to analyse with
> git diff --ignore-space-change
>
> Signed-off-by: Max Staudt
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/pata_buddha.c | 228
> +++---
> 1 file changed, 135 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/ata/pata_buddha.c b/drivers/ata/pata_buddha.c
> index 11a8044ff..27d4c417f 100644
> --- a/drivers/ata/pata_buddha.c
> +++ b/drivers/ata/pata_buddha.c
> @@ -18,7 +18,9 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> @@ -29,7 +31,7 @@
> #include
>
> #define DRV_NAME "pata_buddha"
> -#define DRV_VERSION "0.1.0"
> +#define DRV_VERSION "0.1.1"
>
> #define BUDDHA_BASE1 0x800
> #define BUDDHA_BASE2 0xa00
> @@ -47,11 +49,11 @@ enum {
> BOARD_XSURF
> };
>
> -static unsigned int buddha_bases[3] __initdata = {
> +static unsigned int buddha_bases[3] = {
> BUDDHA_BASE1, BUDDHA_BASE2, BUDDHA_BASE3
> };
>
> -static unsigned int xsurf_bases[2] __initdata = {
> +static unsigned int xsurf_bases[2] = {
> XSURF_BASE1, XSURF_BASE2
> };
>
> @@ -145,111 +147,151 @@ static struct ata_port_operations pata_xsurf_ops = {
> .set_mode = pata_buddha_set_mode,
> };
>
> -static int __init pata_buddha_init_one(void)
> +static int pata_buddha_probe(struct zorro_dev *z,
> + const struct zorro_device_id *ent)
> {
> - struct zorro_dev *z = NULL;
> + static const char * const board_name[] = {
> + "Buddha", "Catweasel", "X-Surf"
> + };
> + struct ata_host *host;
> + void __iomem *buddha_board;
> + unsigned long board;
> + unsigned int type = ent->driver_data;
> + unsigned int nr_ports = (type == BOARD_CATWEASEL) ? 3 : 2;
> + void *old_drvdata;
> + int i;
> +
> + dev_info(>dev, "%s IDE controller\n", board_name[type]);
> +
> + board = z->resource.start;
> +
> + if (type != BOARD_XSURF) {
> + if (!devm_request_mem_region(>dev,
> + board + BUDDHA_BASE1,
> + 0x800, DRV_NAME))
> + return -ENXIO;
> + } else {
> + if (!devm_request_mem_region(>dev,
> + board + XSURF_BASE1,
> + 0x1000, DRV_NAME))
> + return -ENXIO;
> + if (!devm_request_mem_region(>dev,
> + board + XSURF_BASE2,
> + 0x1000, DRV_NAME)) {
> + }
> + }
> +
> + /* Workaround for X-Surf: Save drvdata in case zorro8390 has set it */
> + if (type == BOARD_XSURF)
> + old_drvdata = dev_get_drvdata(>dev);
> +
> + /* allocate host */
> + host = ata_host_alloc(>dev, nr_ports);
> + if (type == BOARD_XSURF)
> + dev_set_drvdata(>dev, old_drvdata);
> + if (!host)
> + return -ENXIO;
> +
> + buddha_board = ZTWO_VADDR(board);
> +
> + /* enable the board IRQ on Buddha/Catweasel */
> + if (type != BOARD_XSURF)
> + z_writeb(0, buddha_board + BUDDHA_IRQ_MR);
>
> - while ((z = zorro_find_device(ZORRO_WILDCARD, z))) {
> - static const char *board_name[]
> - = { "Buddha", "Catweasel", "X-Surf" };
> - struct ata_host *host;
> - void __iomem *buddha_board;
> - unsigned long board;
> - unsigned int type, nr_ports = 2;
> - int i;
> -
> - if (z->id == ZORRO_P
Re: [PATCH v6] ata/pata_buddha: Probe via modalias instead of initcall
Hi,
On 8/20/19 6:57 PM, Max Staudt wrote:
> Up until now, the pata_buddha driver would only check for cards on
> initcall time. Now, the kernel will call its probe function as soon
> as a compatible card is detected.
>
> v6: Only do the drvdata workaround for X-Surf (remove breaks otherwise)
> Style
>
> v5: Remove module_exit(): There's no good way to handle the X-Surf hack.
> Also include a workaround to save X-Surf's drvdata in case zorro8390
> is active.
>
> v4: Clean up pata_buddha_probe() by using ent->driver_data.
> Support X-Surf via late_initcall()
>
> v3: Clean up devm_*, implement device removal.
>
> v2: Rename 'zdev' to 'z' to make the patch easy to analyse with
> git diff --ignore-space-change
>
> Signed-off-by: Max Staudt
> ---
> drivers/ata/pata_buddha.c | 231
> +++---
> 1 file changed, 138 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/ata/pata_buddha.c b/drivers/ata/pata_buddha.c
> index 11a8044ff..9e1b57866 100644
> --- a/drivers/ata/pata_buddha.c
> +++ b/drivers/ata/pata_buddha.c
[...]
> +static struct zorro_driver pata_buddha_driver = {
> + .name = "pata_buddha",
> + .id_table = pata_buddha_zorro_tbl,
> + .probe = pata_buddha_probe,
> + .remove = pata_buddha_remove,
> + .driver = {
> + .suppress_bind_attrs = true,
I thought that we had agreed that this is not needed?
With that fixed:
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
On 8/21/19 2:16 PM, Krzysztof Kozlowski wrote:
> On Wed, 21 Aug 2019 at 13:51, Bartlomiej Zolnierkiewicz
> wrote:
>>>>> Following this change, I am now seeing the above error on our Tegra
>>>>> boards where this driver is enabled. This is triggering a kernel
>>>>> warnings test we have to fail. Hence, I don't think that you can remove
>>>>> the compatible node test here, unless you have a better way to determine
>>>>> if this is a samsung device.
>>>>
>>>> Right, this is really wrong... I missed that it is not a probe but
>>>> early init. And this init will be called on every board... Probably it
>>>> should be converted to a regular driver.
>>
>> Early initialization is needed for SoC driver to be used from within
>> arch/arm/mach-exynos/ and _initcall() usage is the usual way for SoC
>> drivers to be initialized:
>>
>> drivers/soc/amlogic/meson-gx-socinfo.c
>> drivers/soc/amlogic/meson-mx-socinfo.c
>> drivers/soc/atmel/soc.c
>> drivers/soc/bcm/brcmstb/common.c
>> drivers/soc/imx/soc-imx-scu.c
>> drivers/soc/imx/soc-imx8.c
>> drivers/soc/renesas/renesas-soc.c
>> drivers/soc/tegra/fuse/fuse-tegra.c
>> drivers/soc/ux500/ux500-soc-id.c
>> drivers/soc/versatile/soc-integrator.c
>> drivers/soc/versatile/soc-integrator.c
>>
>> The only SoC drivers that are regular drivers are:
>>
>> drivers/soc/fsl/guts.c
>> drivers/soc/versatile/soc-realview.c
>
> Thanks for pointing it out.
>
> Indeed, the initcall was needed in your set of patches here:
> https://patchwork.kernel.org/project/linux-samsung-soc/list/?series=43565=*
> but this work was not continued here. Maybe it will be later
> resubmitted... maybe not... who knows? Therefore I would prefer proper
The work got delayed mainly because of the request for the formal
audit of each usage vs cache coherency. Since it is rather small
cleanup and such audit is time consuming it became a low priority.
> solution for current case (driver), unless patches for mach are being
> brought back to life now.
>
>>> I'm also inclined to have it converted to a regular driver. We already
>>> have "exynos-asv" driver matching on the chipid node (patch 3/9).
>>> The ASV patches will not be merged soon anyway, all this needs some more
>>> thought. Krzysztof, can we abandon the chipid patches for now? Your
>>
>> chipid driver is good and useful on its own. The preferred solution
>> IMHO would be to just revert "soc: samsung: Convert exynos-chipid
>> driver to use the regmap API" commit.
Or just fix it by re-adding removed Exynos chipid compatible checking:
diff --git a/drivers/soc/samsung/exynos-chipid.c
b/drivers/soc/samsung/exynos-chipid.c
index 006a95feb618..d9912bd52479 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -55,6 +55,11 @@ int __init exynos_chipid_early_init(void)
u32 revision;
int ret;
+ /* look up for chipid node */
+ np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
+ if (!np)
+ return -ENODEV;
+
regmap =
syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid");
if (IS_ERR(regmap)) {
pr_err("Failed to get CHIPID regmap\n");
> I queued the chipid as a dependency for ASV but ASV requires the
> regmap. What would be left after reverting the regmap part? Simple
> unused printk driver? No need for such. If reverting, then let's drop
It provides sysfs information about SoC/platform and is useful on its
own (for debugging, reporting etc. purposes). Maybe not terrible useful
but on OTOH the driver is only ~100 LOC.
> entire driver and rework it offline.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v3 2/9] soc: samsung: Convert exynos-chipid driver to use the regmap API
Hi,
On 8/20/19 11:38 PM, Sylwester Nawrocki wrote:
> On 8/20/19 21:37, Krzysztof Kozlowski wrote:
>>>> diff --git a/drivers/soc/samsung/exynos-chipid.c
>>>> b/drivers/soc/samsung/exynos-chipid.c
>
>>>> @@ -51,29 +48,24 @@ static const char * __init
>>>> product_id_to_soc_id(unsigned int product_id)
>>>> int __init exynos_chipid_early_init(void)
>>>> {
>>>>struct soc_device_attribute *soc_dev_attr;
>>>> - void __iomem *exynos_chipid_base;
>>>>struct soc_device *soc_dev;
>>>>struct device_node *root;
>>>> - struct device_node *np;
>>>> + struct regmap *regmap;
>>>>u32 product_id;
>>>>u32 revision;
>>>> + int ret;
>>>>
>>>> - /* look up for chipid node */
>>>> - np = of_find_compatible_node(NULL, NULL,
>>>> "samsung,exynos4210-chipid");
>>>> - if (!np)
>>>> - return -ENODEV;
>>>> -
>>>> - exynos_chipid_base = of_iomap(np, 0);
>>>> - of_node_put(np);
>>>> -
>>>> - if (!exynos_chipid_base) {
>>>> - pr_err("Failed to map SoC chipid\n");
>>>> - return -ENXIO;
>>>> + regmap =
>>>> syscon_regmap_lookup_by_compatible("samsung,exynos4210-chipid");
>>>> + if (IS_ERR(regmap)) {
>>>> + pr_err("Failed to get CHIPID regmap\n");
>>>> + return PTR_ERR(regmap);
>>>>}
>>> Following this change, I am now seeing the above error on our Tegra
>>> boards where this driver is enabled. This is triggering a kernel
>>> warnings test we have to fail. Hence, I don't think that you can remove
>>> the compatible node test here, unless you have a better way to determine
>>> if this is a samsung device.
>>
>> Right, this is really wrong... I missed that it is not a probe but
>> early init. And this init will be called on every board... Probably it
>> should be converted to a regular driver.
Early initialization is needed for SoC driver to be used from within
arch/arm/mach-exynos/ and _initcall() usage is the usual way for SoC
drivers to be initialized:
drivers/soc/amlogic/meson-gx-socinfo.c
drivers/soc/amlogic/meson-mx-socinfo.c
drivers/soc/atmel/soc.c
drivers/soc/bcm/brcmstb/common.c
drivers/soc/imx/soc-imx-scu.c
drivers/soc/imx/soc-imx8.c
drivers/soc/renesas/renesas-soc.c
drivers/soc/tegra/fuse/fuse-tegra.c
drivers/soc/ux500/ux500-soc-id.c
drivers/soc/versatile/soc-integrator.c
drivers/soc/versatile/soc-integrator.c
The only SoC drivers that are regular drivers are:
drivers/soc/fsl/guts.c
drivers/soc/versatile/soc-realview.c
> I'm also inclined to have it converted to a regular driver. We already
> have "exynos-asv" driver matching on the chipid node (patch 3/9).
> The ASV patches will not be merged soon anyway, all this needs some more
> thought. Krzysztof, can we abandon the chipid patches for now? Your
chipid driver is good and useful on its own. The preferred solution
IMHO would be to just revert "soc: samsung: Convert exynos-chipid
driver to use the regmap API" commit.
> pull request doesn't appear to be merged to arm-soc yet. Sorry about
> that.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v5] ata/pata_buddha: Probe via modalias instead of initcall
On 8/20/19 6:42 PM, Max Staudt wrote:
> Hi Bartlomiej,
>
> On 08/20/2019 02:06 PM, Bartlomiej Zolnierkiewicz wrote:
>> WARNING: line over 80 characters
>> #354: FILE: drivers/ata/pata_buddha.c:287:
>> +while ((z =
>> zorro_find_device(ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF, z))) {
>
> I see no good way to shorten this one. I think it's obvious enough to allow
> overflowing by a few chars - do you agree?
Yes, I agree.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v5] ata/pata_buddha: Probe via modalias instead of initcall
On 8/20/19 5:59 PM, Max Staudt wrote:
> Hi Bartlomiej,
>
> Thank you very much for your review!
>
> Question below.
>
>
> On 08/20/2019 02:06 PM, Bartlomiej Zolnierkiewicz wrote:
>>> + /* Workaround for X-Surf: Save drvdata in case zorro8390 has set it */
>>> + old_drvdata = dev_get_drvdata(>dev);
>>
>> This should be done only for type == BOARD_XSURF.
>
> Agreed, as I want to keep unloading functional for Buddha/Catweasel - see
> below.
>
>
>>> +static struct zorro_driver pata_buddha_driver = {
>>> + .name = "pata_buddha",
>>> + .id_table = pata_buddha_zorro_tbl,
>>> + .probe = pata_buddha_probe,
>>> + .remove = pata_buddha_remove,
>>
>> I think that we should also add:
>>
>> .driver = {
>> .suppress_bind_attrs = true,
>> },
>>
>> to prevent the device from being unbinded (and thus ->remove called)
>> from the driver using sysfs interface.
>
> Interesting idea - here's my question now:
>
> My intention is to allow remove() for boards where we support IDE only
> (Buddha, Catweasel) - these are autoprobed via zorro_register_driver().
> This shouldn't affect the X-Surf case, as it's not autoprobed in this way
> anyway - and thus pata_buddha_driver isn't even used.
>
> Am I missing something? We want to inhibit module unloading (hence no
> module_exit()), but driver unbinding for Buddha/Catweasel should be fine to
> remain, right?
Indeed, pata_buddha_driver is not even used for X-Surf so this is not
an issue (please disregard my comment about suppress_bind_attrs).
>> Please also always check your patches with scripts/checkpatch.pl and
>> fix the reported issues:
>
> Apologies, must've been something in my coffee. I will.
>
>
> Thanks for the review, I'll send a new patch once my question above is
> resolved.
>
> Max
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 4/9] drivers: ata: ahci_xgene: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
> ---
> drivers/ata/ahci_xgene.c | 21 +++--
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
> index 16246c8..5391f5d 100644
> --- a/drivers/ata/ahci_xgene.c
> +++ b/drivers/ata/ahci_xgene.c
> @@ -739,7 +739,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
> struct device *dev = >dev;
> struct ahci_host_priv *hpriv;
> struct xgene_ahci_context *ctx;
> - struct resource *res;
> const struct of_device_id *of_devid;
> enum xgene_ahci_version version = XGENE_AHCI_V1;
> const struct ata_port_info *ppi[] = { _ahci_v1_port_info,
> @@ -759,31 +758,25 @@ static int xgene_ahci_probe(struct platform_device
> *pdev)
> ctx->dev = dev;
>
> /* Retrieve the IP core resource */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - ctx->csr_core = devm_ioremap_resource(dev, res);
> + ctx->csr_core = devm_platform_ioremap_resource(pdev, 1);
> if (IS_ERR(ctx->csr_core))
> return PTR_ERR(ctx->csr_core);
>
> /* Retrieve the IP diagnostic resource */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> - ctx->csr_diag = devm_ioremap_resource(dev, res);
> + ctx->csr_diag = devm_platform_ioremap_resource(pdev, 2);
> if (IS_ERR(ctx->csr_diag))
> return PTR_ERR(ctx->csr_diag);
>
> /* Retrieve the IP AXI resource */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> - ctx->csr_axi = devm_ioremap_resource(dev, res);
> + ctx->csr_axi = devm_platform_ioremap_resource(pdev, 3);
> if (IS_ERR(ctx->csr_axi))
> return PTR_ERR(ctx->csr_axi);
>
> /* Retrieve the optional IP mux resource */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 4);
> - if (res) {
> - void __iomem *csr = devm_ioremap_resource(dev, res);
> - if (IS_ERR(csr))
> - return PTR_ERR(csr);
> -
> - ctx->csr_mux = csr;
> + ctx->csr_mux = csr = devm_platform_ioremap_resource(pdev, 4);
This conversion is incorrect - despite the resource being optional
we will get error message from devm_[platform_]ioremap_resource()
(it always prints an error on !res condition).
> + if (IS_ERR(ctx->csr_mux)) {
> + dev_info(>dev, "cant get ip mux resource (optional)");
No need for this message.
> + ctx->csr_mux = NULL;
> }
>
> of_devid = of_match_device(xgene_ahci_of_match, dev);
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 3/9] drivers: ata: ahci_tegra: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
> ---
> drivers/ata/ahci_tegra.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> index e3163da..3845c23 100644
> --- a/drivers/ata/ahci_tegra.c
> +++ b/drivers/ata/ahci_tegra.c
> @@ -481,7 +481,6 @@ static int tegra_ahci_probe(struct platform_device *pdev)
> {
> struct ahci_host_priv *hpriv;
> struct tegra_ahci_priv *tegra;
> - struct resource *res;
> int ret;
> unsigned int i;
>
> @@ -498,19 +497,17 @@ static int tegra_ahci_probe(struct platform_device
> *pdev)
> tegra->pdev = pdev;
> tegra->soc = of_device_get_match_data(>dev);
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - tegra->sata_regs = devm_ioremap_resource(>dev, res);
> + tegra->sata_regs = devm_platform_ioremap_resource(pdev, 1);
> if (IS_ERR(tegra->sata_regs))
> return PTR_ERR(tegra->sata_regs);
>
> /*
>* AUX registers is optional.
>*/
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> - if (res) {
> - tegra->sata_aux_regs = devm_ioremap_resource(>dev, res);
> - if (IS_ERR(tegra->sata_aux_regs))
> - return PTR_ERR(tegra->sata_aux_regs);
> + tegra->sata_aux_regs = devm_platform_ioremap_resource(pdev, 2);
This conversion is incorrect - despite the resource being optional
we will get error message from devm_[platform_]ioremap_resource()
(it always prints an error on !res condition).
> + if (IS_ERR(tegra->sata_aux_regs)) {
> + dev_info(>dev, "Cant get aux registers (optional)");
No need for this message.
> + tegra->sata_aux_regs = NULL;
> }
>
> tegra->sata_rst = devm_reset_control_get(>dev, "sata");
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 9/9] drivers: ata: sata_rcar: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/sata_rcar.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
> index 3495e17..14ea1d6 100644
> --- a/drivers/ata/sata_rcar.c
> +++ b/drivers/ata/sata_rcar.c
> @@ -886,7 +886,6 @@ static int sata_rcar_probe(struct platform_device *pdev)
> struct device *dev = >dev;
> struct ata_host *host;
> struct sata_rcar_priv *priv;
> - struct resource *mem;
> int irq;
> int ret = 0;
>
> @@ -915,8 +914,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
>
> host->private_data = priv;
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - priv->base = devm_ioremap_resource(dev, mem);
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(priv->base)) {
> ret = PTR_ERR(priv->base);
> goto err_pm_put;
Re: [PATCH 8/9] drivers: ata: sata_gemini: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote: > Use the new helper that wraps the calls to platform_get_resource() > and devm_ioremap_resource() together. It would also be worth to mention in the patch description that on !res condition the driver will now return -EINVAL (instead of -ENODEV) and print an error. > Signed-off-by: Enrico Weigelt, metux IT consult Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/ata/sata_gemini.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/ata/sata_gemini.c b/drivers/ata/sata_gemini.c > index f793564..55e2689 100644 > --- a/drivers/ata/sata_gemini.c > +++ b/drivers/ata/sata_gemini.c > @@ -318,7 +318,6 @@ static int gemini_sata_probe(struct platform_device *pdev) > struct device_node *np = dev->of_node; > struct sata_gemini *sg; > struct regmap *map; > - struct resource *res; > enum gemini_muxmode muxmode; > u32 gmode; > u32 gmask; > @@ -329,11 +328,7 @@ static int gemini_sata_probe(struct platform_device > *pdev) > return -ENOMEM; > sg->dev = dev; > > - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - if (!res) > - return -ENODEV; > - > - sg->base = devm_ioremap_resource(dev, res); > + sg->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(sg->base)) > return PTR_ERR(sg->base);
Re: [PATCH 7/9] drivers: ata: sata_dwc_460ex: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/sata_dwc_460ex.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 9dcef6a..de248fa 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -237,7 +237,6 @@ static int sata_dwc_dma_init_old(struct platform_device
> *pdev,
>struct sata_dwc_device *hsdev)
> {
> struct device_node *np = pdev->dev.of_node;
> - struct resource *res;
>
> hsdev->dma = devm_kzalloc(>dev, sizeof(*hsdev->dma), GFP_KERNEL);
> if (!hsdev->dma)
> @@ -254,8 +253,7 @@ static int sata_dwc_dma_init_old(struct platform_device
> *pdev,
> }
>
> /* Get physical SATA DMA register base address */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - hsdev->dma->regs = devm_ioremap_resource(>dev, res);
> + hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
> if (IS_ERR(hsdev->dma->regs))
> return PTR_ERR(hsdev->dma->regs);
Re: [PATCH 6/9] drivers: ata: pata_bk3710: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/pata_bk3710.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/ata/pata_bk3710.c b/drivers/ata/pata_bk3710.c
> index fad95cf..92b036d 100644
> --- a/drivers/ata/pata_bk3710.c
> +++ b/drivers/ata/pata_bk3710.c
> @@ -291,7 +291,6 @@ static void pata_bk3710_chipinit(void __iomem *base)
> static int __init pata_bk3710_probe(struct platform_device *pdev)
> {
> struct clk *clk;
> - struct resource *mem;
> struct ata_host *host;
> struct ata_port *ap;
> void __iomem *base;
> @@ -310,15 +309,13 @@ static int __init pata_bk3710_probe(struct
> platform_device *pdev)
> /* NOTE: round *down* to meet minimum timings; we count in clocks */
> ideclk_period = 10UL / rate;
>
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> pr_err(DRV_NAME ": failed to get IRQ resource\n");
> return irq;
> }
>
> - base = devm_ioremap_resource(>dev, mem);
> + base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(base))
> return PTR_ERR(base);
Re: [PATCH 5/9] drivers: ata: libahci_platform: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/libahci_platform.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 9e9583a..3d84be8 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -408,8 +408,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct
> platform_device *pdev,
>
> devres_add(dev, hpriv);
>
> - hpriv->mmio = devm_ioremap_resource(dev,
> - platform_get_resource(pdev, IORESOURCE_MEM, 0));
> + hpriv->mmio = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(hpriv->mmio)) {
> rc = PTR_ERR(hpriv->mmio);
> goto err_out;
Re: [PATCH 2/9] drivers: ata: ahci_seattle: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote: > Use the new helper that wraps the calls to platform_get_resource() > and devm_ioremap_resource() together. > > Signed-off-by: Enrico Weigelt, metux IT consult Acked-by: Bartlomiej Zolnierkiewicz Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/ata/ahci_seattle.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/ata/ahci_seattle.c b/drivers/ata/ahci_seattle.c > index ced12705..2d88d36 100644 > --- a/drivers/ata/ahci_seattle.c > +++ b/drivers/ata/ahci_seattle.c > @@ -132,8 +132,7 @@ static const struct ata_port_info > *ahci_seattle_get_port_info( > if (!plat_data) > return _port_info; > > - plat_data->sgpio_ctrl = devm_ioremap_resource(dev, > - platform_get_resource(pdev, IORESOURCE_MEM, 1)); > + plat_data->sgpio_ctrl = devm_platform_ioremap_resource(pdev, 1); > if (IS_ERR(plat_data->sgpio_ctrl)) > return _port_info;
Re: [PATCH 1/9] drivers: ata: ahci_octeon: use devm_platform_ioremap_resource()
On 8/20/19 2:35 PM, Enrico Weigelt, metux IT consult wrote:
> Use the new helper that wraps the calls to platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Enrico Weigelt, metux IT consult
Acked-by: Bartlomiej Zolnierkiewicz
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
> ---
> drivers/ata/ahci_octeon.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/ata/ahci_octeon.c b/drivers/ata/ahci_octeon.c
> index 5a44e08..2280180 100644
> --- a/drivers/ata/ahci_octeon.c
> +++ b/drivers/ata/ahci_octeon.c
> @@ -32,13 +32,11 @@ static int ahci_octeon_probe(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct device_node *node = dev->of_node;
> - struct resource *res;
> void __iomem *base;
> u64 cfg;
> int ret;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - base = devm_ioremap_resource(>dev, res);
> + base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(base))
> return PTR_ERR(base);
Re: [PATCH v5] ata/pata_buddha: Probe via modalias instead of initcall
= base + 2 + 1 * 4;
> + ap->ioaddr.nsect_addr = base + 2 + 2 * 4;
> + ap->ioaddr.lbal_addr= base + 2 + 3 * 4;
> + ap->ioaddr.lbam_addr= base + 2 + 4 * 4;
> + ap->ioaddr.lbah_addr= base + 2 + 5 * 4;
> + ap->ioaddr.device_addr = base + 2 + 6 * 4;
> + ap->ioaddr.status_addr = base + 2 + 7 * 4;
> + ap->ioaddr.command_addr = base + 2 + 7 * 4;
> +
> + if (ctl) {
> + ap->ioaddr.altstatus_addr = base + ctl;
> + ap->ioaddr.ctl_addr = base + ctl;
> }
>
> - ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
> - IRQF_SHARED, _buddha_sht);
> + ap->private_data = (void *)irqport;
>
> + ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", board,
> + ctl ? board + buddha_bases[i] + ctl : 0);
> }
>
> + ata_host_activate(host, IRQ_AMIGA_PORTS, ata_sff_interrupt,
> + IRQF_SHARED, _buddha_sht);
> +
> +
> return 0;
> }
>
> -module_init(pata_buddha_init_one);
> +static void pata_buddha_remove(struct zorro_dev *z)
> +{
> + struct ata_host *host = dev_get_drvdata(>dev);
> +
> + ata_host_detach(host);
> +}
> +
> +static const struct zorro_device_id pata_buddha_zorro_tbl[] = {
> + { ZORRO_PROD_INDIVIDUAL_COMPUTERS_BUDDHA, BOARD_BUDDHA},
> + { ZORRO_PROD_INDIVIDUAL_COMPUTERS_CATWEASEL, BOARD_CATWEASEL},
> + /* { ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF, BOARD_XSURF}, */
Commented out code should be removed.
> + { 0 }
> +};
> +
Extra newline, please remove it.
> +MODULE_DEVICE_TABLE(zorro, pata_buddha_zorro_tbl);
> +
> +static struct zorro_driver pata_buddha_driver = {
> + .name = "pata_buddha",
> + .id_table = pata_buddha_zorro_tbl,
> + .probe = pata_buddha_probe,
> + .remove = pata_buddha_remove,
I think that we should also add:
.driver = {
.suppress_bind_attrs = true,
},
to prevent the device from being unbinded (and thus ->remove called)
from the driver using sysfs interface.
> +};
> +
Extra newline, please remove it.
> +
ditto
> +
> +/*
> + * We cannot have a modalias for X-Surf boards, as it competes with the
> + * zorro8390 network driver. As a stopgap measure until we have proper
> + * MFC support for this board, we manually attach to it late after Zorro
s/MFC/MFD/
> + * has enumerated its boards.
> + */
> +static int __init pata_buddha_late_init(void)
> +{
> +struct zorro_dev *z = NULL;
> +
> + pr_info("pata_buddha: Scanning for stand-alone IDE controllers...\n");
I think that there is no need for being extra verbose,
please remove it.
> + zorro_register_driver(_buddha_driver);
> +
> + pr_info("pata_buddha: Scanning for X-Surf boards...\n");
ditto
> +while ((z =
> zorro_find_device(ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF, z))) {
> + static struct zorro_device_id xsurf_ent =
> + { ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF, BOARD_XSURF};
> +
> + pata_buddha_probe(z, _ent);
> +}
> +
> +return 0;
> +}
> +
> +late_initcall(pata_buddha_late_init);
> +
Extra newline, please remove it.
>
> MODULE_AUTHOR("Bartlomiej Zolnierkiewicz");
> MODULE_DESCRIPTION("low-level driver for Buddha/Catweasel/X-Surf PATA");
Please also always check your patches with scripts/checkpatch.pl and
fix the reported issues:
ERROR: code indent should use tabs where possible
#348: FILE: drivers/ata/pata_buddha.c:281:
+struct zorro_dev *z = NULL;$
WARNING: please, no spaces at the start of a line
#348: FILE: drivers/ata/pata_buddha.c:281:
+struct zorro_dev *z = NULL;$
WARNING: line over 80 characters
#354: FILE: drivers/ata/pata_buddha.c:287:
+while ((z = zorro_find_device(ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF,
z))) {
ERROR: code indent should use tabs where possible
#354: FILE: drivers/ata/pata_buddha.c:287:
+while ((z = zorro_find_device(ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF,
z))) {$
WARNING: please, no spaces at the start of a line
#354: FILE: drivers/ata/pata_buddha.c:287:
+while ((z = zorro_find_device(ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF,
z))) {$
ERROR: that open brace { should be on the previous line
#356: FILE: drivers/ata/pata_buddha.c:289:
+ static struct zorro_device_id xsurf_ent =
+ { ZORRO_PROD_INDIVIDUAL_COMPUTERS_X_SURF, BOARD_XSURF};
ERROR: code indent should use tabs where possible
#359: FILE: drivers/ata/pata_buddha.c:292:
+}$
WARNING: please, no spaces at the start of a line
#359: FILE: drivers/ata/pata_buddha.c:292:
+}$
ERROR: code indent should use tabs where possible
#361: FILE: drivers/ata/pata_buddha.c:294:
+return 0;$
WARNING: please, no spaces at the start of a line
#361: FILE: drivers/ata/pata_buddha.c:294:
+return 0;$
total: 5 errors, 6 warnings, 276 lines checked
Otherwise the patch looks fine.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH 6/7] efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
On 8/11/19 5:08 PM, Denis Efremov wrote:
> This patch refactors the loop condition scheme from
> 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
>
> Signed-off-by: Denis Efremov
Acked-by: Bartlomiej Zolnierkiewicz
> ---
> drivers/video/fbdev/efifb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 04a22663b4fb..6c72b825e92a 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -668,7 +668,7 @@ static void efifb_fixup_resources(struct pci_dev *dev)
> if (!base)
> return;
>
> - for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
> + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> struct resource *res = >resource[i];
>
> if (!(res->flags & IORESOURCE_MEM))
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
Re: [PATCH v4] ata/pata_buddha: Probe via modalias instead of initcall
On 8/12/19 4:26 PM, Max Staudt wrote: > On 08/12/2019 02:15 PM, Bartlomiej Zolnierkiewicz wrote: >>> What's a good way to do that, given that we now have module_exit()> defined >>> and an exit function is void? >> >> What about something like this: >> >> static bool xsurf_present; >> ... >> static int __init pata_buddha_late_init(void) >> ... >> if (pata_buddha_probe(z, _ent) == 0 && >> xsurf_present == false) >> xsurf_present = true; >> ... >> static void __exit pata_buddha_exit(void) >> ... >> if (xsurf_present) >> return -EBUSY; >> ... >> >> ? > > Okay, so we're talking about the same idea. Great! > > Unfortunately, pata_buddha_exit() is void, and thus can't fail. According to > Documentation/kernel-hacking/hacking.rst this is by design. You are of course right and the example code is broken (+ I need more caffeine). > Any other ideas? We could also continue to disallow unloading completely > until MFD support comes along. Yes, this would also be OK. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics
Re: [PATCH v4] ata/pata_buddha: Probe via modalias instead of initcall
On 8/12/19 12:55 PM, Max Staudt wrote:
> Hi Bartlomiej,
>
> Thanks for your feedback!
Hi Max,
> On 08/12/2019 12:42 PM, Bartlomiej Zolnierkiewicz wrote:
>>
>> ide/buddha driver cannot be unloaded currently (it lacks module_exit()).
>>
>> [... snip ...]
>>
>> It should work exactly like the old code in case of X-Surf,
>> what do we need to release?
>
>
> So what shall I do? Once an X-Surf has been detected, we refuse to
> unload, and therefore we never have to release X-Surf resources?
> That would simplify things a lot.
Yes, it seems to be a simplest solution.
> What's a good way to do that, given that we now have module_exit()> defined
> and an exit function is void?
What about something like this:
static bool xsurf_present;
...
static int __init pata_buddha_late_init(void)
...
if (pata_buddha_probe(z, _ent) == 0 &&
xsurf_present == false)
xsurf_present = true;
...
static void __exit pata_buddha_exit(void)
...
if (xsurf_present)
return -EBUSY;
...
?
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics
