2007-05-16 21:36:13

by Antonino A. Daplas

[permalink] [raw]
Subject: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

Move arch-specific bits of fb_mmap() to their respective subdirectories

Signed-off-by: Antonino Daplas <[email protected]>
---
Hi all,

Please let me know if there are mistakes with this patch.

Thanks.

Tony

drivers/video/fbmem.c | 65 +++++---------------------------------------
include/asm-alpha/fb.h | 7 +++++
include/asm-arm/fb.h | 12 ++++++++
include/asm-arm26/fb.h | 7 +++++
include/asm-avr32/fb.h | 14 +++++++++
include/asm-blackfin/fb.h | 7 +++++
include/asm-cris/fb.h | 7 +++++
include/asm-frv/fb.h | 7 +++++
include/asm-h8300/fb.h | 7 +++++
include/asm-i386/fb.h | 13 +++++++++
include/asm-ia64/fb.h | 15 ++++++++++
include/asm-m32r/fb.h | 12 ++++++++
include/asm-m68k/fb.h | 28 +++++++++++++++++++
include/asm-m68knommu/fb.h | 7 +++++
include/asm-mips/fb.h | 12 ++++++++
include/asm-parisc/fb.h | 12 ++++++++
include/asm-powerpc/fb.h | 14 +++++++++
include/asm-ppc/fb.h | 14 +++++++++
include/asm-s390/fb.h | 7 +++++
include/asm-sh/fb.h | 12 ++++++++
include/asm-sh64/fb.h | 12 ++++++++
include/asm-sparc/fb.h | 6 ++++
include/asm-sparc64/fb.h | 15 ++++++++++
include/asm-v850/fb.h | 7 +++++
include/asm-x86_64/fb.h | 13 +++++++++
include/asm-xtensa/fb.h | 7 +++++
include/linux/fb.h | 12 ++++++++
27 files changed, 293 insertions(+), 58 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 38c2e25..68d60b7 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -33,17 +33,10 @@ #endif
#include <linux/err.h>
#include <linux/device.h>
#include <linux/efi.h>
+#include <linux/fb.h>

-#if defined(__mc68000__) || defined(CONFIG_APUS)
-#include <asm/setup.h>
-#endif
-
-#include <asm/io.h>
-#include <asm/uaccess.h>
-#include <asm/page.h>
-#include <asm/pgtable.h>
+#include <asm/fb.h>

-#include <linux/fb.h>

/*
* Frame buffer device initialization and setup routines
@@ -1155,17 +1148,15 @@ fb_compat_ioctl(struct file *file, unsig
}
#endif

-static int
+static int
fb_mmap(struct file *file, struct vm_area_struct * vma)
{
int fbidx = iminor(file->f_path.dentry->d_inode);
struct fb_info *info = registered_fb[fbidx];
struct fb_ops *fb = info->fbops;
unsigned long off;
-#if !defined(__sparc__) || defined(__sparc_v9__)
unsigned long start;
u32 len;
-#endif

if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
return -EINVAL;
@@ -1180,12 +1171,9 @@ #endif
return res;
}

-#if defined(__sparc__) && !defined(__sparc_v9__)
- /* Should never get here, all fb drivers should have their own
- mmap routines */
- return -EINVAL;
-#else
- /* !sparc32... */
+ if (fb_mmap_required())
+ return -EINVAL;
+
lock_kernel();

/* frame buffer memory */
@@ -1209,50 +1197,11 @@ #else
vma->vm_pgoff = off >> PAGE_SHIFT;
/* This is an IO map - tell maydump to skip this VMA */
vma->vm_flags |= VM_IO | VM_RESERVED;
-#if defined(__mc68000__)
-#if defined(CONFIG_SUN3)
- pgprot_val(vma->vm_page_prot) |= SUN3_PAGE_NOCACHE;
-#elif defined(CONFIG_MMU)
- if (CPU_IS_020_OR_030)
- pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE030;
- if (CPU_IS_040_OR_060) {
- pgprot_val(vma->vm_page_prot) &= _CACHEMASK040;
- /* Use no-cache mode, serialized */
- pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE_S;
- }
-#endif
-#elif defined(__powerpc__)
- vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
- vma->vm_end - vma->vm_start,
- vma->vm_page_prot);
-#elif defined(__alpha__)
- /* Caching is off in the I/O space quadrant by design. */
-#elif defined(__i386__) || defined(__x86_64__)
- if (boot_cpu_data.x86 > 3)
- pgprot_val(vma->vm_page_prot) |= _PAGE_PCD;
-#elif defined(__mips__) || defined(__sparc_v9__)
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-#elif defined(__hppa__)
- pgprot_val(vma->vm_page_prot) |= _PAGE_NO_CACHE;
-#elif defined(__arm__) || defined(__sh__) || defined(__m32r__)
- vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
-#elif defined(__avr32__)
- vma->vm_page_prot = __pgprot((pgprot_val(vma->vm_page_prot)
- & ~_PAGE_CACHABLE)
- | (_PAGE_BUFFER | _PAGE_DIRTY));
-#elif defined(__ia64__)
- if (efi_range_is_wc(vma->vm_start, vma->vm_end - vma->vm_start))
- vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
- else
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-#else
-#warning What do we have to do here??
-#endif
+ fb_pgprotect(file, vma);
if (io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT,
vma->vm_end - vma->vm_start, vma->vm_page_prot))
return -EAGAIN;
return 0;
-#endif /* !sparc32 */
}

