Re: [RFC v6 00/25] Re-use nvram module

Laurent Vivier
2015-10-11T20:06:18Z

On 23/08/2015 12:41, Finn Thain wrote:
> The generic NVRAM module, drivers/char/generic_nvram, implements a
> /dev/nvram misc device. It is used only by 32-bit PowerPC platforms and
> isn't generic enough to be more widely used.
>
> The RTC NVRAM module, drivers/char/nvram, also implements a /dev/nvram
> misc device. It is used by x86, ARM and m68k.
>
> The former module cannot be used on x86, ARM or m68k because it
> cannot co-exist with the latter module, partly due to the Kconfig logic.
>
> It is possible to modify the modules so that one kernel binary could
> have either, neither or both. However, automatically loading the
> appropriate module is then impossible; if both provide the
> char-major-10-144 alias then the wrong module will end up being loaded.
> Hence a multi-platform kernel binary needs a single generic nvram module
> with alias char-major-10-144.
>
> Therefore, drivers/char/nvram.c should be made more generic and the
> arch-specific code therein should be moved to a more appropriate
> place under arch/. Also, drivers/char/generic_nvram.c should be removed
> to reduce code duplication.
>
> In this patch series, Atari-specific code is moved from the nvram module
> to arch/m68k/atari. More arch-specific code in the nvram module could
> be moved, probably to arch/x86, but it is difficult to determine just
> what code is relevant to ARM platforms and what code is x86-only.
>
> In addressing code duplication, v1 of this patch series removes three
> inconsistent /dev/nvram misc device implementations. One of these,
> drivers/macintosh/nvram.c is entirely unused already. The other two,
> drivers/char/generic_nvram.c and the misc device implementation in
> arch/powerpc/kernel/nvram_64.c, are replaced by drivers/char/nvram.c.
>
> A benefit of this work is better consistency -- between PPC32 and PPC64
> as well as between PPC_PMAC and MAC. This new uniformity does have
> implications for userspace, that is, some error codes for some ioctl calls
> become consistent on all PowerPC platforms.
>
> The drivers/char/nvram module becomes sufficiently generic to be useful
> to other platforms and architectures, besides those with "CMOS" RTC. At the
> end of this patch series the module is adopted by the m68k Mac port, which
> already has PRAM access functions but lacks the /dev/nvram misc device.
>
> This patch series has been compile-tested for arm, m68k, powerpc and x86.
> The nvram and thinkpad_acpi modules were regression tested on a ThinkPad
> T43. The /dev/nvram functionality was also regression tested on a G3
> PowerMac. The nvram module was also tested on a PowerBook 520, Quadra 650
> and Atari Falcon. AFAIK, no testing has been done on PPC64 as yet.

For PPC64, tested on a PowerMac G5 (PowerMac11,2), kernel 4.3.0-rc4
with nvram tools 1.2.24 (Fedora 22 ppc64).

Tested-by: Laurent Vivier <lvivier@redhat.com>

Tests done:
# modinfo nvram
filename: /lib/modules/4.3.0-rc4+/kernel/drivers/char/nvram.ko
alias: devname:nvram
alias: char-major-10-144
license: GPL
depends:
intree: Y
vermagic: 4.3.0-rc4+ SMP mod_unload

# modprobe nvram
[ 364.655577] Non-volatile memory driver v1.3

# nvram --partitions
# Sig Chk Len Name
0 5a 82 0002 nvram
1 5f 45 003e system
2 70 bd 00c1 common
3 a0 1e 0052 APL,MacOS75
4 a1 15 0081 APL,OSXPanic
5 7f 45 002c wwwwwwwwwwww

# nvram --dump "nvram"
0x00000000 5a820002 6e767261 6d000000 00000000 |Z...nvram.......|
0x00000010 cae8613d 000001c9 00000000 00000000 |..a=............|

# nvram --print-config=boot-volume
1

# nvram --update-config=boot-volume=6

# rmmod nvram

# nvram --print-config=boot-volume
6

# reboot

# nvram --print-config=boot-volume
6

> Changes since v1:
> - Minor changes to patches 7, 15 and 20 as described in commit logs.
> - Revised patches 21 and 24 to address comments from Geert.
>
> Changes since v2:
> - Dropped patch 1, "macintosh/nvram: Remove as unused", because it has
> since been merged.
> - Inserted a new patch, "m68k/mac: Use macros for RTC accesses not
> magic numbers".
> - Revised patches 21 and 23 to address comments from Geert.
>
> Changes since v3:
> - Split the patch, "m68k/atari: Move Atari-specific code out of
> drivers/char/nvram.c", as suggested by Geert.
> - Revised patches 11 and 25 to address comments from Geert.
> - Minor change to patch 21 as described in commit log.
>
> Changes since v4:
> - Split the patch, "powerpc, fbdev: Use arch_nvram_ops methods instead
> of nvram_read_byte() and nvram_write_byte()", so as to separately address
> inconsistent use of Kconfig symbols in PowerMac fbdev drivers.
> - Fix possible git bisect build failure with CONFIG_PPC_PMAC=n.
>
> Changes since v5:
> - Dropped patch 2, "char/nvram: Use bitwise OR to obtain Atari video mode
> data", since it has been added to Greg Kroah-Hartman's char-misc-next queue.
> - In patch 15, error codes from the IOC_NVRAM_GET_OFFSET ioctl are now
> consistent with the old generic_nvram module.
>
> ---
> arch/m68k/Kconfig | 3
> arch/m68k/Kconfig.machine | 2
> arch/m68k/atari/Makefile | 2
> arch/m68k/atari/nvram.c | 291 +++++++++++
> arch/m68k/include/asm/atarihw.h | 6
> arch/m68k/include/asm/macintosh.h | 4
> arch/m68k/kernel/setup_mm.c | 100 +++
> arch/m68k/mac/misc.c | 207 +++++---
> arch/powerpc/Kconfig | 5
> arch/powerpc/include/asm/nvram.h | 9
> arch/powerpc/kernel/nvram_64.c | 203 +------
> arch/powerpc/kernel/setup_32.c | 27 -
> arch/powerpc/platforms/chrp/Makefile | 2
> arch/powerpc/platforms/chrp/nvram.c | 14
> arch/powerpc/platforms/chrp/setup.c | 2
> arch/powerpc/platforms/powermac/Makefile | 5
> arch/powerpc/platforms/powermac/nvram.c | 9
> arch/powerpc/platforms/powermac/setup.c | 3
> arch/powerpc/platforms/pseries/nvram.c | 2
> drivers/char/Kconfig | 13
> drivers/char/Makefile | 6
> drivers/char/generic_nvram.c | 174 ------
> drivers/char/nvram.c | 742 ++++++++++++-----------------
> drivers/platform/x86/thinkpad_acpi.c | 20
> drivers/scsi/Kconfig | 6
> drivers/scsi/atari_scsi.c | 16
> drivers/video/fbdev/Kconfig | 2
> drivers/video/fbdev/controlfb.c | 4
> drivers/video/fbdev/imsttfb.c | 17
> drivers/video/fbdev/matrox/matroxfb_base.c | 6
> drivers/video/fbdev/platinumfb.c | 4
> drivers/video/fbdev/valkyriefb.c | 18
> include/linux/nvram.h | 23
> include/uapi/linux/pmu.h | 2
> 34 files changed, 998 insertions(+), 951 deletions(-)
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>