On Sat, 29 Dec 2018, I wrote:
> On Fri, 28 Dec 2018, LEROY Christophe wrote:
>
> > > --- a/drivers/char/nvram.c
> > > +++ b/drivers/char/nvram.c
> > > @@ -21,13 +21,6 @@
> > > * ioctl(NVRAM_SETCKS) (doesn't change contents, just makes checksum
> > > valid
> > > * again; use with care!)
> > > *
> > > - * This file also provides some functions for other parts of the kernel
> > > that
> > > - * want to access the NVRAM:
> > > nvram_{read,write,check_checksum,set_checksum}.
> > > - * Obviously this can be used only if this driver is always configured
> > > into
> > > - * the kernel and is not a module. Since the functions are used by
> > > some Atari
> > > - * drivers, this is the case on the Atari.
> > > - *
> > > - *
> > > * 1.1 Cesar Barros: SMP locking fixes
> > > * added changelog
> > > * 1.2 Erik Gilling: Cobalt Networks support
> > > @@ -39,64 +32,6 @@
> > >
> > > #include <linux/module.h>
> > > #include <linux/nvram.h>
> > > -
> > > -#define PC 1
> > > -#define ATARI 2
> > > -
> > > -/* select machine configuration */
> > > -#if defined(CONFIG_ATARI)
> > > -# define MACH ATARI
> > > -#elif defined(__i386__) || defined(__x86_64__) || defined(__arm__)
> > > /* and ?? */
> > > -# define MACH PC
> > > -#else
> > > -# error Cannot build nvram driver for this machine configuration.
> > > -#endif
> > > -
> > > -#if MACH == PC
> > > -
> > > -/* RTC in a PC */
> > > -#define CHECK_DRIVER_INIT() 1
> > > -
> > > -/* On PCs, the checksum is built only over bytes 2..31 */
> > > -#define PC_CKS_RANGE_START 2
> > > -#define PC_CKS_RANGE_END 31
> > > -#define PC_CKS_LOC 32
> > > -#define NVRAM_BYTES (128-NVRAM_FIRST_BYTE)
> > > -
> > > -#define mach_check_checksum pc_check_checksum
> > > -#define mach_set_checksum pc_set_checksum
> > > -#define mach_proc_infos pc_proc_infos
> > > -
> > > -#endif
> > > -
> > > -#if MACH == ATARI
> > > -
> > > -/* Special parameters for RTC in Atari machines */
> > > -#include <asm/atarihw.h>
> > > -#include <asm/atariints.h>
> > > -#define RTC_PORT(x) (TT_RTC_BAS + 2*(x))
> > > -#define CHECK_DRIVER_INIT() (MACH_IS_ATARI &&
> > > ATARIHW_PRESENT(TT_CLK))
> > > -
> > > -#define NVRAM_BYTES 50
> > > -
> > > -/* On Ataris, the checksum is over all bytes except the checksum bytes
> > > - * themselves; these are at the very end */
> > > -#define ATARI_CKS_RANGE_START 0
> > > -#define ATARI_CKS_RANGE_END 47
> > > -#define ATARI_CKS_LOC 48
> > > -
> > > -#define mach_check_checksum atari_check_checksum
> > > -#define mach_set_checksum atari_set_checksum
> > > -#define mach_proc_infos atari_proc_infos
> > > -
> > > -#endif
> > > -
> > > -/* Note that *all* calls to CMOS_READ and CMOS_WRITE must be done with
> > > - * rtc_lock held. Due to the index-port/data-port design of the RTC, we
> > > - * don't want two different things trying to get to it at once. (e.g. the
> > > - * periodic 11 min sync from kernel/time/ntp.c vs. this driver.)
> > > - */
> > > -
> > > #include <linux/types.h>
> > > #include <linux/errno.h>
> > > #include <linux/miscdevice.h>
> > > @@ -120,12 +55,9 @@ static int nvram_open_mode; /* special open modes */
> > > #define NVRAM_WRITE 1 /* opened for writing (exclusive) */
> > > #define NVRAM_EXCL 2 /* opened with O_EXCL */
> > >
> > > -static int mach_check_checksum(void);
> > > -static void mach_set_checksum(void);
> > > -
> > > #ifdef CONFIG_PROC_FS
> > > -static void mach_proc_infos(unsigned char *contents, struct seq_file
> > > *seq,
> > > - void *offset);
> > > +static void pc_nvram_proc_read(unsigned char *contents, struct
> > > seq_file *seq,
> > > + void *offset);
> > > #endif
> > >
> > > /*
> > > @@ -139,6 +71,14 @@ static void mach_proc_infos(unsigned char
> > > *contents, struct seq_file *seq,
> > > * know about the RTC cruft.
> > > */
> > >
> > > +#define NVRAM_BYTES (128 - NVRAM_FIRST_BYTE)
> > > +
> > > +/* Note that *all* calls to CMOS_READ and CMOS_WRITE must be done with
> > > + * rtc_lock held. Due to the index-port/data-port design of the RTC, we
> > > + * don't want two different things trying to get to it at once. (e.g. the
> > > + * periodic 11 min sync from kernel/time/ntp.c vs. this driver.)
> > > + */
> > > +
> > > unsigned char __nvram_read_byte(int i)
> > > {
> > > return CMOS_READ(NVRAM_FIRST_BYTE + i);
> > > @@ -174,9 +114,22 @@ void nvram_write_byte(unsigned char c, int i)
> > > }
> > > EXPORT_SYMBOL(nvram_write_byte);
> > >
> > > +/* On PCs, the checksum is built only over bytes 2..31 */
> > > +#define PC_CKS_RANGE_START 2
> > > +#define PC_CKS_RANGE_END 31
> > > +#define PC_CKS_LOC 32
> > > +
> > > int __nvram_check_checksum(void)
> > > {
> > > - return mach_check_checksum();
> > > + int i;
> > > + unsigned short sum = 0;
> > > + unsigned short expect;
> > > +
> > > + for (i = PC_CKS_RANGE_START; i <= PC_CKS_RANGE_END; ++i)
> > > + sum += __nvram_read_byte(i);
> > > + expect = __nvram_read_byte(PC_CKS_LOC)<<8 |
> > > + __nvram_read_byte(PC_CKS_LOC+1);
> > > + return (sum & 0xffff) == expect;
> > > }
> >
> >
> > I don't understand how this is part of the code move.
> > Does the pc specific checksum becomes the generic one ?
> >
>
> This is not generic code, of course. Please refer to the two patches
> that follow this one, in which all of the x86-specific code gets wrapped
> with #ifdef CONFIG_X86.
>
> This code gets moved because the MACH macro is made redundant. You might
> defer this code motion to patch 4 or patch 5 but I don't see that as being
> an improvement.
>
> [...]
>
> You may argue that there should be no CONFIG_X86 code in drivers/char.
I think I now remember why this x86-specific code doesn't get moved from
drivers/char to arch/x86 in this patch series.
In the case of PPC32 and PPC64, the nvram accessors are presently built-in
using rules like this,
arch/powerpc/platforms/powermac/Makefile:obj-$(CONFIG_PPC64) += nvram.o
arch/powerpc/platforms/powernv/Makefile:obj-y += ... opal-nvram.o ...
arch/powerpc/platforms/pseries/Makefile:obj-y := ... nvram.o ...
... or like this,
arch/powerpc/platforms/powermac/Makefile:obj-$(CONFIG_NVRAM:m=y) += nvram.o
... except in one case they are in a separate module, though this doesn't
work for built-in callers such as chrp_init2(),
arch/powerpc/platforms/chrp/Makefile:obj-$(CONFIG_NVRAM) += nvram.o
Anyway, I didn't think that any of these options would make it past the
x86 maintainers so I just left the x86-specific code in place.
--