static int
diff --git a/include/asm-alpha/fb.h b/include/asm-alpha/fb.h
new file mode 100644
index 0000000..ca714a4
--- /dev/null
+++ b/include/asm-alpha/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+/* Caching is off in the I/O space quadrant by design. */
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-arm/fb.h b/include/asm-arm/fb.h
new file mode 100644
index 0000000..067b599
--- /dev/null
+++ b/include/asm-arm/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-arm26/fb.h b/include/asm-arm26/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-arm26/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-avr32/fb.h b/include/asm-avr32/fb.h
new file mode 100644
index 0000000..710255a
--- /dev/null
+++ b/include/asm-avr32/fb.h
@@ -0,0 +1,14 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = __pgprot((pgprot_val(vma->vm_page_prot)
+ & ~_PAGE_CACHABLE)
+ | (_PAGE_BUFFER | _PAGE_DIRTY));
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-blackfin/fb.h b/include/asm-blackfin/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-blackfin/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-cris/fb.h b/include/asm-cris/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-cris/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-frv/fb.h b/include/asm-frv/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-frv/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-h8300/fb.h b/include/asm-h8300/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-h8300/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-i386/fb.h b/include/asm-i386/fb.h
new file mode 100644
index 0000000..ff83013
--- /dev/null
+++ b/include/asm-i386/fb.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ if (boot_cpu_data.x86 > 3)
+ pgprot_val(vma->vm_page_prot) |= _PAGE_PCD;
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-ia64/fb.h b/include/asm-ia64/fb.h
new file mode 100644
index 0000000..31b9743
--- /dev/null
+++ b/include/asm-ia64/fb.h
@@ -0,0 +1,15 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ if (efi_range_is_wc(vma->vm_start, vma->vm_end - vma->vm_start))
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+ else
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-m32r/fb.h b/include/asm-m32r/fb.h
new file mode 100644
index 0000000..067b599
--- /dev/null
+++ b/include/asm-m32r/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-m68k/fb.h b/include/asm-m68k/fb.h
new file mode 100644
index 0000000..7d4a28f
--- /dev/null
+++ b/include/asm-m68k/fb.h
@@ -0,0 +1,28 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+#include <asm/setup.h>
+
+#ifdef CONFIG_SUN3
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ pgprot_val(vma->vm_page_prot) |= SUN3_PAGE_NOCACHE;
+}
+#elif defined(CONFIG_MMU)
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+ if (CPU_IS_020_OR_030)
+ pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE030;
+ if (CPU_IS_040_OR_060) {
+ pgprot_val(vma->vm_page_prot) &= _CACHEMASK040;
+ /* Use no-cache mode, serialized */
+ pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE_S;
+ }
+}
+#else
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+#endif /* CONFIG_SUN3 */
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-m68knommu/fb.h b/include/asm-m68knommu/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-m68knommu/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-mips/fb.h b/include/asm-mips/fb.h
new file mode 100644
index 0000000..dab64a8
--- /dev/null
+++ b/include/asm-mips/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-parisc/fb.h b/include/asm-parisc/fb.h
new file mode 100644
index 0000000..a9c2f67
--- /dev/null
+++ b/include/asm-parisc/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ pgprot_val(vma->vm_page_prot) |= _PAGE_NO_CACHE;
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-powerpc/fb.h b/include/asm-powerpc/fb.h
new file mode 100644
index 0000000..32eb8b7
--- /dev/null
+++ b/include/asm-powerpc/fb.h
@@ -0,0 +1,14 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+ vma->vm_end - vma->vm_start,
+ vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-ppc/fb.h b/include/asm-ppc/fb.h
new file mode 100644
index 0000000..32eb8b7
--- /dev/null
+++ b/include/asm-ppc/fb.h
@@ -0,0 +1,14 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+ vma->vm_end - vma->vm_start,
+ vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-s390/fb.h b/include/asm-s390/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-s390/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-sh/fb.h b/include/asm-sh/fb.h
new file mode 100644
index 0000000..067b599
--- /dev/null
+++ b/include/asm-sh/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-sh64/fb.h b/include/asm-sh64/fb.h
new file mode 100644
index 0000000..067b599
--- /dev/null
+++ b/include/asm-sh64/fb.h
@@ -0,0 +1,12 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-sparc/fb.h b/include/asm-sparc/fb.h
new file mode 100644
index 0000000..9f8c88d
--- /dev/null
+++ b/include/asm-sparc/fb.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-sparc64/fb.h b/include/asm-sparc64/fb.h
new file mode 100644
index 0000000..01b50b2
--- /dev/null
+++ b/include/asm-sparc64/fb.h
@@ -0,0 +1,15 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#if defined(__sparc_v9__)
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+}
+#else
+#define fb_pgprotect(...) do {} while (0)
+#endif /* defined(__sparc_v9__) */
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-v850/fb.h b/include/asm-v850/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-v850/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-x86_64/fb.h b/include/asm-x86_64/fb.h
new file mode 100644
index 0000000..ff83013
--- /dev/null
+++ b/include/asm-x86_64/fb.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#include <linux/fs.h>
+#include <asm/page.h>
+
+static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
+{
+ if (boot_cpu_data.x86 > 3)
+ pgprot_val(vma->vm_page_prot) |= _PAGE_PCD;
+}
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/asm-xtensa/fb.h b/include/asm-xtensa/fb.h
new file mode 100644
index 0000000..dc9d1be
--- /dev/null
+++ b/include/asm-xtensa/fb.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_FB_H_
+#define _ASM_FB_H_
+
+#warning What do we have to do here??
+#define fb_pgprotect(...) do {} while (0)
+
+#endif /* _ASM_FB_H_ */
diff --git a/include/linux/fb.h b/include/linux/fb.h
index c654d0e..0e929c3 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -1064,6 +1064,18 @@ extern int fb_find_mode(struct fb_var_sc
const struct fb_videomode *default_mode,
unsigned int default_bpp);

