| Message ID | alpine.DEB.2.21.2402100048440.2376@angie.orcam.me.uk (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Rework error reporting with PCIe failed link retraining | expand |
On Sat, 10 Feb 2024, Maciej W. Rozycki wrote: > Given how the call place in `pcie_wait_for_link_delay' got structured > now, and that `pcie_retrain_link' returns a potentially useful error > code, convert `pcie_failed_link_retrain' to return an error code rather > than a boolean status, fixing handling at the call site mentioned. > Update the other call site accordingly. > > Fixes: 1abb47390350 ("Merge branch 'pci/enumeration'") > Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Link: https://lore.kernel.org/r/aa2d1c4e-9961-d54a-00c7-ddf8e858a9b0@linux.intel.com/ > Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> > Cc: stable@vger.kernel.org # v6.5+ > --- > drivers/pci/pci.c | 2 +- > drivers/pci/pci.h | 6 +++--- > drivers/pci/quirks.c | 14 +++++++------- > 3 files changed, 11 insertions(+), 11 deletions(-) > > linux-pcie-failed-link-retrain-status-int.diff > Index: linux-macro/drivers/pci/pci.c > =================================================================== > --- linux-macro.orig/drivers/pci/pci.c > +++ linux-macro/drivers/pci/pci.c > @@ -1263,7 +1263,7 @@ static int pci_dev_wait(struct pci_dev * > if (delay > PCI_RESET_WAIT) { > if (retrain) { > retrain = false; > - if (pcie_failed_link_retrain(bridge)) { > + if (pcie_failed_link_retrain(bridge) == 0) { > delay = 1; > continue; > } > Index: linux-macro/drivers/pci/pci.h > =================================================================== > --- linux-macro.orig/drivers/pci/pci.h > +++ linux-macro/drivers/pci/pci.h > @@ -540,7 +540,7 @@ void pci_acs_init(struct pci_dev *dev); > int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags); > int pci_dev_specific_enable_acs(struct pci_dev *dev); > int pci_dev_specific_disable_acs_redir(struct pci_dev *dev); > -bool pcie_failed_link_retrain(struct pci_dev *dev); > +int pcie_failed_link_retrain(struct pci_dev *dev); > #else > static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev, > u16 acs_flags) > @@ -555,9 +555,9 @@ static inline int pci_dev_specific_disab > { > return -ENOTTY; > } > -static inline bool pcie_failed_link_retrain(struct pci_dev *dev) > +static inline int pcie_failed_link_retrain(struct pci_dev *dev) > { > - return false; > + return -ENOTTY; > } > #endif > > Index: linux-macro/drivers/pci/quirks.c > =================================================================== > --- linux-macro.orig/drivers/pci/quirks.c > +++ linux-macro/drivers/pci/quirks.c > @@ -74,17 +74,17 @@ > * firmware may have already arranged and lift it with ports that already > * report their data link being up. > * > - * Return TRUE if the link has been successfully retrained, otherwise FALSE, > + * Return 0 if the link has been successfully retrained, otherwise an error, > * also when retraining was not needed in the first place. > */ > -bool pcie_failed_link_retrain(struct pci_dev *dev) > +int pcie_failed_link_retrain(struct pci_dev *dev) > { > static const struct pci_device_id ids[] = { > { PCI_VDEVICE(ASMEDIA, 0x2824) }, /* ASMedia ASM2824 */ > {} > }; > u16 lnksta, lnkctl2; > - bool ret = false; > + int ret = -ENOTTY; > > if (!pci_is_pcie(dev) || !pcie_downstream_port(dev) || > !pcie_cap_has_lnkctl2(dev) || !dev->link_active_reporting) > @@ -100,8 +100,8 @@ bool pcie_failed_link_retrain(struct pci > lnkctl2 |= PCI_EXP_LNKCTL2_TLS_2_5GT; > pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, lnkctl2); > > - ret = pcie_retrain_link(dev, false) == 0; > - if (!ret) { > + ret = pcie_retrain_link(dev, false); > + if (ret) { > pci_info(dev, "retraining failed\n"); > return ret; > } > @@ -120,8 +120,8 @@ bool pcie_failed_link_retrain(struct pci > lnkctl2 |= lnkcap & PCI_EXP_LNKCAP_SLS; > pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, lnkctl2); > > - ret = pcie_retrain_link(dev, false) == 0; > - if (!ret) { > + ret = pcie_retrain_link(dev, false); > + if (ret) { > pci_info(dev, "retraining failed\n"); > return ret; > } Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> I closed my patch from patchwork that is not need after this change.
Index: linux-macro/drivers/pci/pci.c =================================================================== --- linux-macro.orig/drivers/pci/pci.c +++ linux-macro/drivers/pci/pci.c @@ -1263,7 +1263,7 @@ static int pci_dev_wait(struct pci_dev * if (delay > PCI_RESET_WAIT) { if (retrain) { retrain = false; - if (pcie_failed_link_retrain(bridge)) { + if (pcie_failed_link_retrain(bridge) == 0) { delay = 1; continue; } Index: linux-macro/drivers/pci/pci.h =================================================================== --- linux-macro.orig/drivers/pci/pci.h +++ linux-macro/drivers/pci/pci.h @@ -540,7 +540,7 @@ void pci_acs_init(struct pci_dev *dev); int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags); int pci_dev_specific_enable_acs(struct pci_dev *dev); int pci_dev_specific_disable_acs_redir(struct pci_dev *dev); -bool pcie_failed_link_retrain(struct pci_dev *dev); +int pcie_failed_link_retrain(struct pci_dev *dev); #else static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags) @@ -555,9 +555,9 @@ static inline int pci_dev_specific_disab { return -ENOTTY; } -static inline bool pcie_failed_link_retrain(struct pci_dev *dev) +static inline int pcie_failed_link_retrain(struct pci_dev *dev) { - return false; + return -ENOTTY; } #endif Index: linux-macro/drivers/pci/quirks.c =================================================================== --- linux-macro.orig/drivers/pci/quirks.c +++ linux-macro/drivers/pci/quirks.c @@ -74,17 +74,17 @@ * firmware may have already arranged and lift it with ports that already * report their data link being up. * - * Return TRUE if the link has been successfully retrained, otherwise FALSE, + * Return 0 if the link has been successfully retrained, otherwise an error, * also when retraining was not needed in the first place. */ -bool pcie_failed_link_retrain(struct pci_dev *dev) +int pcie_failed_link_retrain(struct pci_dev *dev) { static const struct pci_device_id ids[] = { { PCI_VDEVICE(ASMEDIA, 0x2824) }, /* ASMedia ASM2824 */ {} }; u16 lnksta, lnkctl2; - bool ret = false; + int ret = -ENOTTY; if (!pci_is_pcie(dev) || !pcie_downstream_port(dev) || !pcie_cap_has_lnkctl2(dev) || !dev->link_active_reporting) @@ -100,8 +100,8 @@ bool pcie_failed_link_retrain(struct pci lnkctl2 |= PCI_EXP_LNKCTL2_TLS_2_5GT; pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, lnkctl2); - ret = pcie_retrain_link(dev, false) == 0; - if (!ret) { + ret = pcie_retrain_link(dev, false); + if (ret) { pci_info(dev, "retraining failed\n"); return ret; } @@ -120,8 +120,8 @@ bool pcie_failed_link_retrain(struct pci lnkctl2 |= lnkcap & PCI_EXP_LNKCAP_SLS; pcie_capability_write_word(dev, PCI_EXP_LNKCTL2, lnkctl2); - ret = pcie_retrain_link(dev, false) == 0; - if (!ret) { + ret = pcie_retrain_link(dev, false); + if (ret) { pci_info(dev, "retraining failed\n"); return ret; }
Given how the call place in `pcie_wait_for_link_delay' got structured now, and that `pcie_retrain_link' returns a potentially useful error code, convert `pcie_failed_link_retrain' to return an error code rather than a boolean status, fixing handling at the call site mentioned. Update the other call site accordingly. Fixes: 1abb47390350 ("Merge branch 'pci/enumeration'") Reported-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Link: https://lore.kernel.org/r/aa2d1c4e-9961-d54a-00c7-ddf8e858a9b0@linux.intel.com/ Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> Cc: stable@vger.kernel.org # v6.5+ --- drivers/pci/pci.c | 2 +- drivers/pci/pci.h | 6 +++--- drivers/pci/quirks.c | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) linux-pcie-failed-link-retrain-status-int.diff