From: Finn Thain <hidden> Date: 2015-07-12 10:42:20
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 <redacted>
---
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
@@ -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_NVRAMif(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 fbcmodeÞfault_cmode;#ifdef CONFIG_NVRAMif(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);
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_inprintk(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_indefault_vmode=VMODE_640_480_60;#ifdef CONFIG_NVRAMif(default_cmode=CMODE_NVRAM)-default_cmode=nvram_read_byte(NV_CMODE);+default_cmode=arch_nvram_ops.read_byte(NV_CMODE);#endifif(default_cmode<CMODE_8||default_cmode>CMODE_32)default_cmode=CMODE_8;
@@ -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])
From: Finn Thain <hidden> Date: 2015-07-14 07:58:31
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 <redacted>
---
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
@@ -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_NVRAMif(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 fbcmodeÞfault_cmode;#ifdef CONFIG_NVRAMif(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);
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_inprintk(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_indefault_vmode=VMODE_640_480_60;#ifdef CONFIG_NVRAMif(default_cmode=CMODE_NVRAM)-default_cmode=nvram_read_byte(NV_CMODE);+default_cmode=arch_nvram_ops.read_byte(NV_CMODE);#endifif(default_cmode<CMODE_8||default_cmode>CMODE_32)default_cmode=CMODE_8;
@@ -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])
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-07-14 11:52:45
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 ?
@@ -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_NVRAMif(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 fbcmodeÞfault_cmode;#ifdef CONFIG_NVRAMif(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);
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_inprintk(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_indefault_vmode=VMODE_640_480_60;#ifdef CONFIG_NVRAMif(default_cmode=CMODE_NVRAM)-default_cmode=nvram_read_byte(NV_CMODE);+default_cmode=arch_nvram_ops.read_byte(NV_CMODE);#endifif(default_cmode<CMODE_8||default_cmode>CMODE_32)default_cmode=CMODE_8;
@@ -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])
From: Finn Thain <hidden> Date: 2015-07-15 05:21:37
On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:
On Tue, 2015-07-14 at 17:58 +1000, Finn Thain wrote:
quoted
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.
--
From: Finn Thain <hidden> Date: 2015-07-16 06:02:15
On Wed, 15 Jul 2015, I wrote:
On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:
quoted
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.
--
From: Finn Thain <hidden> Date: 2015-09-18 08:18:10
Hi Ben,
On Thu, 16 Jul 2015, I wrote:
On Wed, 15 Jul 2015, I wrote:
quoted
On Tue, 14 Jul 2015, Benjamin Herrenschmidt wrote:
quoted
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
--