| Message ID | 20240508120700.51374-1-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ae9daffd9028f2500c9ac1517e46d4f2b57efb80 |
| Headers | show |
| Series | [1/1] MIPS: Routerboard 532: Fix vendor retry check code | expand |
On Wed, May 08, 2024 at 03:07:00PM +0300, Ilpo Järvinen wrote: > read_config_dword() contains strange condition checking ret for a > number of values. The ret variable, however, is always zero because > config_access() never returns anything else. Thus, the retry is always > taken until number of tries is exceeded. > > The code looks like it wants to check *val instead of ret to see if the > read gave an error response. > > Fixes: 73b4390fb234 ("[MIPS] Routerboard 532: Support for base system") > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > -- > > IMPORTANT NOTE TO MAINTAINER: > > This change has potential of breaking something and I don't have HW to > test this with. I only came across it while going through all PCIBIOS_* > call chains. Clearly the current code non-sensical so something is not > right but whether this is the right way to solve the problem, I'm not > entirely sure because it will make small change into the behavior. I have rework of this code sitting in one of my development branches and I didn't spot the bug in the old code, but the new code does what your fix is doing. So I'm pretty sure this change is correct. > --- > arch/mips/pci/ops-rc32434.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) applied to mips-fixes. Thomas.
diff --git a/arch/mips/pci/ops-rc32434.c b/arch/mips/pci/ops-rc32434.c index 874ed6df9768..34b9323bdabb 100644 --- a/arch/mips/pci/ops-rc32434.c +++ b/arch/mips/pci/ops-rc32434.c @@ -112,8 +112,8 @@ static int read_config_dword(struct pci_bus *bus, unsigned int devfn, * gives them time to settle */ if (where == PCI_VENDOR_ID) { - if (ret == 0xffffffff || ret == 0x00000000 || - ret == 0x0000ffff || ret == 0xffff0000) { + if (*val == 0xffffffff || *val == 0x00000000 || + *val == 0x0000ffff || *val == 0xffff0000) { if (delay > 4) return 0; delay *= 2;
read_config_dword() contains strange condition checking ret for a number of values. The ret variable, however, is always zero because config_access() never returns anything else. Thus, the retry is always taken until number of tries is exceeded. The code looks like it wants to check *val instead of ret to see if the read gave an error response. Fixes: 73b4390fb234 ("[MIPS] Routerboard 532: Support for base system") Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> -- IMPORTANT NOTE TO MAINTAINER: This change has potential of breaking something and I don't have HW to test this with. I only came across it while going through all PCIBIOS_* call chains. Clearly the current code non-sensical so something is not right but whether this is the right way to solve the problem, I'm not entirely sure because it will make small change into the behavior. --- arch/mips/pci/ops-rc32434.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)