| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200117193112.GC24812@ravnborg.org>
Date: Fri, 17 Jan 2020 20:31:12 +0100
From: Sam Ravnborg <sam@...nborg.org>
To: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Cc: linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/6] video: fbdev: controlfb: add COMPILE_TEST support
Hi Bartlomiej
On Thu, Jan 16, 2020 at 03:08:57PM +0100, Bartlomiej Zolnierkiewicz wrote:
> Add COMPILE_TEST support to controlfb driver for better compile
> testing coverage.
This is not a nice patch to add COMPILE_TEST support :-(
But I see why you do it.
I already spent too much time being side-tracked by this, but here are
some comments to consider.
With the comments considered:
Acked-by: Sam Ravnborg <sam@...nborg.org>
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> ---
> drivers/video/fbdev/Kconfig | 2 +-
> drivers/video/fbdev/controlfb.c | 21 +++++++++++++++++++--
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
> index aa9541bf964b..91c872457863 100644
> --- a/drivers/video/fbdev/Kconfig
> +++ b/drivers/video/fbdev/Kconfig
> @@ -472,7 +472,7 @@ config FB_OF
>
> config FB_CONTROL
> bool "Apple \"control\" display support"
> - depends on (FB = y) && PPC_PMAC && PPC32
> + depends on (FB = y) && ((PPC_PMAC && PPC32) || COMPILE_TEST)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
> index bd0f61d8bdb5..87cd817ad4c6 100644
> --- a/drivers/video/fbdev/controlfb.c
> +++ b/drivers/video/fbdev/controlfb.c
> @@ -47,12 +47,25 @@
> #include <linux/nvram.h>
> #include <linux/adb.h>
> #include <linux/cuda.h>
> +#ifdef CONFIG_PPC_PMAC
> #include <asm/prom.h>
> #include <asm/btext.h>
> +#endif
>
> #include "macmodes.h"
> #include "controlfb.h"
>
> +#ifndef CONFIG_PPC_PMAC
> +#undef in_8
> +#undef out_8
> +#undef in_le32
> +#undef out_le32
> +#define in_8(addr) 0
> +#define out_8(addr, val)
> +#define in_le32(addr) 0
> +#define out_le32(addr, val)
> +#endif
> +
> struct fb_par_control {
> int vmode, cmode;
> int xres, yres;
> @@ -278,7 +291,9 @@ static int controlfb_mmap(struct fb_info *info,
> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> } else {
> /* framebuffer */
> +#ifdef CONFIG_PPC_PMAC
> vma->vm_page_prot = pgprot_cached_wthru(vma->vm_page_prot);
> +#endif
Add:
#define pgprot_cached_wthru(x) 0
in the CONFIG_PPC_PMAC block?
> }
>
> return vm_iomap_memory(vma, start, len);
> @@ -582,13 +597,14 @@ static void __init find_vram_size(struct fb_info_control *p)
>
> out_8(&p->frame_buffer[0x600000], 0xb3);
> out_8(&p->frame_buffer[0x600001], 0x71);
> +#ifdef CONFIG_PPC_PMAC
> asm volatile("eieio; dcbf 0,%0" : : "r" (&p->frame_buffer[0x600000])
> : "memory" );
> mb();
> asm volatile("eieio; dcbi 0,%0" : : "r" (&p->frame_buffer[0x600000])
> : "memory" );
> mb();
> -
> +#endif
The inline asm block could be written as:
static void invalid_vram_cache(void * addr)
{
eieio();
dcbf(addr);
mb;
eieio();
dcbi(addr);
mb();
}
And then this inline function could be in the CONFIG_PPC_PMAC block -
and a dummy in the else part.
The function name is just my best guess what the assembler does.
> bank2 = (in_8(&p->frame_buffer[0x600000]) == 0xb3)
> && (in_8(&p->frame_buffer[0x600001]) == 0x71);
>
> @@ -601,13 +617,14 @@ static void __init find_vram_size(struct fb_info_control *p)
>
> out_8(&p->frame_buffer[0], 0x5a);
> out_8(&p->frame_buffer[1], 0xc7);
> +#ifdef CONFIG_PPC_PMAC
> asm volatile("eieio; dcbf 0,%0" : : "r" (&p->frame_buffer[0])
> : "memory" );
> mb();
> asm volatile("eieio; dcbi 0,%0" : : "r" (&p->frame_buffer[0])
> : "memory" );
> mb();
> -
> +#endif
Same here.
> bank1 = (in_8(&p->frame_buffer[0]) == 0x5a)
> && (in_8(&p->frame_buffer[1]) == 0xc7);
>
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Powered by blists - more mailing lists