Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap

[Dave Martin <Dave.Martin@arm.com>]

On Wed, Apr 03, 2019 at 09:53:26AM +0100, Andrew Murray wrote:
> On Tue, Apr 02, 2019 at 04:55:58PM +0100, Dave Martin wrote:
> > On Tue, Apr 02, 2019 at 04:32:57PM +0100, Suzuki K Poulose wrote:

[...]

> > > nit:
> > > 
> > > As mentioned above we have "cpu_hwcaps" for the features only internally
> > > by the kernel. Naming it "kernel_hwcap" kind of looses the hint that the
> > > major purpose is for userspace consumption and could easily confuse with
> > > the poorly named "cpu_hwcaps" which should have been called kernel_hwcaps.
> > > 
> > > How about "user_hwcaps" ? Or preferrably something closer to that.
> > 
> > Yes, that may be better.
> > 
> > Of course, we also have this naming in all the KERNEL_HWCAP #defined now.
> > 
> > Since kernel_hwcap is just a static variable now, maybe it's sufficient
> > to stick a comment next to it explaining what it is (and what it isn't).
> > "user_hwcaps" still implies that this might be the userspace view of the
> > flags, which it isn't.
> > 
> > But I don't feel strongly about this.  If someone wants to make a
> > decision, I'm happy to defer to it.
> 
> I think changing the name will cause more confusion - there isn't an obvious
> name for it and needing a comment to explain it hints that this may not be
> the best approach. As it's a static variable with only 4 uses in the same
> file it should be pretty clear to anyone interested. Also keeping the same
> name will help users find it and understand how it has changed if they
> incorrectly attempt to use it by setting/testing bits on it.
> 
> Afterall the elf_hwcap variable does still hold the elf_hwcap bits and it's
> obtained by cpu_get_elf_hwcap. The naming of KERNEL_HWCAP also makes sense
> in this context.
> 
> Perhaps a better name would be something like elf_hwcaps implying that there
> is some mapping required (though this would only last until we run out of
> space in it and need another one).
> 
> Shall we stick with what we have?

I'm happy enough with what you propose: I agree, there's not an
obviously a better name, and now that this is local, the scope for
confusion is lessened.  So, add a comment, but keep whetever name you're
happy with.

Cheers
---Dave