Re: [PATCH v2 02/24] drivers/video/fbdev: use ioremap_wc/wt() instead of __ioremap()

2018-09-13 Thread Bartlomiej Zolnierkiewicz


On 09/12/2018 05:58 PM, Christophe Leroy wrote:
> _PAGE_NO_CACHE is a platform specific flag. In addition, this flag
> is misleading because one would think it requests a noncached page
> whereas a noncached page is _PAGE_NO_CACHE | _PAGE_GUARDED
> 
> _PAGE_NO_CACHE alone means write combined noncached page, so lets
> use ioremap_wc() instead.
> 
> _PAGE_WRITETHRU is also platform specific flag. Use ioremap_wt()
> instead.
> 
> Signed-off-by: Christophe Leroy 

After reading patch #1 this one LGTM.

Acked-by: Bartlomiej Zolnierkiewicz 

> ---
>  drivers/video/fbdev/chipsfb.c|  3 +--
>  drivers/video/fbdev/controlfb.c  |  5 +
>  drivers/video/fbdev/platinumfb.c |  5 +
>  drivers/video/fbdev/valkyriefb.c | 12 ++--
>  4 files changed, 9 insertions(+), 16 deletions(-)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Re: [PATCH v2 02/24] drivers/video/fbdev: use ioremap_wc/wt() instead of __ioremap()

2018-09-13 Thread Daniel Vetter
On Wed, Sep 12, 2018 at 03:58:17PM +, Christophe Leroy wrote:
> _PAGE_NO_CACHE is a platform specific flag. In addition, this flag
> is misleading because one would think it requests a noncached page
> whereas a noncached page is _PAGE_NO_CACHE | _PAGE_GUARDED
> 
> _PAGE_NO_CACHE alone means write combined noncached page, so lets
> use ioremap_wc() instead.
> 
> _PAGE_WRITETHRU is also platform specific flag. Use ioremap_wt()
> instead.
> 
> Signed-off-by: Christophe Leroy 

I wondered why this all showed up in my gfx inbox, then spotted this patch
here.

I trust you way more on the _PAGE_ flags than me, but this looks
reasonable.

Acked-by: Daniel Vetter 

