Message ID | 20250524185304.26698-1-manivannan.sadhasivam@linaro.org |
---|---|
Headers | show |
Series | PCI: Slot reset fixes | expand |
On Sun, May 25, 2025 at 12:23:03AM +0530, Manivannan Sadhasivam wrote: > host_bridge::reset_slot() is supposed to reset the PCI root port/slot. Once > that happens, the config space content would be lost. This was reported by > Niklas on the dw-rockchip based platform where the MPS setting of the root > port was lost after the host_bridge::reset_slot() callback. Hence, save the > config space before calling the host_bridge::reset_slot() callback and > restore it afterwards. > > While at it, make sure that the callback is only called for root ports by > checking if the bridge is behind the root bus. > > Fixes: d5c1e1c25b37 ("PCI/ERR: Add support for resetting the slots in a platform specific way") > Reported-by: Niklas Cassel <cassel@kernel.org> > Closes: https://lore.kernel.org/linux-pci/aC9OrPAfpzB_A4K2@ryzen > Suggested-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/pci.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 4d396bbab4a8..6d6e9ce2bbcc 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -4985,10 +4985,19 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) > struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); > int ret; > > - if (host->reset_slot) { > + if (pci_is_root_bus(dev->bus) && host->reset_slot) { > + /* > + * Save the config space of the root port before doing the > + * reset, since the state could be lost. The device state > + * should've been saved by the caller. > + */ > + pci_save_state(dev); > ret = host->reset_slot(host, dev); > if (ret) > pci_err(dev, "failed to reset slot: %d\n", ret); > + else > + /* Now restore it on success */ > + pci_restore_state(dev); > > return; > } > -- > 2.43.0 > Looks good to me: Reviewed-by: Niklas Cassel <cassel@kernel.org>
On Sun, 2025-05-25 at 00:23 +0530, Manivannan Sadhasivam wrote: > host_bridge::reset_slot() is supposed to reset the PCI root > port/slot. Once > that happens, the config space content would be lost. This was > reported by > Niklas on the dw-rockchip based platform where the MPS setting of the > root > port was lost after the host_bridge::reset_slot() callback. Hence, > save the > config space before calling the host_bridge::reset_slot() callback > and > restore it afterwards. > > While at it, make sure that the callback is only called for root > ports by > checking if the bridge is behind the root bus. > > Fixes: d5c1e1c25b37 ("PCI/ERR: Add support for resetting the slots in > a platform specific way") > Reported-by: Niklas Cassel <cassel@kernel.org> > Closes: https://lore.kernel.org/linux-pci/aC9OrPAfpzB_A4K2@ryzen > Suggested-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/pci.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 4d396bbab4a8..6d6e9ce2bbcc 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -4985,10 +4985,19 @@ void __weak > pcibios_reset_secondary_bus(struct pci_dev *dev) > struct pci_host_bridge *host = pci_find_host_bridge(dev- > >bus); > int ret; > > - if (host->reset_slot) { > + if (pci_is_root_bus(dev->bus) && host->reset_slot) { > + /* > + * Save the config space of the root port before > doing the > + * reset, since the state could be lost. The device > state > + * should've been saved by the caller. > + */ > + pci_save_state(dev); > ret = host->reset_slot(host, dev); > if (ret) > pci_err(dev, "failed to reset slot: %d\n", > ret); > + else > + /* Now restore it on success */ > + pci_restore_state(dev); > > return; > } Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
On Sun, 2025-05-25 at 00:23 +0530, Manivannan Sadhasivam wrote: > Hi, > > This series fixes the issues reported for the slot reset feature > merged for > v6.16. > > This series is on top of dw-rockchip branch where the slot reset > patches are > merged. The patches in this series can be squashed into the > respective commits > since they are not merged into mainline. > > - Mani > > Manivannan Sadhasivam (2): > PCI: Save and restore root port config space in > pcibios_reset_secondary_bus() > PCI: Rename host_bridge::reset_slot() to > host_bridge::reset_root_port() > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 8 ++++---- > drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++---- > drivers/pci/controller/pci-host-common.c | 20 +++++++++-------- > -- > drivers/pci/pci.c | 15 +++++++++++--- > include/linux/pci.h | 2 +- > 5 files changed, 31 insertions(+), 22 deletions(-) Hey Mani, I tested this series with the Rock5B RC <-> Rock5B EP. As expected, bus resets now work as intended. Feel free to use: Tested-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> Cheers, Wilfred
On Sun, May 25, 2025 at 12:23:02AM +0530, Manivannan Sadhasivam wrote: > Hi, > > This series fixes the issues reported for the slot reset feature merged for > v6.16. > > This series is on top of dw-rockchip branch where the slot reset patches are > merged. The patches in this series can be squashed into the respective commits > since they are not merged into mainline. > Squashed to dw-rockchip! - Mani > - Mani > > Manivannan Sadhasivam (2): > PCI: Save and restore root port config space in > pcibios_reset_secondary_bus() > PCI: Rename host_bridge::reset_slot() to > host_bridge::reset_root_port() > > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 8 ++++---- > drivers/pci/controller/dwc/pcie-qcom.c | 8 ++++---- > drivers/pci/controller/pci-host-common.c | 20 +++++++++---------- > drivers/pci/pci.c | 15 +++++++++++--- > include/linux/pci.h | 2 +- > 5 files changed, 31 insertions(+), 22 deletions(-) > > -- > 2.43.0 >