| Message ID | 20200601130315.18895-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | b1bd5714472cc72e14409f5659b154c765a76c65 |
| Headers | show |
| Series | PCI: aardvark: Indicate error in 'val' when config read fails | expand |
Hello Lorenzo! Could you please review this patch? On Monday 01 June 2020 15:03:15 Pali Rohár wrote: > Most callers of config read do not check for return value. But most of the > ones that do, checks for error indication in 'val' variable. > > This patch updates error handling in advk_pcie_rd_conf() function. If PIO > transfer fails then 'val' variable is set to 0xffffffff which indicates > failture. > > Signed-off-by: Pali Rohár <pali@kernel.org> I should add credit for Bjorn as he found this issue Reported-by: Bjorn Helgaas <helgaas@kernel.org> > --- > drivers/pci/controller/pci-aardvark.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > index 53a4cfd7d377..783a7f1f2c44 100644 > --- a/drivers/pci/controller/pci-aardvark.c > +++ b/drivers/pci/controller/pci-aardvark.c > @@ -691,8 +691,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, > advk_writel(pcie, 1, PIO_START); > > ret = advk_pcie_wait_pio(pcie); > - if (ret < 0) > + if (ret < 0) { > + *val = 0xffffffff; > return PCIBIOS_SET_FAILED; > + } > > advk_pcie_check_pio_status(pcie); > > -- > 2.20.1 >
On Fri, Jun 19, 2020 at 12:56:18PM +0200, Pali Rohár wrote: > Hello Lorenzo! Could you please review this patch? > > On Monday 01 June 2020 15:03:15 Pali Rohár wrote: > > Most callers of config read do not check for return value. But most of the > > ones that do, checks for error indication in 'val' variable. > > > > This patch updates error handling in advk_pcie_rd_conf() function. If PIO > > transfer fails then 'val' variable is set to 0xffffffff which indicates > > failture. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > I should add credit for Bjorn as he found this issue Could you provide a lore archive link to the relevant discussion please ? I will apply it then. Lorenzo > Reported-by: Bjorn Helgaas <helgaas@kernel.org> > > > --- > > drivers/pci/controller/pci-aardvark.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > > index 53a4cfd7d377..783a7f1f2c44 100644 > > --- a/drivers/pci/controller/pci-aardvark.c > > +++ b/drivers/pci/controller/pci-aardvark.c > > @@ -691,8 +691,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, > > advk_writel(pcie, 1, PIO_START); > > > > ret = advk_pcie_wait_pio(pcie); > > - if (ret < 0) > > + if (ret < 0) { > > + *val = 0xffffffff; > > return PCIBIOS_SET_FAILED; > > + } > > > > advk_pcie_check_pio_status(pcie); > > > > -- > > 2.20.1 > >
On Tuesday 07 July 2020 14:53:11 Lorenzo Pieralisi wrote: > On Fri, Jun 19, 2020 at 12:56:18PM +0200, Pali Rohár wrote: > > Hello Lorenzo! Could you please review this patch? > > > > On Monday 01 June 2020 15:03:15 Pali Rohár wrote: > > > Most callers of config read do not check for return value. But most of the > > > ones that do, checks for error indication in 'val' variable. > > > > > > This patch updates error handling in advk_pcie_rd_conf() function. If PIO > > > transfer fails then 'val' variable is set to 0xffffffff which indicates > > > failture. > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > I should add credit for Bjorn as he found this issue > > Could you provide a lore archive link to the relevant > discussion please ? I will apply it then. Hello Lorenzo! Here is link to the Bjorn's email: https://lore.kernel.org/linux-pci/20200528162604.GA323482@bjorn-Precision-5520/ > Lorenzo > > > Reported-by: Bjorn Helgaas <helgaas@kernel.org> > > > > > --- > > > drivers/pci/controller/pci-aardvark.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > > > index 53a4cfd7d377..783a7f1f2c44 100644 > > > --- a/drivers/pci/controller/pci-aardvark.c > > > +++ b/drivers/pci/controller/pci-aardvark.c > > > @@ -691,8 +691,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, > > > advk_writel(pcie, 1, PIO_START); > > > > > > ret = advk_pcie_wait_pio(pcie); > > > - if (ret < 0) > > > + if (ret < 0) { > > > + *val = 0xffffffff; > > > return PCIBIOS_SET_FAILED; > > > + } > > > > > > advk_pcie_check_pio_status(pcie); > > > > > > -- > > > 2.20.1 > > >
On Tue, Jul 07, 2020 at 04:02:44PM +0200, Pali Rohár wrote: > On Tuesday 07 July 2020 14:53:11 Lorenzo Pieralisi wrote: > > On Fri, Jun 19, 2020 at 12:56:18PM +0200, Pali Rohár wrote: > > > Hello Lorenzo! Could you please review this patch? > > > > > > On Monday 01 June 2020 15:03:15 Pali Rohár wrote: > > > > Most callers of config read do not check for return value. But most of the > > > > ones that do, checks for error indication in 'val' variable. > > > > > > > > This patch updates error handling in advk_pcie_rd_conf() function. If PIO > > > > transfer fails then 'val' variable is set to 0xffffffff which indicates > > > > failture. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > I should add credit for Bjorn as he found this issue > > > > Could you provide a lore archive link to the relevant > > discussion please ? I will apply it then. > > Hello Lorenzo! Here is link to the Bjorn's email: > https://lore.kernel.org/linux-pci/20200528162604.GA323482@bjorn-Precision-5520/ Thanks applied to pci/aardvark. Lorenzo > > Lorenzo > > > > > Reported-by: Bjorn Helgaas <helgaas@kernel.org> > > > > > > > --- > > > > drivers/pci/controller/pci-aardvark.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c > > > > index 53a4cfd7d377..783a7f1f2c44 100644 > > > > --- a/drivers/pci/controller/pci-aardvark.c > > > > +++ b/drivers/pci/controller/pci-aardvark.c > > > > @@ -691,8 +691,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, > > > > advk_writel(pcie, 1, PIO_START); > > > > > > > > ret = advk_pcie_wait_pio(pcie); > > > > - if (ret < 0) > > > > + if (ret < 0) { > > > > + *val = 0xffffffff; > > > > return PCIBIOS_SET_FAILED; > > > > + } > > > > > > > > advk_pcie_check_pio_status(pcie); > > > > > > > > -- > > > > 2.20.1 > > > >
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index 53a4cfd7d377..783a7f1f2c44 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -691,8 +691,10 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn, advk_writel(pcie, 1, PIO_START); ret = advk_pcie_wait_pio(pcie); - if (ret < 0) + if (ret < 0) { + *val = 0xffffffff; return PCIBIOS_SET_FAILED; + } advk_pcie_check_pio_status(pcie);
Most callers of config read do not check for return value. But most of the ones that do, checks for error indication in 'val' variable. This patch updates error handling in advk_pcie_rd_conf() function. If PIO transfer fails then 'val' variable is set to 0xffffffff which indicates failture. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/controller/pci-aardvark.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)