+#if defined(__sparc__) && !defined(__sparc_v9__)
+static inline int fb_mmap_required(void)
+{
+ return 1;
+}
+#else
+static inline int fb_mmap_required(void)
+{
+ return 0;
+}
+#endif
+
#endif /* __KERNEL__ */

#endif /* _LINUX_FB_H */



2007-05-16 21:53:18

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

From: "Antonino A. Daplas" <[email protected]>
Date: Thu, 17 May 2007 05:33:48 +0800

> Move arch-specific bits of fb_mmap() to their respective subdirectories
>
> Signed-off-by: Antonino Daplas <[email protected]>
> ---
> Hi all,
>
> Please let me know if there are mistakes with this patch.

Thank you for doing this work.

__sparc_v9__ means CONFIG_SPARC64, if that is not defined it's
CONFIG_SPARC32, so you could simplify this patch a lot.

1) Remove __sparc_v9__ ifdefs from asm-sparc64/fb.h
2) Define fb_mmap_required() to return 0 in asm-sparc64/fb.h
3) Define fb_mmap_required() to return 1 in asm-sparc/fb.h
4) Remove all the sparc ifdef stuff you added to linux/fb.h
5) There won't be a default fb_mmap_required() provided by
linux/fb.h on platforms other than sparc ones, you'll need
to provide one in order for the build to work non-sparc since
this call is unconditional in fbmem.c:

+ if (fb_mmap_required())
+ return -EINVAL;

With those fixes:

Signed-off-by: David S. Miller <[email protected]>

2007-05-16 22:59:22

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Wed, 2007-05-16 at 14:53 -0700, David Miller wrote:
> From: "Antonino A. Daplas" <[email protected]>
> Date: Thu, 17 May 2007 05:33:48 +0800
>
> > Move arch-specific bits of fb_mmap() to their respective subdirectories
> >
> > Signed-off-by: Antonino Daplas <[email protected]>
> > ---
> > Hi all,
> >
> > Please let me know if there are mistakes with this patch.
>
> Thank you for doing this work.
>
> __sparc_v9__ means CONFIG_SPARC64, if that is not defined it's
> CONFIG_SPARC32, so you could simplify this patch a lot.
>
> 1) Remove __sparc_v9__ ifdefs from asm-sparc64/fb.h
> 2) Define fb_mmap_required() to return 0 in asm-sparc64/fb.h
> 3) Define fb_mmap_required() to return 1 in asm-sparc/fb.h
> 4) Remove all the sparc ifdef stuff you added to linux/fb.h
> 5) There won't be a default fb_mmap_required() provided by
> linux/fb.h on platforms other than sparc ones, you'll need
> to provide one in order for the build to work non-sparc since
> this call is unconditional in fbmem.c:
>
> + if (fb_mmap_required())
> + return -EINVAL;
>
> With those fixes:
>

