Re: [Linux-kernel-mentees] [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86

From: Bjorn Helgaas <helgaas@kernel.org>
To: David Laight <David.Laight@ACULAB.COM>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Keith Busch <kbusch@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	sparclinux <sparclinux@vger.kernel.org>,
	Toan Le <toan@os.amperecomputing.com>,
	Kjetil Oftedal <oftedal@gmail.com>,
	Greg Ungerer <gerg@linux-m68k.org>,
	Marek Vasut <marek.vasut+renesas@gmail.com>,
	Rob Herring <robh@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Sagi Grimberg <sagi@grimberg.me>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Russell King <linux@armlinux.org.uk>,
	Ley Foon Tan <ley.foon.tan@intel.com>,
	Christoph Hellwig <hch@lst.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Kevin Hilman <khilman@baylibre.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	Matt Turner <mattst88@gmail.com>,
	"linux-kernel-mentees@lists.linuxfoundation.org"
	<linux-kernel-mentees@lists.linuxfoundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	'Arnd Bergmann' <arnd@arndb.de>, Ray Jui <rjui@broadcom.com>,
	Jens Axboe <axboe@fb.com>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Richard Henderson <rth@twiddle.net>,
	Juergen Gross <jgross@suse.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Scott Branden <sbranden@broadcom.com>,
	Jingoo Han <jingoohan1@gmail.com>,
	"Saheed O. Bolarinwa" <refactormyself@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"David S. Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [Linux-kernel-mentees] [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86
Date: Wed, 15 Jul 2020 17:01:35 -0500	[thread overview]
Message-ID: <20200715220135.GA563272@bjorn-Precision-5520> (raw)
In-Reply-To: <4c994d9a804b4a2c8555c50b63e20772@AcuMS.aculab.com>

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@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.

> Make the return int and return the read value or -1 on error.
> (Or maybe 0xffff0000 on error??)
> 
> For a 32bit read (there must be one for the BARs) returning
> a 64bit signed integer would work even for 32bit systems.
> 
> If code cares about the error, and it can be detected then
> it can check. Otherwise the it all 'just works'.

There are u8 (byte), u16 (word), and u32 (dword) config reads &
writes.  But I don't think it really helps to return something wider
than the access.  For programmatic errors like invalid alignment, we
could indeed use the extra bits to return an unambiguous error.  But
we still have the "device was unplugged" sort of errors where the
*hardware* typically returns ~0 and the config accessor doesn't know
whether that's valid data or an error.

Bjorn
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees