Re: [PATCH v8 00/25] Re-use nvram module
From: Arnd Bergmann
Date: Mon Dec 31 2018 - 07:23:17 EST
On Sun, Dec 30, 2018 at 5:05 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, 30 Dec 2018, I wrote:
>
> >
> > 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.)
>
> The rationale for the ops struct was that it offers introspection.
>
> It turns out that PPC64 device drivers don't care about byte-at-a-time
> accessors and X86 device drivers don't care about checksum validation.
> But that only gets us so far.
>
> We still needed a way to find out whether the arch has provided
> byte-at-a-time accessors (i.e. PPC32 and M68K Mac) or byte range accessors
> (i.e. PPC64 and those platforms with checksummed NVRAM like X86 and M68K
> Atari).
>
> You can't resolve this question at build time for a multi-platform kernel
> binary, so pre-processor tricks don't help.
>
> Device drivers tend to want to access NVRAM one byte at a time. With this
> patch series, those platforms which need checksum validation always set
> byte-at-a-time methods to NULL. (Hence the atari_scsi changes in patch 3.)
>
> The char misc driver is quite different to the usual device drivers,
> because the struct file_operations methods always access a byte range.
>
> The NULL methods in the ops struct allow the nvram.c misc device to avoid
> inefficient byte-at-a-time accessors where possible, just as
> arch/powerpc/kernel/nvram_64.c presently does.
Ok, I see. That sounds absolutely reasonable, so let's stay with
the structure as you proposed.
Arnd