Re: [PATCH] Replace -EINVAL with PCIBIOS_BAD_REGISTER_NUMBER

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Apr 10, 2020 at 09:28:07AM +0800, Yicong Yang wrote:
> Hi Bolarinwa,
> 
> I notice some drivers use these functions and if there is an error,
> pass the error code directly to the userspace. As it's our private
> error code, is it appropriate to pass or should we call
> pcibios_err_to_errno()(include/linux/pci.h, line 672) to do the
> conversion?

The whole point of this is to make the return values of the
pcie_capability_{read,write,etc}*() functions work the same as
the pci_{read,write}_config*() functions.

The latter return PCIBIOS_* error codes, so the former should as well.

When we do this, we do need to audit every caller of the
pcie_capability_{read,write}*() functions to make sure we don't break
them.  If some callers pass the error code directly to userspace, they
may need some change.

Yicong, can you point to the ones you noticed so Saheed can check them
out?

Bjorn

> On 2020/4/10 0:16, Bolarinwa Olayemi Saheed wrote:
> > Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> > Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
> > ---
> >  drivers/pci/access.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> > index 79c4a2ef269a..451f2b8b2b3c 100644
> > --- a/drivers/pci/access.c
> > +++ b/drivers/pci/access.c
> > @@ -409,7 +409,7 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
> >  
> >  	*val = 0;
> >  	if (pos & 1)
> > -		return -EINVAL;
> > +		return PCIBIOS_BAD_REGISTER_NUMBER;
> >  
> >  	if (pcie_capability_reg_implemented(dev, pos)) {
> >  		ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
> > @@ -444,7 +444,7 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
> >  
> >  	*val = 0;
> >  	if (pos & 3)
> > -		return -EINVAL;
> > +		return PCIBIOS_BAD_REGISTER_NUMBER;
> >  
> >  	if (pcie_capability_reg_implemented(dev, pos)) {
> >  		ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
>