> ---
>  drivers/video/fbdev/chipsfb.c|  3 +--
>  drivers/video/fbdev/controlfb.c  |  5 +
>  drivers/video/fbdev/platinumfb.c |  5 +
>  drivers/video/fbdev/valkyriefb.c | 12 ++--
>  4 files changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
> index f103665cad43..40182ed85648 100644
> --- a/drivers/video/fbdev/chipsfb.c
> +++ b/drivers/video/fbdev/chipsfb.c
> @@ -27,7 +27,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  
>  #ifdef CONFIG_PMAC_BACKLIGHT
>  #include 
> @@ -401,7 +400,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const 
> struct pci_device_id *ent)
>  #endif /* CONFIG_PMAC_BACKLIGHT */
>  
>  #ifdef CONFIG_PPC
> - p->screen_base = __ioremap(addr, 0x20, _PAGE_NO_CACHE);
> + p->screen_base = ioremap_wc(addr, 0x20);
>  #else
>   p->screen_base = ioremap(addr, 0x20);
>  #endif
> diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
> index 8d14b29aafea..9cb0ef7ac29e 100644
> --- a/drivers/video/fbdev/controlfb.c
> +++ b/drivers/video/fbdev/controlfb.c
> @@ -48,9 +48,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> -#include 
>  #include 
>  
>  #include "macmodes.h"
> @@ -715,8 +713,7 @@ static int __init control_of_init(struct device_node *dp)
>   goto error_out;
>   }
>   /* map at most 8MB for the frame buffer */
> - p->frame_buffer = __ioremap(p->frame_buffer_phys, 0x80,
> - _PAGE_WRITETHRU);
> + p->frame_buffer = ioremap_wt(p->frame_buffer_phys, 0x80);
>  
>   if (!p->control_regs_phys ||
>   !request_mem_region(p->control_regs_phys, p->control_regs_size,
> diff --git a/drivers/video/fbdev/platinumfb.c 
> b/drivers/video/fbdev/platinumfb.c
> index 377d3399a3ad..bf6b7fb83cf4 100644
> --- a/drivers/video/fbdev/platinumfb.c
> +++ b/drivers/video/fbdev/platinumfb.c
> @@ -32,9 +32,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> -#include 
>  
>  #include "macmodes.h"
>  #include "platinumfb.h"
> @@ -577,8 +575,7 @@ static int platinumfb_probe(struct platform_device* odev)
>  
>   /* frame buffer - map only 4MB */
>   pinfo->frame_buffer_phys = pinfo->rsrc_fb.start;
> - pinfo->frame_buffer = __ioremap(pinfo->rsrc_fb.start, 0x40,
> - _PAGE_WRITETHRU);
> + pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x40);
>   pinfo->base_frame_buffer = pinfo->frame_buffer;
>  
>   /* registers */
> diff --git a/drivers/video/fbdev/valkyriefb.c 
> b/drivers/video/fbdev/valkyriefb.c
> index 275fb98236d3..d51c3a8009cb 100644
> --- a/drivers/video/fbdev/valkyriefb.c
> +++ b/drivers/video/fbdev/valkyriefb.c
> @@ -54,13 +54,11 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #ifdef CONFIG_MAC
>  #include 
>  #else
>  #include 
>  #endif
> -#include 
>  
>  #include "macmodes.h"
>  #include "valkyriefb.h"
> @@ -318,7 +316,7 @@ static void __init valkyrie_choose_mode(struct 
> fb_info_valkyrie *p)
>  int __init valkyriefb_init(void)
>  {
>   struct fb_info_valkyrie *p;
> - unsigned long frame_buffer_phys, cmap_regs_phys, flags;
> + unsigned long frame_buffer_phys, cmap_regs_phys;
>   int err;
>   char *option = NULL;
>  
> @@ -337,7 +335,6 @@ int __init valkyriefb_init(void)
>   /* Hardcoded addresses... welcome to 68k Macintosh country :-) */
>   frame_buffer_phys = 0xf900;
>   cmap_regs_phys = 0x50f24000;
> - flags = IOMAP_NOCACHE_SER; /* IOMAP_WRITETHROUGH?? */
>  #else /* ppc (!CONFIG_MAC) */
>   {
>   struct device_node *dp;
> @@ -354,7 +351,6 @@ int __init valkyriefb_init(void)
>  
>   frame_buffer_phys = r.start;
>   cmap_regs_phys = r.start + 0x304000;
> - flags = _PAGE_WRITETHRU;
>   }
>  #endif /* ppc (!CONFIG_MAC) */
>  
> @@ -369,7 +365,11 @@ int __init valkyriefb_init(void)
>   }
>   p->total_vram = 0x10;
>   p->frame_buffer_phys = frame_buffer_phys;
> - p->frame_buffer = __ioremap(frame_buffer_phys, p->total_vram, flags);
> +#ifdef CONFIG_MAC
> + p->frame_buffer = ioremap_nocache(frame_buffer_phys, p->total_vram);
> +#else
> + p->frame_buffer = ioremap_wt(frame_buff

[PATCH v2 02/24] drivers/video/fbdev: use ioremap_wc/wt() instead of __ioremap()

2018-09-12 Thread Christophe Leroy
_PAGE_NO_CACHE is a platform specific flag. In addition, this flag
is misleading because one would think it requests a noncached page
whereas a noncached page is _PAGE_NO_CACHE | _PAGE_GUARDED

_PAGE_NO_CACHE alone means write combined noncached page, so lets
use ioremap_wc() instead.

_PAGE_WRITETHRU is also platform specific flag. Use ioremap_wt()
instead.

Signed-off-by: Christophe Leroy 
---
 drivers/video/fbdev/chipsfb.c|  3 +--
 drivers/video/fbdev/controlfb.c  |  5 +
 drivers/video/fbdev/platinumfb.c |  5 +
 drivers/video/fbdev/valkyriefb.c | 12 ++--
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index f103665cad43..40182ed85648 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -27,7 +27,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef CONFIG_PMAC_BACKLIGHT
 #include 
@@ -401,7 +400,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const 
struct pci_device_id *ent)
 #endif /* CONFIG_PMAC_BACKLIGHT */
 
 #ifdef CONFIG_PPC
-   p->screen_base = __ioremap(addr, 0x20, _PAGE_NO_CACHE);
+   p->screen_base = ioremap_wc(addr, 0x20);
 #else
p->screen_base = ioremap(addr, 0x20);
 #endif
diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
index 8d14b29aafea..9cb0ef7ac29e 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -48,9 +48,7 @@
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 #include 
 
 #include "macmodes.h"
@@ -715,8 +713,7 @@ static int __init control_of_init(struct device_node *dp)
goto error_out;
}
/* map at most 8MB for the frame buffer */
-   p->frame_buffer = __ioremap(p->frame_buffer_phys, 0x80,
-   _PAGE_WRITETHRU);
+   p->frame_buffer = ioremap_wt(p->frame_buffer_phys, 0x80);
 
