2021-10-16 13:04:18

by Naveen Naidu

[permalink] [raw]
Subject: [PATCH v2 02/24] PCI: Set error response in config access defines when ops->read() fails

Make PCI_OP_READ and PCI_USER_READ_CONFIG set the data value with error
response (~0), when the PCI device read by a host controller fails.

This ensures that the controller drivers no longer need to fabricate
(~0) value when they detect error. It also gurantees that the error
response (~0) is always set when the controller drivers fails to read a
config register from a device.

This makes error response fabrication consistent and helps in removal of
a lot of repeated code.

Suggested-by: Rob Herring <[email protected]>
Signed-off-by: Naveen Naidu <[email protected]>
---
drivers/pci/access.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 46935695cfb9..b3b2006ed1d2 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -42,7 +42,10 @@ int noinline pci_bus_read_config_##size \
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
pci_lock_config(flags); \
res = bus->ops->read(bus, devfn, pos, len, &data); \
- *value = (type)data; \
+ if (res) \
+ SET_PCI_ERROR_RESPONSE(value); \
+ else \
+ *value = (type)data; \
pci_unlock_config(flags); \
return res; \
}
@@ -228,7 +231,10 @@ int pci_user_read_config_##size \
ret = dev->bus->ops->read(dev->bus, dev->devfn, \
pos, sizeof(type), &data); \
raw_spin_unlock_irq(&pci_lock); \
- *val = (type)data; \
+ if (ret) \
+ SET_PCI_ERROR_RESPONSE(val); \
+ else \
+ *val = (type)data; \
return pcibios_err_to_errno(ret); \
} \
EXPORT_SYMBOL_GPL(pci_user_read_config_##size);
--
2.25.1


2021-10-20 13:44:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 02/24] PCI: Set error response in config access defines when ops->read() fails

On Fri, Oct 15, 2021 at 07:58:17PM +0530, Naveen Naidu wrote:
> Make PCI_OP_READ and PCI_USER_READ_CONFIG set the data value with error
> response (~0), when the PCI device read by a host controller fails.
>
> This ensures that the controller drivers no longer need to fabricate
> (~0) value when they detect error. It also gurantees that the error
> response (~0) is always set when the controller drivers fails to read a
> config register from a device.
>
> This makes error response fabrication consistent and helps in removal of
> a lot of repeated code.
>
> Suggested-by: Rob Herring <[email protected]>
> Signed-off-by: Naveen Naidu <[email protected]>
> ---
> drivers/pci/access.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring <[email protected]>

2021-10-20 13:55:41

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v2 02/24] PCI: Set error response in config access defines when ops->read() fails

On Friday 15 October 2021 19:58:17 Naveen Naidu wrote:
> Make PCI_OP_READ and PCI_USER_READ_CONFIG set the data value with error
> response (~0), when the PCI device read by a host controller fails.
>
> This ensures that the controller drivers no longer need to fabricate
> (~0) value when they detect error. It also gurantees that the error
> response (~0) is always set when the controller drivers fails to read a
> config register from a device.
>
> This makes error response fabrication consistent and helps in removal of
> a lot of repeated code.
>
> Suggested-by: Rob Herring <[email protected]>
> Signed-off-by: Naveen Naidu <[email protected]>
> ---
> drivers/pci/access.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 46935695cfb9..b3b2006ed1d2 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -42,7 +42,10 @@ int noinline pci_bus_read_config_##size \
> if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> pci_lock_config(flags); \
> res = bus->ops->read(bus, devfn, pos, len, &data); \
> - *value = (type)data; \
> + if (res) \
> + SET_PCI_ERROR_RESPONSE(value); \
> + else \
> + *value = (type)data; \

Hello! Just one small comment. It looks like that in this patch is
broken alignment of backslashes on the end of lines. Prior this patch
backslashes on all lines were at the same column. With this patch they
are not.

> pci_unlock_config(flags); \
> return res; \
> }
> @@ -228,7 +231,10 @@ int pci_user_read_config_##size \
> ret = dev->bus->ops->read(dev->bus, dev->devfn, \
> pos, sizeof(type), &data); \
> raw_spin_unlock_irq(&pci_lock); \
> - *val = (type)data; \
> + if (ret) \
> + SET_PCI_ERROR_RESPONSE(val); \
> + else \
> + *val = (type)data; \
> return pcibios_err_to_errno(ret); \
> } \
> EXPORT_SYMBOL_GPL(pci_user_read_config_##size);
> --
> 2.25.1
>

2021-10-20 15:14:49

by Naveen Naidu

[permalink] [raw]
Subject: Re: [PATCH v2 02/24] PCI: Set error response in config access defines when ops->read() fails

On 20/10, Pali Roh?r wrote:
> On Friday 15 October 2021 19:58:17 Naveen Naidu wrote:
> > Make PCI_OP_READ and PCI_USER_READ_CONFIG set the data value with error
> > response (~0), when the PCI device read by a host controller fails.
> >
> > This ensures that the controller drivers no longer need to fabricate
> > (~0) value when they detect error. It also gurantees that the error
> > response (~0) is always set when the controller drivers fails to read a
> > config register from a device.
> >
> > This makes error response fabrication consistent and helps in removal of
> > a lot of repeated code.
> >
> > Suggested-by: Rob Herring <[email protected]>
> > Signed-off-by: Naveen Naidu <[email protected]>
> > ---
> > drivers/pci/access.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> > index 46935695cfb9..b3b2006ed1d2 100644
> > --- a/drivers/pci/access.c
> > +++ b/drivers/pci/access.c
> > @@ -42,7 +42,10 @@ int noinline pci_bus_read_config_##size \
> > if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> > pci_lock_config(flags); \
> > res = bus->ops->read(bus, devfn, pos, len, &data); \
> > - *value = (type)data; \
> > + if (res) \
> > + SET_PCI_ERROR_RESPONSE(value); \
> > + else \
> > + *value = (type)data; \
>
> Hello! Just one small comment. It looks like that in this patch is
> broken alignment of backslashes on the end of lines. Prior this patch
> backslashes on all lines were at the same column. With this patch they
> are not.
>

Thank you for spotting this. I apologise for this. I did not configure
my editor properlly so I couldn't recognize this error. But I'll fix
this when I send the v3

> > pci_unlock_config(flags); \
> > return res; \
> > }
> > @@ -228,7 +231,10 @@ int pci_user_read_config_##size \
> > ret = dev->bus->ops->read(dev->bus, dev->devfn, \
> > pos, sizeof(type), &data); \
> > raw_spin_unlock_irq(&pci_lock); \
> > - *val = (type)data; \
> > + if (ret) \
> > + SET_PCI_ERROR_RESPONSE(val); \
> > + else \
> > + *val = (type)data; \
> > return pcibios_err_to_errno(ret); \
> > } \
> > EXPORT_SYMBOL_GPL(pci_user_read_config_##size);
> > --
> > 2.25.1
> >