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

Finn Thain
2015-07-15T05:21:37Z

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.

--