Re: [PATCH v8 18/25] powerpc: Implement nvram sync ioctl
From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-12-31 12:17:52
Also in:
lkml
On Mon, Dec 31, 2018 at 12:13 AM Finn Thain [off-list ref] wrote:
On Sun, 30 Dec 2018, Finn Thain wrote:quoted
quoted
quoted
diff --git a/include/linux/nvram.h b/include/linux/nvram.h index b7bfaec60a43..24a57675dba1 100644 --- a/include/linux/nvram.h +++ b/include/linux/nvram.h@@ -18,8 +18,12 @@ struct nvram_ops { unsigned char (*read_byte)(int); void (*write_byte)(unsigned char, int); ssize_t (*get_size)(void); +#ifdef CONFIG_PPC + long (*sync)(void); +#else long (*set_checksum)(void); long (*initialize)(void); +#endif };Maybe just leave all entries visible here, and avoid the above #ifdef checks.The #ifdef isn't there just to save a few bytes, though it does do that. It's really meant to cause a build failure when I mess up somewhere. But I'm happy to change it if you can see a reason to do so (?)I think the problem with these #ifdef conditionals is that they don't express the correct constraints. So, at the end of this series I'd prefer to see, struct nvram_ops { ssize_t (*read)(char *, size_t, loff_t *); ssize_t (*write)(char *, size_t, loff_t *); unsigned char (*read_byte)(int); void (*write_byte)(unsigned char, int); ssize_t (*get_size)(void); #if defined(CONFIG_PPC) long (*sync)(void); int (*get_partition)(int); #elif defined(CONFIG_X86) || defined(CONFIG_M68K) long (*set_checksum)(void); long (*initialize)(void); #endif }; Is that okay with you?
My preference would be no #ifdef here, but the compile time
error you mention is a good enough reason, so I'm fine with
either version you pick.
Arnd