[Linux-kernel-mentees] [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
David.Laight at ACULAB.COM
Thu Jul 16 08:18:41 UTC 2020
From: Bjorn Helgaas
> Sent: 15 July 2020 23:02
> On Wed, Jul 15, 2020 at 02:24:21PM +0000, David Laight wrote:
> > From: Arnd Bergmann
> > > Sent: 15 July 2020 07:47
> > > On Wed, Jul 15, 2020 at 1:46 AM Bjorn Helgaas <helgaas at kernel.org> wrote:
> > >
> > > So something like:
> > > >
> > > > void pci_read_config_word(struct pci_dev *dev, int where, u16 *val)
> > > >
> > > > and where we used to return anything non-zero, we just set *val = ~0
> > > > instead? I think we do that already in most, maybe all, cases.
> > >
> > > Right, this is what I had in mind. If we start by removing the handling
> > > of the return code in all files that clearly don't need it, looking at
> > > whatever remains will give a much better idea of what a good interface
> > > should be.
> > It would be best to get rid of that nasty 'u16 *' parameter.
> Do you mean nasty because it's basically a return value, but not
> returned as the *function's* return value? I agree that if we were
> starting from scratch it would nicer to have:
> u16 pci_read_config_word(struct pci_dev *dev, int where)
> but I don't think it's worth changing the thousands of callers just
> for that.
It'll shrink the kernel text size somewhat.
It could also be 'fixed' with a static inline.
Actually you don't even want the result to be u16.
Even though the domain of the value is 0..65535 keeping
the type as int (or unsigned int) will save the compiler
having to generate lots of masking instructions.
Code performance here will be overwhelmed by the time taken
for the config space access.
But more generally all local variables should really be
the size of cpu registers.
On x86-64 you need to use 'unsigned int' for anything used
as array subscripts to avoid the 'sign extend' instructions.
In some code paths it may matter...
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
More information about the Linux-kernel-mentees