Okay, I see what you mean. I will concoct another patch later.

Tony


2007-05-17 06:13:33

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Wed, 2007-05-16 at 19:54 -0500, Kumar Gala wrote:
> On May 16, 2007, at 4:33 PM, Antonino A. Daplas wrote:
>
> > Move arch-specific bits of fb_mmap() to their respective
> > subdirectories
> >

> You don't need a version in asm-ppc since you have one in asm-
> powerpc. We are slowly phasing out arch/ppc in favor of arch/powerpc
> which merges the 32 and 64-bit support.
>

Okay, thanks.

Tony

2007-05-17 09:43:46

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Thu, 16 May 2007, Antonino A. Daplas wrote:
> diff --git a/include/asm-m68k/fb.h b/include/asm-m68k/fb.h
> new file mode 100644
> index 0000000..7d4a28f
> --- /dev/null
> +++ b/include/asm-m68k/fb.h
> @@ -0,0 +1,28 @@
> +#ifndef _ASM_FB_H_
> +#define _ASM_FB_H_
> +
> +#include <linux/fs.h>
> +#include <asm/page.h>
> +#include <asm/setup.h>
> +
> +#ifdef CONFIG_SUN3
> +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
> +{
> + pgprot_val(vma->vm_page_prot) |= SUN3_PAGE_NOCACHE;
> +}
> +#elif defined(CONFIG_MMU)
^^^^^^^^^^^^^^^^^^^^^^^^^
This can be a plain `#else', as m68knommu hasn't been merged with m68k
(yet :-)

> +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
> + if (CPU_IS_020_OR_030)
> + pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE030;
> + if (CPU_IS_040_OR_060) {
> + pgprot_val(vma->vm_page_prot) &= _CACHEMASK040;
> + /* Use no-cache mode, serialized */
> + pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE_S;
> + }
> +}
> +#else
^^^^^
So this branch can go away

> +#warning What do we have to do here??
> +#define fb_pgprotect(...) do {} while (0)
> +#endif /* CONFIG_SUN3 */
> +
> +#endif /* _ASM_FB_H_ */
> diff --git a/include/asm-m68knommu/fb.h b/include/asm-m68knommu/fb.h
> new file mode 100644
> index 0000000..dc9d1be
> --- /dev/null
> +++ b/include/asm-m68knommu/fb.h
> @@ -0,0 +1,7 @@
> +#ifndef _ASM_FB_H_
> +#define _ASM_FB_H_
> +
> +#warning What do we have to do here??
> +#define fb_pgprotect(...) do {} while (0)
> +
> +#endif /* _ASM_FB_H_ */

If the m68knommu case in fb_mmap() was empty before, it should be empty here
too, so I guess the warning can go away.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2007-05-17 09:49:49

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Thu, May 17, 2007 at 05:33:48AM +0800, Antonino A. Daplas wrote:
> -#if defined(__sparc__) && !defined(__sparc_v9__)
> - /* Should never get here, all fb drivers should have their own
> - mmap routines */
> - return -EINVAL;
> -#else
> - /* !sparc32... */
> + if (fb_mmap_required())
> + return -EINVAL;
> +

I don't like this bit. It's just that all sparc frame buffers happen
to have a mmap method but there's nothing fundamentally arch specific
here. You should be able to plug a pci video card into any linux
supported machine provided the fb driver does the full setup (which
only very few do in practice, but..)

So I'd just require drivers to have a fb_mmap method and make fb_mmap
the default one. The page protection cleanup is still nice of course,
but it might make sense to give it a more generic name as it's not
really fb specific.

2007-05-17 10:51:39

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Thu, 2007-05-17 at 10:49 +0100, Christoph Hellwig wrote:
> On Thu, May 17, 2007 at 05:33:48AM +0800, Antonino A. Daplas wrote:
> > -#if defined(__sparc__) && !defined(__sparc_v9__)
> > - /* Should never get here, all fb drivers should have their own
> > - mmap routines */
> > - return -EINVAL;
> > -#else
> > - /* !sparc32... */
> > + if (fb_mmap_required())
> > + return -EINVAL;
> > +
>
> I don't like this bit.

Me too, actually. I did it this way because I don't want to deviate from
the old code too much. Since all sparc32 drivers are either linked to
sbuslib.c or has their own fb_mmap method, the above test is redundant.
I'll leave the decision to remove the above test to David Miller.

> It's just that all sparc frame buffers happen
> to have a mmap method but there's nothing fundamentally arch specific
> here. You should be able to plug a pci video card into any linux
> supported machine provided the fb driver does the full setup (which
> only very few do in practice, but..)
>
> So I'd just require drivers to have a fb_mmap method and make fb_mmap
> the default one.

This one is a tad bit more complicated, so I'll leave the present setup
for now. Once the arch-specific parts are properly separated, maybe I'll
take that step.

> The page protection cleanup is still nice of course,
> but it might make sense to give it a more generic name as it's not
> really fb specific.

Any suggestion for a name? io_pgprotect() perhaps?

Tony


2007-05-17 10:52:49

by Antonino A. Daplas

[permalink] [raw]
Subject: Re: [Linux-fbdev-devel] [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

On Thu, 2007-05-17 at 11:43 +0200, Geert Uytterhoeven wrote:
> On Thu, 16 May 2007, Antonino A. Daplas wrote:
> > diff --git a/include/asm-m68k/fb.h b/include/asm-m68k/fb.h
> > new file mode 100644
> > index 0000000..7d4a28f
> > --- /dev/null
> > +++ b/include/asm-m68k/fb.h
> > @@ -0,0 +1,28 @@
> > +#ifndef _ASM_FB_H_
> > +#define _ASM_FB_H_
> > +
> > +#include <linux/fs.h>
> > +#include <asm/page.h>
> > +#include <asm/setup.h>
> > +
> > +#ifdef CONFIG_SUN3
> > +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
> > +{
> > + pgprot_val(vma->vm_page_prot) |= SUN3_PAGE_NOCACHE;
> > +}
> > +#elif defined(CONFIG_MMU)
> ^^^^^^^^^^^^^^^^^^^^^^^^^
> This can be a plain `#else', as m68knommu hasn't been merged with m68k
> (yet :-)
>
> > +static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma)
> > + if (CPU_IS_020_OR_030)
> > + pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE030;
> > + if (CPU_IS_040_OR_060) {
> > + pgprot_val(vma->vm_page_prot) &= _CACHEMASK040;
> > + /* Use no-cache mode, serialized */
> > + pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE_S;
> > + }
> > +}
> > +#else
> ^^^^^
> So this branch can go away
>
> > +#warning What do we have to do here??
> > +#define fb_pgprotect(...) do {} while (0)
> > +#endif /* CONFIG_SUN3 */
> > +
> > +#endif /* _ASM_FB_H_ */
> > diff --git a/include/asm-m68knommu/fb.h b/include/asm-m68knommu/fb.h
> > new file mode 100644
> > index 0000000..dc9d1be
> > --- /dev/null
> > +++ b/include/asm-m68knommu/fb.h
> > @@ -0,0 +1,7 @@
> > +#ifndef _ASM_FB_H_
> > +#define _ASM_FB_H_
> > +
> > +#warning What do we have to do here??
> > +#define fb_pgprotect(...) do {} while (0)
> > +
> > +#endif /* _ASM_FB_H_ */
>
> If the m68knommu case in fb_mmap() was empty before, it should be empty here
> too, so I guess the warning can go away.

Thanks. It's confusing when there's multiple directories for one
architecture.

Tony


2007-05-17 20:30:38

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 10/10] [HEADS UP] fbdev: Move arch-specific bits to their respective subdirectories

From: "Antonino A. Daplas" <[email protected]>
Date: Thu, 17 May 2007 18:51:22 +0800

> On Thu, 2007-05-17 at 10:49 +0100, Christoph Hellwig wrote:
> > On Thu, May 17, 2007 at 05:33:48AM +0800, Antonino A. Daplas wrote:
> > > -#if defined(__sparc__) && !defined(__sparc_v9__)
> > > - /* Should never get here, all fb drivers should have their own
> > > - mmap routines */
> > > - return -EINVAL;
> > > -#else
> > > - /* !sparc32... */
> > > + if (fb_mmap_required())
> > > + return -EINVAL;
> > > +
> >
> > I don't like this bit.
>
> Me too, actually. I did it this way because I don't want to deviate from
> the old code too much. Since all sparc32 drivers are either linked to
> sbuslib.c or has their own fb_mmap method, the above test is redundant.
> I'll leave the decision to remove the above test to David Miller.

Feel free to remove it.