[RFC v4 13/25] powerpc: Cleanup nvram includes

[RFC v4 13/25] powerpc: Cleanup nvram includes

[Finn Thain]

The nvram_read_byte() and nvram_write_byte() definitions in asm/nvram.h
duplicate those in linux/nvram.h. Get rid of the former to prepare for
adoption of struct arch_nvram_ops (which is defined in linux/nvram.h for
general use).

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 arch/powerpc/include/asm/nvram.h           |    3 ---
 arch/powerpc/kernel/setup_32.c             |    1 +
 drivers/char/generic_nvram.c               |    4 +++-
 drivers/video/fbdev/matrox/matroxfb_base.c |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

Index: linux/arch/powerpc/include/asm/nvram.h
=================================--- linux.orig/arch/powerpc/include/asm/nvram.h	2015-07-12 20:24:53.000000000 +1000
+++ linux/arch/powerpc/include/asm/nvram.h	2015-07-12 20:25:08.000000000 +1000
@@ -101,7 +101,4 @@ extern int nvram_write_os_partition(stru
 /* Determine NVRAM size */
 extern ssize_t nvram_get_size(void);
 
-/* Normal access to NVRAM */
-extern unsigned char nvram_read_byte(int i);
-extern void nvram_write_byte(unsigned char c, int i);
 #endif /* _ASM_POWERPC_NVRAM_H */
Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c	2015-07-12 20:25:08.000000000 +1000
@@ -16,6 +16,7 @@
 #include <linux/cpu.h>
 #include <linux/console.h>
 #include <linux/memblock.h>
+#include <linux/nvram.h>
 
 #include <asm/io.h>
 #include <asm/prom.h>
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/char/generic_nvram.c	2015-07-12 20:25:08.000000000 +1000
@@ -20,9 +20,11 @@
 #include <linux/fcntl.h>
 #include <linux/init.h>
 #include <linux/mutex.h>
+#include <linux/nvram.h>
 #include <asm/uaccess.h>
-#include <asm/nvram.h>
+
 #ifdef CONFIG_PPC_PMAC
+#include <asm/nvram.h>
 #include <asm/machdep.h>
 #endif
 
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-12 20:25:08.000000000 +1000
@@ -111,12 +111,12 @@
 #include "matroxfb_g450.h"
 #include <linux/matroxfb.h>
 #include <linux/interrupt.h>
+#include <linux/nvram.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 
 #ifdef CONFIG_PPC_PMAC
 #include <asm/machdep.h>
-unsigned char nvram_read_byte(int);
 static int default_vmode = VMODE_NVRAM;
 static int default_cmode = CMODE_NVRAM;
 #endif

[RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_wri

[Finn Thain]

Make use of arch_nvram_ops in device drivers so that the nvram_*
function exports can be removed.

Since they are no longer global symbols, rename the PPC32 nvram_* functions
appropriately.

Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 arch/powerpc/kernel/setup_32.c             |    8 ++++----
 drivers/char/generic_nvram.c               |    4 ++--
 drivers/video/fbdev/controlfb.c            |    4 ++--
 drivers/video/fbdev/imsttfb.c              |    7 +++----
 drivers/video/fbdev/matrox/matroxfb_base.c |    2 +-
 drivers/video/fbdev/platinumfb.c           |    4 ++--
 drivers/video/fbdev/valkyriefb.c           |    4 ++--
 7 files changed, 16 insertions(+), 17 deletions(-)

Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c	2015-07-12 20:25:11.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c	2015-07-12 20:25:13.000000000 +1000
@@ -170,20 +170,18 @@ __setup("l3cr=", ppc_setup_l3cr);
 
 #ifdef CONFIG_GENERIC_NVRAM
 
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
 {
 	if (ppc_md.nvram_read_val)
 		return ppc_md.nvram_read_val(addr);
 	return 0xff;
 }
-EXPORT_SYMBOL(nvram_read_byte);
 
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
 {
 	if (ppc_md.nvram_write_val)
 		ppc_md.nvram_write_val(addr, val);
 }
-EXPORT_SYMBOL(nvram_write_byte);
 
 static ssize_t ppc_nvram_get_size(void)
 {
@@ -200,6 +198,8 @@ static long ppc_nvram_sync(void)
 }
 
 const struct nvram_ops arch_nvram_ops = {
+	.read_byte      = ppc_nvram_read_byte,
+	.write_byte     = ppc_nvram_write_byte,
 	.get_size       = ppc_nvram_get_size,
 	.sync           = ppc_nvram_sync,
 };
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c	2015-07-12 20:25:11.000000000 +1000
+++ linux/drivers/char/generic_nvram.c	2015-07-12 20:25:13.000000000 +1000
@@ -64,7 +64,7 @@ static ssize_t read_nvram(struct file *f
 	if (*ppos >= nvram_len)
 		return 0;
 	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
-		if (__put_user(nvram_read_byte(i), p))
+		if (__put_user(arch_nvram_ops.read_byte(i), p))
 			return -EFAULT;
 	*ppos = i;
 	return p - buf;
@@ -84,7 +84,7 @@ static ssize_t write_nvram(struct file *
 	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
 		if (__get_user(c, p))
 			return -EFAULT;
-		nvram_write_byte(c, i);
+		arch_nvram_ops.write_byte(c, i);
 	}
 	*ppos = i;
 	return p - buf;
Index: linux/drivers/video/fbdev/controlfb.c
=================================--- linux.orig/drivers/video/fbdev/controlfb.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/video/fbdev/controlfb.c	2015-07-12 20:25:13.000000000 +1000
@@ -415,7 +415,7 @@ static int __init init_control(struct fb
 	/* Try to pick a video mode out of NVRAM if we have one. */
 #ifdef CONFIG_NVRAM
 	if (default_cmode = CMODE_NVRAM) {
-		cmode = nvram_read_byte(NV_CMODE);
+		cmode = arch_nvram_ops.read_byte(NV_CMODE);
 		if(cmode < CMODE_8 || cmode > CMODE_32)
 			cmode = CMODE_8;
 	} else
@@ -423,7 +423,7 @@ static int __init init_control(struct fb
 		cmodeÞfault_cmode;
 #ifdef CONFIG_NVRAM
 	if (default_vmode = VMODE_NVRAM) {
-		vmode = nvram_read_byte(NV_VMODE);
+		vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (vmode < 1 || vmode > VMODE_MAX ||
 		    control_mac_modes[vmode - 1].m[full] < cmode) {
 			sense = read_control_sense(p);
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-12 20:25:08.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-12 20:25:13.000000000 +1000
@@ -1888,7 +1888,7 @@ static int initMatrox2(struct matrox_fb_
 			default_vmode = VMODE_640_480_60;
 #ifdef CONFIG_NVRAM
 		if (default_cmode = CMODE_NVRAM)
-			default_cmode = nvram_read_byte(NV_CMODE);
+			default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 		if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
 			default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/platinumfb.c
=================================--- linux.orig/drivers/video/fbdev/platinumfb.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/video/fbdev/platinumfb.c	2015-07-12 20:25:13.000000000 +1000
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_in
 	printk(KERN_INFO "platinumfb: Monitor sense value = 0x%x, ", sense);
 	if (default_vmode = VMODE_NVRAM) {
 #ifdef CONFIG_NVRAM
-		default_vmode = nvram_read_byte(NV_VMODE);
+		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (default_vmode <= 0 || default_vmode > VMODE_MAX ||
 		    !platinum_reg_init[default_vmode-1])
 #endif
@@ -362,7 +362,7 @@ static int platinum_init_fb(struct fb_in
 		default_vmode = VMODE_640_480_60;
 #ifdef CONFIG_NVRAM
 	if (default_cmode = CMODE_NVRAM)
-		default_cmode = nvram_read_byte(NV_CMODE);
+		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 	if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
 		default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/valkyriefb.c
=================================--- linux.orig/drivers/video/fbdev/valkyriefb.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c	2015-07-12 20:25:13.000000000 +1000
@@ -287,7 +287,7 @@ static void __init valkyrie_choose_mode(
 	/* Try to pick a video mode out of NVRAM if we have one. */
 #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
 	if (default_vmode = VMODE_NVRAM) {
-		default_vmode = nvram_read_byte(NV_VMODE);
+		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (default_vmode <= 0
 		 || default_vmode > VMODE_MAX
 		 || !valkyrie_reg_init[default_vmode - 1])
@@ -300,7 +300,7 @@ static void __init valkyrie_choose_mode(
 		default_vmode = VMODE_640_480_67;
 #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
 	if (default_cmode = CMODE_NVRAM)
-		default_cmode = nvram_read_byte(NV_CMODE);
+		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 
 	/*
Index: linux/drivers/video/fbdev/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c	2015-07-12 20:24:53.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c	2015-07-12 20:25:13.000000000 +1000
@@ -328,7 +328,6 @@ enum {
 	TVP = 1
 };
 
-#define USE_NV_MODES		1
 #define INIT_BPP		8
 #define INIT_XRES		640
 #define INIT_YRES		480
@@ -1391,17 +1390,17 @@ static void init_imstt(struct fb_info *i
 		}
 	}
 
-#if USE_NV_MODES && defined(CONFIG_PPC32)
+#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
 	{
 		int vmode = init_vmode, cmode = init_cmode;
 
 		if (vmode = -1) {
-			vmode = nvram_read_byte(NV_VMODE);
+			vmode = arch_nvram_ops.read_byte(NV_VMODE);
 			if (vmode <= 0 || vmode > VMODE_MAX)
 				vmode = VMODE_640_480_67;
 		}
 		if (cmode = -1) {
-			cmode = nvram_read_byte(NV_CMODE);
+			cmode = arch_nvram_ops.read_byte(NV_CMODE);
 			if (cmode < CMODE_8 || cmode > CMODE_32)
 				cmode = CMODE_8;
 		}

Re: [RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram

[Finn Thain]

Make use of arch_nvram_ops in device drivers so that the nvram_* function 
exports can be removed.

Since they are no longer global symbols, rename the PPC32 nvram_* 
functions appropriately.

Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.

Add a CONFIG_PPC32 test to matroxfb because PPC64 doesn't implement the 
read_byte() method.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---

Changed since v4:
- Added CONFIG_PPC32 test to matroxfb.

---
 arch/powerpc/kernel/setup_32.c             |    8 ++++----
 drivers/char/generic_nvram.c               |    4 ++--
 drivers/video/fbdev/controlfb.c            |    4 ++--
 drivers/video/fbdev/imsttfb.c              |    7 +++----
 drivers/video/fbdev/matrox/matroxfb_base.c |    4 ++--
 drivers/video/fbdev/platinumfb.c           |    4 ++--
 drivers/video/fbdev/valkyriefb.c           |    4 ++--
 7 files changed, 17 insertions(+), 18 deletions(-)

Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c	2015-07-13 21:33:01.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c	2015-07-13 21:33:02.000000000 +1000
@@ -170,20 +170,18 @@ __setup("l3cr=", ppc_setup_l3cr);
 
 #ifdef CONFIG_GENERIC_NVRAM
 
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
 {
 	if (ppc_md.nvram_read_val)
 		return ppc_md.nvram_read_val(addr);
 	return 0xff;
 }
-EXPORT_SYMBOL(nvram_read_byte);
 
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
 {
 	if (ppc_md.nvram_write_val)
 		ppc_md.nvram_write_val(addr, val);
 }
-EXPORT_SYMBOL(nvram_write_byte);
 
 static ssize_t ppc_nvram_get_size(void)
 {
@@ -200,6 +198,8 @@ static long ppc_nvram_sync(void)
 }
 
 const struct nvram_ops arch_nvram_ops = {
+	.read_byte      = ppc_nvram_read_byte,
+	.write_byte     = ppc_nvram_write_byte,
 	.get_size       = ppc_nvram_get_size,
 	.sync           = ppc_nvram_sync,
 };
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c	2015-07-13 21:33:01.000000000 +1000
+++ linux/drivers/char/generic_nvram.c	2015-07-13 21:33:02.000000000 +1000
@@ -64,7 +64,7 @@ static ssize_t read_nvram(struct file *f
 	if (*ppos >= nvram_len)
 		return 0;
 	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
-		if (__put_user(nvram_read_byte(i), p))
+		if (__put_user(arch_nvram_ops.read_byte(i), p))
 			return -EFAULT;
 	*ppos = i;
 	return p - buf;
@@ -84,7 +84,7 @@ static ssize_t write_nvram(struct file *
 	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
 		if (__get_user(c, p))
 			return -EFAULT;
-		nvram_write_byte(c, i);
+		arch_nvram_ops.write_byte(c, i);
 	}
 	*ppos = i;
 	return p - buf;
Index: linux/drivers/video/fbdev/controlfb.c
=================================--- linux.orig/drivers/video/fbdev/controlfb.c	2015-07-13 21:32:43.000000000 +1000
+++ linux/drivers/video/fbdev/controlfb.c	2015-07-13 21:33:02.000000000 +1000
@@ -415,7 +415,7 @@ static int __init init_control(struct fb
 	/* Try to pick a video mode out of NVRAM if we have one. */
 #ifdef CONFIG_NVRAM
 	if (default_cmode = CMODE_NVRAM) {
-		cmode = nvram_read_byte(NV_CMODE);
+		cmode = arch_nvram_ops.read_byte(NV_CMODE);
 		if(cmode < CMODE_8 || cmode > CMODE_32)
 			cmode = CMODE_8;
 	} else
@@ -423,7 +423,7 @@ static int __init init_control(struct fb
 		cmodeÞfault_cmode;
 #ifdef CONFIG_NVRAM
 	if (default_vmode = VMODE_NVRAM) {
-		vmode = nvram_read_byte(NV_VMODE);
+		vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (vmode < 1 || vmode > VMODE_MAX ||
 		    control_mac_modes[vmode - 1].m[full] < cmode) {
 			sense = read_control_sense(p);
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-13 21:32:57.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-13 21:33:02.000000000 +1000
@@ -1886,9 +1886,9 @@ static int initMatrox2(struct matrox_fb_
 		struct fb_var_screeninfo var;
 		if (default_vmode <= 0 || default_vmode > VMODE_MAX)
 			default_vmode = VMODE_640_480_60;
-#ifdef CONFIG_NVRAM
+#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
 		if (default_cmode = CMODE_NVRAM)
-			default_cmode = nvram_read_byte(NV_CMODE);
+			default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 		if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
 			default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/platinumfb.c
=================================--- linux.orig/drivers/video/fbdev/platinumfb.c	2015-07-13 21:32:43.000000000 +1000
+++ linux/drivers/video/fbdev/platinumfb.c	2015-07-13 21:33:02.000000000 +1000
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_in
 	printk(KERN_INFO "platinumfb: Monitor sense value = 0x%x, ", sense);
 	if (default_vmode = VMODE_NVRAM) {
 #ifdef CONFIG_NVRAM
-		default_vmode = nvram_read_byte(NV_VMODE);
+		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (default_vmode <= 0 || default_vmode > VMODE_MAX ||
 		    !platinum_reg_init[default_vmode-1])
 #endif
@@ -362,7 +362,7 @@ static int platinum_init_fb(struct fb_in
 		default_vmode = VMODE_640_480_60;
 #ifdef CONFIG_NVRAM
 	if (default_cmode = CMODE_NVRAM)
-		default_cmode = nvram_read_byte(NV_CMODE);
+		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 	if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
 		default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/valkyriefb.c
=================================--- linux.orig/drivers/video/fbdev/valkyriefb.c	2015-07-13 21:32:43.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c	2015-07-13 21:33:02.000000000 +1000
@@ -287,7 +287,7 @@ static void __init valkyrie_choose_mode(
 	/* Try to pick a video mode out of NVRAM if we have one. */
 #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
 	if (default_vmode = VMODE_NVRAM) {
-		default_vmode = nvram_read_byte(NV_VMODE);
+		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
 		if (default_vmode <= 0
 		 || default_vmode > VMODE_MAX
 		 || !valkyrie_reg_init[default_vmode - 1])
@@ -300,7 +300,7 @@ static void __init valkyrie_choose_mode(
 		default_vmode = VMODE_640_480_67;
 #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
 	if (default_cmode = CMODE_NVRAM)
-		default_cmode = nvram_read_byte(NV_CMODE);
+		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
 #endif
 
 	/*
Index: linux/drivers/video/fbdev/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c	2015-07-13 21:32:43.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c	2015-07-13 21:33:02.000000000 +1000
@@ -328,7 +328,6 @@ enum {
 	TVP = 1
 };
 
-#define USE_NV_MODES		1
 #define INIT_BPP		8
 #define INIT_XRES		640
 #define INIT_YRES		480
@@ -1391,17 +1390,17 @@ static void init_imstt(struct fb_info *i
 		}
 	}
 
-#if USE_NV_MODES && defined(CONFIG_PPC32)
+#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
 	{
 		int vmode = init_vmode, cmode = init_cmode;
 
 		if (vmode = -1) {
-			vmode = nvram_read_byte(NV_VMODE);
+			vmode = arch_nvram_ops.read_byte(NV_VMODE);
 			if (vmode <= 0 || vmode > VMODE_MAX)
 				vmode = VMODE_640_480_67;
 		}
 		if (cmode = -1) {
-			cmode = nvram_read_byte(NV_CMODE);
+			cmode = arch_nvram_ops.read_byte(NV_CMODE);
 			if (cmode < CMODE_8 || cmode > CMODE_32)
 				cmode = CMODE_8;
 		}

Re: [RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram

[Benjamin Herrenschmidt]

On Tue, 2015-07-14 at 17:58 +1000, Finn Thain wrote:
> Make use of arch_nvram_ops in device drivers so that the nvram_* function 
> exports can be removed.
> 
> Since they are no longer global symbols, rename the PPC32 nvram_* 
> functions appropriately.
> 
> Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.
> 
> Add a CONFIG_PPC32 test to matroxfb because PPC64 doesn't implement the 
> read_byte() method.

This is a bit fishy in a way because some of that nvram stuff is really
about powermac/apple nvram offsets, ie, "XPRAM". Maybe we
should have a dedicated accessor for "mac_xpram" and NULL-check it
rather than using ifdef's ?

> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> 
> ---
> 
> Changed since v4:
> - Added CONFIG_PPC32 test to matroxfb.
> 
> ---
>  arch/powerpc/kernel/setup_32.c             |    8 ++++----
>  drivers/char/generic_nvram.c               |    4 ++--
>  drivers/video/fbdev/controlfb.c            |    4 ++--
>  drivers/video/fbdev/imsttfb.c              |    7 +++----
>  drivers/video/fbdev/matrox/matroxfb_base.c |    4 ++--
>  drivers/video/fbdev/platinumfb.c           |    4 ++--
>  drivers/video/fbdev/valkyriefb.c           |    4 ++--
>  7 files changed, 17 insertions(+), 18 deletions(-)
> 
> Index: linux/arch/powerpc/kernel/setup_32.c
> =================================> --- linux.orig/arch/powerpc/kernel/setup_32.c	2015-07-13 21:33:01.000000000 +1000
> +++ linux/arch/powerpc/kernel/setup_32.c	2015-07-13 21:33:02.000000000 +1000
> @@ -170,20 +170,18 @@ __setup("l3cr=", ppc_setup_l3cr);
>  
>  #ifdef CONFIG_GENERIC_NVRAM
>  
> -unsigned char nvram_read_byte(int addr)
> +static unsigned char ppc_nvram_read_byte(int addr)
>  {
>  	if (ppc_md.nvram_read_val)
>  		return ppc_md.nvram_read_val(addr);
>  	return 0xff;
>  }
> -EXPORT_SYMBOL(nvram_read_byte);
>  
> -void nvram_write_byte(unsigned char val, int addr)
> +static void ppc_nvram_write_byte(unsigned char val, int addr)
>  {
>  	if (ppc_md.nvram_write_val)
>  		ppc_md.nvram_write_val(addr, val);
>  }
> -EXPORT_SYMBOL(nvram_write_byte);
>  
>  static ssize_t ppc_nvram_get_size(void)
>  {
> @@ -200,6 +198,8 @@ static long ppc_nvram_sync(void)
>  }
>  
>  const struct nvram_ops arch_nvram_ops = {
> +	.read_byte      = ppc_nvram_read_byte,
> +	.write_byte     = ppc_nvram_write_byte,
>  	.get_size       = ppc_nvram_get_size,
>  	.sync           = ppc_nvram_sync,
>  };
> Index: linux/drivers/char/generic_nvram.c
> =================================> --- linux.orig/drivers/char/generic_nvram.c	2015-07-13 21:33:01.000000000 +1000
> +++ linux/drivers/char/generic_nvram.c	2015-07-13 21:33:02.000000000 +1000
> @@ -64,7 +64,7 @@ static ssize_t read_nvram(struct file *f
>  	if (*ppos >= nvram_len)
>  		return 0;
>  	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
> -		if (__put_user(nvram_read_byte(i), p))
> +		if (__put_user(arch_nvram_ops.read_byte(i), p))
>  			return -EFAULT;
>  	*ppos = i;
>  	return p - buf;
> @@ -84,7 +84,7 @@ static ssize_t write_nvram(struct file *
>  	for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
>  		if (__get_user(c, p))
>  			return -EFAULT;
> -		nvram_write_byte(c, i);
> +		arch_nvram_ops.write_byte(c, i);
>  	}
>  	*ppos = i;
>  	return p - buf;
> Index: linux/drivers/video/fbdev/controlfb.c
> =================================> --- linux.orig/drivers/video/fbdev/controlfb.c	2015-07-13 21:32:43.000000000 +1000
> +++ linux/drivers/video/fbdev/controlfb.c	2015-07-13 21:33:02.000000000 +1000
> @@ -415,7 +415,7 @@ static int __init init_control(struct fb
>  	/* Try to pick a video mode out of NVRAM if we have one. */
>  #ifdef CONFIG_NVRAM
>  	if (default_cmode = CMODE_NVRAM) {
> -		cmode = nvram_read_byte(NV_CMODE);
> +		cmode = arch_nvram_ops.read_byte(NV_CMODE);
>  		if(cmode < CMODE_8 || cmode > CMODE_32)
>  			cmode = CMODE_8;
>  	} else
> @@ -423,7 +423,7 @@ static int __init init_control(struct fb
>  		cmodeÞfault_cmode;
>  #ifdef CONFIG_NVRAM
>  	if (default_vmode = VMODE_NVRAM) {
> -		vmode = nvram_read_byte(NV_VMODE);
> +		vmode = arch_nvram_ops.read_byte(NV_VMODE);
>  		if (vmode < 1 || vmode > VMODE_MAX ||
>  		    control_mac_modes[vmode - 1].m[full] < cmode) {
>  			sense = read_control_sense(p);
> Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
> =================================> --- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-13 21:32:57.000000000 +1000
> +++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-07-13 21:33:02.000000000 +1000
> @@ -1886,9 +1886,9 @@ static int initMatrox2(struct matrox_fb_
>  		struct fb_var_screeninfo var;
>  		if (default_vmode <= 0 || default_vmode > VMODE_MAX)
>  			default_vmode = VMODE_640_480_60;
> -#ifdef CONFIG_NVRAM
> +#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
>  		if (default_cmode = CMODE_NVRAM)
> -			default_cmode = nvram_read_byte(NV_CMODE);
> +			default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
>  #endif
>  		if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
>  			default_cmode = CMODE_8;
> Index: linux/drivers/video/fbdev/platinumfb.c
> =================================> --- linux.orig/drivers/video/fbdev/platinumfb.c	2015-07-13 21:32:43.000000000 +1000
> +++ linux/drivers/video/fbdev/platinumfb.c	2015-07-13 21:33:02.000000000 +1000
> @@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_in
>  	printk(KERN_INFO "platinumfb: Monitor sense value = 0x%x, ", sense);
>  	if (default_vmode = VMODE_NVRAM) {
>  #ifdef CONFIG_NVRAM
> -		default_vmode = nvram_read_byte(NV_VMODE);
> +		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
>  		if (default_vmode <= 0 || default_vmode > VMODE_MAX ||
>  		    !platinum_reg_init[default_vmode-1])
>  #endif
> @@ -362,7 +362,7 @@ static int platinum_init_fb(struct fb_in
>  		default_vmode = VMODE_640_480_60;
>  #ifdef CONFIG_NVRAM
>  	if (default_cmode = CMODE_NVRAM)
> -		default_cmode = nvram_read_byte(NV_CMODE);
> +		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
>  #endif
>  	if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
>  		default_cmode = CMODE_8;
> Index: linux/drivers/video/fbdev/valkyriefb.c
> =================================> --- linux.orig/drivers/video/fbdev/valkyriefb.c	2015-07-13 21:32:43.000000000 +1000
> +++ linux/drivers/video/fbdev/valkyriefb.c	2015-07-13 21:33:02.000000000 +1000
> @@ -287,7 +287,7 @@ static void __init valkyrie_choose_mode(
>  	/* Try to pick a video mode out of NVRAM if we have one. */
>  #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
>  	if (default_vmode = VMODE_NVRAM) {
> -		default_vmode = nvram_read_byte(NV_VMODE);
> +		default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
>  		if (default_vmode <= 0
>  		 || default_vmode > VMODE_MAX
>  		 || !valkyrie_reg_init[default_vmode - 1])
> @@ -300,7 +300,7 @@ static void __init valkyrie_choose_mode(
>  		default_vmode = VMODE_640_480_67;
>  #if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
>  	if (default_cmode = CMODE_NVRAM)
> -		default_cmode = nvram_read_byte(NV_CMODE);
> +		default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
>  #endif
>  
>  	/*
> Index: linux/drivers/video/fbdev/imsttfb.c
> =================================> --- linux.orig/drivers/video/fbdev/imsttfb.c	2015-07-13 21:32:43.000000000 +1000
> +++ linux/drivers/video/fbdev/imsttfb.c	2015-07-13 21:33:02.000000000 +1000
> @@ -328,7 +328,6 @@ enum {
>  	TVP = 1
>  };
>  
> -#define USE_NV_MODES		1
>  #define INIT_BPP		8
>  #define INIT_XRES		640
>  #define INIT_YRES		480
> @@ -1391,17 +1390,17 @@ static void init_imstt(struct fb_info *i
>  		}
>  	}
>  
> -#if USE_NV_MODES && defined(CONFIG_PPC32)
> +#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
>  	{
>  		int vmode = init_vmode, cmode = init_cmode;
>  
>  		if (vmode = -1) {
> -			vmode = nvram_read_byte(NV_VMODE);
> +			vmode = arch_nvram_ops.read_byte(NV_VMODE);
>  			if (vmode <= 0 || vmode > VMODE_MAX)
>  				vmode = VMODE_640_480_67;
>  		}
>  		if (cmode = -1) {
> -			cmode = nvram_read_byte(NV_CMODE);
> +			cmode = arch_nvram_ops.read_byte(NV_CMODE);
>  			if (cmode < CMODE_8 || cmode > CMODE_32)
>  				cmode = CMODE_8;
>  		}

Re: [RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram

[Finn Thain]

On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:

> On Tue, 2015-07-14 at 17:58 +1000, Finn Thain wrote:
> > Make use of arch_nvram_ops in device drivers so that the nvram_* 
> > function exports can be removed.
> > 
> > Since they are no longer global symbols, rename the PPC32 nvram_* 
> > functions appropriately.
> > 
> > Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.
> > 
> > Add a CONFIG_PPC32 test to matroxfb because PPC64 doesn't implement 
> > the read_byte() method.
> 
> This is a bit fishy in a way because some of that nvram stuff is really 
> about powermac/apple nvram offsets, ie, "XPRAM".

Yes, the generalization that PPC64 does not have XPRAM is wrong, but that 
wasn't originally my doing. If we were to address that issue, this patch 
series may not be the best place to do so.

The situation presently is that CONFIG_NVRAM cannot be enabled on PPC64. I 
took advantage of that simplification, despite the corner cases where it 
fails.

The corner cases are found among PPC64 systems with Matrox cards. The 
other PowerMac video drivers are not really relevant here due to "depends 
on PPC32" or "#if defined(CONFIG_PPC32)", meaning that nvram_read_byte() 
isn't a problem there.

Perhaps only dual-boot systems are at issue because AFAIK only Mac OS 
offers a user friendly way to edit XPRAM settings (?) Further, does the 
video mode setting in XPRAM relate only to the MacOS main screen and not 
to other devices? That is, are we concerned here only with dual-boot PPC64 
machines with one matrox card, as the main screen, and no Linux desktop 
environment and no video mode settings on the kernel command line?

> Maybe we should have a dedicated accessor for "mac_xpram" and NULL-check 
> it rather than using ifdef's ?

I wanted arch_nvram_ops to be const data, which means a NULL check won't 
work, because defined(CONFIG_PPC_PMAC) does not imply availability of 
XPRAM at run-time.

There is a similar situation in the m68k portion of this patch series: a 
multi-platform kernel binary might run on an Atari or a Mac. On m68k I 
resolved this with MACH_IS_MAC(), which is analogous to 
machine_is(powermac).

So I can see how to implement XPRAM for matroxfb and imsttfb on PPC64. But 
this is an enhancement that I would defer unless the present limitation is 
already problematic.

-- 

Re: [RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram

[Finn Thain]

On Wed, 15 Jul 2015, I wrote:

> On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:
> 
> > Maybe we should have a dedicated accessor for "mac_xpram" ...
> 
> ... I can see how to implement XPRAM for matroxfb and imsttfb 

I'll have to retract that. The video mode and color mode settings used by 
the PowerMac framebuffer drivers don't exist in the PRAM portion of NVRAM.

Addresses 0x140F and 0x1410 are found in the partition reserved by Apple 
for "Name Registry properties", according to Designing PCI Cards and 
Drivers for Power Macintosh Computers. There is no equivalent on m68k 
Macs, AFAIK.

This is NVRAM partition 2 on my beige g3, which begins at 0x1400. I'm not 
sure that this is true on New World PowerMacs, and I suspect that the 
framebuffer drivers should be calling pmac_get_partition() to determine 
the offset of the beginning of the Name Registry partition.

The arch_nvram_ops methods don't deal with structures like partitions. 
They treat the entire 8 KiB as unstructured, because that's how /dev/nvram 
treats it.

-- 

Re: [RFC v4 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram

[Finn Thain]

Hi Ben,

On Thu, 16 Jul 2015, I wrote:

> On Wed, 15 Jul 2015, I wrote:
> 
> > On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:
> > 
> > > Maybe we should have a dedicated accessor for "mac_xpram" ...
> > 
> ...
> 
> The arch_nvram_ops methods don't deal with structures like partitions ...

Instead of the accessor you suggested, perhaps it would be better to add a 
method like arch_nvram_ops.get_partition, to replace the 
pmac_get_partition() exported function?

The call sites for pmac_get_partition() are in the implementation of the 
IOC_NVRAM_GET_OFFSET ioctl that's used with /dev/nvram, and in 
pmac_xpram_read(). pmac_xpram_write() has no caller and could be removed.

But this doesn't have much to do with linux-fbdev. I think the old 
NV_CMODE/NV_VMODE issues*, which this patch avoids, are irrelevant to the 
problem of nvram module re-use, which is the aim of this patch series.

But if those issues really are relevant then we should move the discussion 
to the revised patch, that is, [RFC v6 16/25] powerpc, fbdev: Use NV_CMODE 
and NV_VMODE only when CONFIG_PPC32 and CONFIG_PPC_PMAC and CONFIG_NVRAM.

(There was no response to any patch in RFC v6 from any PowerPC 
maintainers, which is why I've revived this thread.)

* https://lists.ozlabs.org/pipermail/linuxppc-dev/2001-November/012662.html

--