if (!p->control_regs_phys ||
!request_mem_region(p->control_regs_phys, p->control_regs_size,
diff --git a/drivers/video/fbdev/platinumfb.c b/drivers/video/fbdev/platinumfb.c
index 377d3399a3ad..bf6b7fb83cf4 100644
--- a/drivers/video/fbdev/platinumfb.c
+++ b/drivers/video/fbdev/platinumfb.c
@@ -32,9 +32,7 @@
 #include 
 #include 
 #include 
-#include 
 #include 
-#include 
 
 #include "macmodes.h"
 #include "platinumfb.h"
@@ -577,8 +575,7 @@ static int platinumfb_probe(struct platform_device* odev)
 
/* frame buffer - map only 4MB */
pinfo->frame_buffer_phys = pinfo->rsrc_fb.start;
-   pinfo->frame_buffer = __ioremap(pinfo->rsrc_fb.start, 0x40,
-   _PAGE_WRITETHRU);
+   pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x40);
pinfo->base_frame_buffer = pinfo->frame_buffer;
 
/* registers */
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 275fb98236d3..d51c3a8009cb 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -54,13 +54,11 @@
 #include 
 #include 
 #include 
-#include 
 #ifdef CONFIG_MAC
 #include 
 #else
 #include 
 #endif
-#include 
 
 #include "macmodes.h"
 #include "valkyriefb.h"
@@ -318,7 +316,7 @@ static void __init valkyrie_choose_mode(struct 
fb_info_valkyrie *p)
 int __init valkyriefb_init(void)
 {
struct fb_info_valkyrie *p;
-   unsigned long frame_buffer_phys, cmap_regs_phys, flags;
+   unsigned long frame_buffer_phys, cmap_regs_phys;
int err;
char *option = NULL;
 
@@ -337,7 +335,6 @@ int __init valkyriefb_init(void)
/* Hardcoded addresses... welcome to 68k Macintosh country :-) */
frame_buffer_phys = 0xf900;
cmap_regs_phys = 0x50f24000;
-   flags = IOMAP_NOCACHE_SER; /* IOMAP_WRITETHROUGH?? */
 #else /* ppc (!CONFIG_MAC) */
{
struct device_node *dp;
@@ -354,7 +351,6 @@ int __init valkyriefb_init(void)
 
frame_buffer_phys = r.start;
cmap_regs_phys = r.start + 0x304000;
-   flags = _PAGE_WRITETHRU;
}
 #endif /* ppc (!CONFIG_MAC) */
 
@@ -369,7 +365,11 @@ int __init valkyriefb_init(void)
}
p->total_vram = 0x10;
p->frame_buffer_phys = frame_buffer_phys;
-   p->frame_buffer = __ioremap(frame_buffer_phys, p->total_vram, flags);
+#ifdef CONFIG_MAC
+   p->frame_buffer = ioremap_nocache(frame_buffer_phys, p->total_vram);
+#else
+   p->frame_buffer = ioremap_wt(frame_buffer_phys, p->total_vram);
+#endif
p->cmap_regs_phys = cmap_regs_phys;
p->cmap_regs = ioremap(p->cmap_regs_phys, 0x1000);
p->valkyrie_regs_phys = cmap_regs_phys+0x6000;
-- 
2.13.3