Messages in this thread | | | Date | Sun, 30 Dec 2018 11:09:13 +1100 (AEDT) | From | Finn Thain <> | Subject | Re: [PATCH v8 00/25] Re-use nvram module |
| |
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
> I had a look at the complete series now, and I think this is a great > cleanup. I replied with a couple of minor comments that you may or may > not want to address first. >
Thanks for reviewing this.
> The one thing I would like to see resolved (I hope this doesn't bring > back an old discussion you had already concluded) is regarding the use > of a global exported structure of function pointers, as opposed to using > either directly exported functions (with a consistent interface) or a > boot-time selectable structure like dma_map_ops or ppc_md. >
If I understand correctly, /dev/nvram was made obsolete by the nvmem subsystem (?). If so, there won't be new /dev/nvram users, and the refactoring here only has to be sufficiently flexible to meet the needs of existing users.
I'm not opposed to exported functions in place of a singleton ops struct. Other things being equal I'm inclined toward the ops struct, perhaps because I like encapsulation or perhaps because I don't like excess generality. (That design decision was made years ago and I don't remember the reasoning.)
All the arch_nvram_ops structs that I've defined in these patches have the 'const' properly:
const struct nvram_ops arch_nvram_ops = { .read_byte = nvram_read_byte, .write_byte = nvram_write_byte, .read = nvram_read, .write = nvram_write, .get_size = nvram_get_size, .set_checksum = nvram_set_checksum, .initialize = nvram_initialize, }; EXPORT_SYMBOL(arch_nvram_ops);
This is because there's no need to do any run-time reconfiguration.
Is a collection of exported functions a better fit here?
--
> Arnd >
| |