| Message ID | 20250524185304.26698-3-manivannan.sadhasivam@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Slot reset fixes | expand |
Hi Manivannan, kernel test robot noticed the following build errors: [auto build test ERROR on pci/next] [also build test ERROR on next-20250523] [cannot apply to pci/for-linus linus/master v6.15-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam/PCI-Save-and-restore-root-port-config-space-in-pcibios_reset_secondary_bus/20250525-025535 base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next patch link: https://lore.kernel.org/r/20250524185304.26698-3-manivannan.sadhasivam%40linaro.org patch subject: [PATCH 2/2] PCI: Rename host_bridge::reset_slot() to host_bridge::reset_root_port() config: csky-randconfig-002-20250525 (https://download.01.org/0day-ci/archive/20250525/202505250525.2csmeURe-lkp@intel.com/config) compiler: csky-linux-gcc (GCC) 10.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250525/202505250525.2csmeURe-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505250525.2csmeURe-lkp@intel.com/ All error/warnings (new ones prefixed by >>): drivers/pci/controller/dwc/pcie-dw-rockchip.c: In function 'rockchip_pcie_host_init': >> drivers/pci/controller/dwc/pcie-dw-rockchip.c:264:32: error: 'rockchip_pcie_rc_reset_slot' undeclared (first use in this function); did you mean 'rockchip_pcie_rc_reset_root_port'? 264 | pp->bridge->reset_root_port = rockchip_pcie_rc_reset_slot; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ | rockchip_pcie_rc_reset_root_port drivers/pci/controller/dwc/pcie-dw-rockchip.c:264:32: note: each undeclared identifier is reported only once for each function it appears in At top level: >> drivers/pci/controller/dwc/pcie-dw-rockchip.c:703:12: warning: 'rockchip_pcie_rc_reset_root_port' defined but not used [-Wunused-function] 703 | static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +264 drivers/pci/controller/dwc/pcie-dw-rockchip.c 244 245 static int rockchip_pcie_host_init(struct dw_pcie_rp *pp) 246 { 247 struct dw_pcie *pci = to_dw_pcie_from_pp(pp); 248 struct rockchip_pcie *rockchip = to_rockchip_pcie(pci); 249 struct device *dev = rockchip->pci.dev; 250 int irq, ret; 251 252 irq = of_irq_get_byname(dev->of_node, "legacy"); 253 if (irq < 0) 254 return irq; 255 256 ret = rockchip_pcie_init_irq_domain(rockchip); 257 if (ret < 0) 258 dev_err(dev, "failed to init irq domain\n"); 259 260 irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler, 261 rockchip); 262 263 rockchip_pcie_enable_l0s(pci); > 264 pp->bridge->reset_root_port = rockchip_pcie_rc_reset_slot; 265 266 return 0; 267 } 268
On Sat, 2025-05-24 at 22:57 +0200, Niklas Cassel wrote: > On Sun, May 25, 2025 at 12:23:04AM +0530, Manivannan Sadhasivam > wrote: > > The callback is supposed to reset the root port, hence it should be > > named > > as 'reset_root_port'. This also warrants renaming the rest of the > > instances > > of 'reset slot' as 'reset root port' in the drivers. > > > > Suggested-by: Lukas Wunner <lukas@wunner.de> > > Signed-off-by: Manivannan Sadhasivam > > <manivannan.sadhasivam@linaro.org> > > --- > > 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 | 6 +++--- > > include/linux/pci.h | 2 +- > > 5 files changed, 22 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > index 193e97adf228..0cc7186758ce 100644 > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > @@ -85,7 +85,7 @@ struct rockchip_pcie_of_data { > > const struct pci_epc_features *epc_features; > > }; > > > > -static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge > > *bridge, > > +static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge > > *bridge, > > struct pci_dev *pdev); > > > > static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, > > u32 reg) > > @@ -261,7 +261,7 @@ static int rockchip_pcie_host_init(struct > > dw_pcie_rp *pp) > > rockchip); > > > > rockchip_pcie_enable_l0s(pci); > > - pp->bridge->reset_slot = rockchip_pcie_rc_reset_slot; > > + pp->bridge->reset_root_port = rockchip_pcie_rc_reset_slot; > > You just renamed the function to rockchip_pcie_rc_reset_root_port(), > but you seem to use the old name here, so I would guess that this > will > not compile. > > With the function pointer renamed, this patch looks good to me: > Reviewed-by: Niklas Cassel <cassel@kernel.org> > Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> >
Eight more occurrences where "Root Port" should be capitalized: On Sun, May 25, 2025 at 12:23:04AM +0530, Manivannan Sadhasivam wrote: > @@ -759,7 +759,7 @@ static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge *bridge, > > /* Ignore errors, the link may come up later. */ > dw_pcie_wait_for_link(pci); > - dev_dbg(dev, "slot reset completed\n"); > + dev_dbg(dev, "Root port reset completed\n"); > return ret; > > deinit_clk: > @@ -1589,7 +1589,7 @@ static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge, > > qcom_pcie_start_link(pci); > > - dev_dbg(dev, "Slot reset completed\n"); > + dev_dbg(dev, "Root port reset completed\n"); > > return 0; > > @@ -99,22 +99,22 @@ void pci_host_common_remove(struct platform_device *pdev) > ret = pci_bus_error_reset(dev); > if (ret) { > - pci_err(dev, "Failed to reset slot: %d\n", ret); > + pci_err(dev, "Failed to reset root port: %d\n", ret); > return PCI_ERS_RESULT_DISCONNECT; > } > > - pci_info(dev, "Slot has been reset\n"); > + pci_info(dev, "Root port has been reset\n"); > > return PCI_ERS_RESULT_RECOVERED; > } > @@ -140,17 +140,17 @@ static void pci_host_recover_slots(struct pci_host_bridge *host) > > ret = pci_bus_error_reset(dev); > if (ret) > - pci_err(dev, "Failed to reset slot: %d\n", ret); > + pci_err(dev, "Failed to reset root port: %d\n", ret); > else > - pci_info(dev, "Slot has been reset\n"); > + pci_info(dev, "Root port has been reset\n"); > } > } > #endif > > void pci_host_handle_link_down(struct pci_host_bridge *bridge) > { > - dev_info(&bridge->dev, "Recovering slots due to Link Down\n"); > - pci_host_recover_slots(bridge); > + dev_info(&bridge->dev, "Recovering root ports due to Link Down\n"); > + pci_host_reset_root_ports(bridge); > } > EXPORT_SYMBOL_GPL(pci_host_handle_link_down); > > @@ -4985,16 +4985,16 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) > + ret = host->reset_root_port(host, dev); > if (ret) > - pci_err(dev, "failed to reset slot: %d\n", ret); > + pci_err(dev, "failed to reset root port: %d\n", ret); > else > /* Now restore it on success */ > pci_restore_state(dev);
On Sat, May 24, 2025 at 10:57:44PM +0200, Niklas Cassel wrote: > On Sun, May 25, 2025 at 12:23:04AM +0530, Manivannan Sadhasivam wrote: > > The callback is supposed to reset the root port, hence it should be named > > as 'reset_root_port'. This also warrants renaming the rest of the instances > > of 'reset slot' as 'reset root port' in the drivers. > > > > Suggested-by: Lukas Wunner <lukas@wunner.de> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > 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 | 6 +++--- > > include/linux/pci.h | 2 +- > > 5 files changed, 22 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > index 193e97adf228..0cc7186758ce 100644 > > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > > @@ -85,7 +85,7 @@ struct rockchip_pcie_of_data { > > const struct pci_epc_features *epc_features; > > }; > > > > -static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge *bridge, > > +static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge, > > struct pci_dev *pdev); > > > > static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, u32 reg) > > @@ -261,7 +261,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp) > > rockchip); > > > > rockchip_pcie_enable_l0s(pci); > > - pp->bridge->reset_slot = rockchip_pcie_rc_reset_slot; > > + pp->bridge->reset_root_port = rockchip_pcie_rc_reset_slot; > > You just renamed the function to rockchip_pcie_rc_reset_root_port(), > but you seem to use the old name here, so I would guess that this will > not compile. > Yeah, I guess I exposed my sed skills here :P Will fix it up while applying. > With the function pointer renamed, this patch looks good to me: > Reviewed-by: Niklas Cassel <cassel@kernel.org> > Thanks! - Mani
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index 193e97adf228..0cc7186758ce 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -85,7 +85,7 @@ struct rockchip_pcie_of_data { const struct pci_epc_features *epc_features; }; -static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge *bridge, +static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge, struct pci_dev *pdev); static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip, u32 reg) @@ -261,7 +261,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp) rockchip); rockchip_pcie_enable_l0s(pci); - pp->bridge->reset_slot = rockchip_pcie_rc_reset_slot; + pp->bridge->reset_root_port = rockchip_pcie_rc_reset_slot; return 0; } @@ -700,7 +700,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev) return ret; } -static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge *bridge, +static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge, struct pci_dev *pdev) { struct pci_bus *bus = bridge->bus; @@ -759,7 +759,7 @@ static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge *bridge, /* Ignore errors, the link may come up later. */ dw_pcie_wait_for_link(pci); - dev_dbg(dev, "slot reset completed\n"); + dev_dbg(dev, "Root port reset completed\n"); return ret; deinit_clk: diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 0c59030a2d55..840263c1efe0 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -291,7 +291,7 @@ struct qcom_pcie { }; #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) -static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge, +static int qcom_pcie_reset_root_port(struct pci_host_bridge *bridge, struct pci_dev *pdev); static void qcom_ep_reset_assert(struct qcom_pcie *pcie) @@ -1277,7 +1277,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp) goto err_assert_reset; } - pp->bridge->reset_slot = qcom_pcie_reset_slot; + pp->bridge->reset_root_port = qcom_pcie_reset_root_port; return 0; @@ -1533,7 +1533,7 @@ static void qcom_pcie_icc_opp_update(struct qcom_pcie *pcie) } } -static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge, +static int qcom_pcie_reset_root_port(struct pci_host_bridge *bridge, struct pci_dev *pdev) { struct pci_bus *bus = bridge->bus; @@ -1589,7 +1589,7 @@ static int qcom_pcie_reset_slot(struct pci_host_bridge *bridge, qcom_pcie_start_link(pci); - dev_dbg(dev, "Slot reset completed\n"); + dev_dbg(dev, "Root port reset completed\n"); return 0; diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c index afa7b140a04a..24e357e85adb 100644 --- a/drivers/pci/controller/pci-host-common.c +++ b/drivers/pci/controller/pci-host-common.c @@ -99,22 +99,22 @@ void pci_host_common_remove(struct platform_device *pdev) EXPORT_SYMBOL_GPL(pci_host_common_remove); #if IS_ENABLED(CONFIG_PCIEAER) -static pci_ers_result_t pci_host_reset_slot(struct pci_dev *dev) +static pci_ers_result_t pci_host_reset_root_port(struct pci_dev *dev) { int ret; ret = pci_bus_error_reset(dev); if (ret) { - pci_err(dev, "Failed to reset slot: %d\n", ret); + pci_err(dev, "Failed to reset root port: %d\n", ret); return PCI_ERS_RESULT_DISCONNECT; } - pci_info(dev, "Slot has been reset\n"); + pci_info(dev, "Root port has been reset\n"); return PCI_ERS_RESULT_RECOVERED; } -static void pci_host_recover_slots(struct pci_host_bridge *host) +static void pci_host_reset_root_ports(struct pci_host_bridge *host) { struct pci_bus *bus = host->bus; struct pci_dev *dev; @@ -124,11 +124,11 @@ static void pci_host_recover_slots(struct pci_host_bridge *host) continue; pcie_do_recovery(dev, pci_channel_io_frozen, - pci_host_reset_slot); + pci_host_reset_root_port); } } #else -static void pci_host_recover_slots(struct pci_host_bridge *host) +static void pci_host_reset_root_ports(struct pci_host_bridge *host) { struct pci_bus *bus = host->bus; struct pci_dev *dev; @@ -140,17 +140,17 @@ static void pci_host_recover_slots(struct pci_host_bridge *host) ret = pci_bus_error_reset(dev); if (ret) - pci_err(dev, "Failed to reset slot: %d\n", ret); + pci_err(dev, "Failed to reset root port: %d\n", ret); else - pci_info(dev, "Slot has been reset\n"); + pci_info(dev, "Root port has been reset\n"); } } #endif void pci_host_handle_link_down(struct pci_host_bridge *bridge) { - dev_info(&bridge->dev, "Recovering slots due to Link Down\n"); - pci_host_recover_slots(bridge); + dev_info(&bridge->dev, "Recovering root ports due to Link Down\n"); + pci_host_reset_root_ports(bridge); } EXPORT_SYMBOL_GPL(pci_host_handle_link_down); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6d6e9ce2bbcc..154d33e1af84 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4985,16 +4985,16 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) struct pci_host_bridge *host = pci_find_host_bridge(dev->bus); int ret; - if (pci_is_root_bus(dev->bus) && host->reset_slot) { + if (pci_is_root_bus(dev->bus) && host->reset_root_port) { /* * 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); + ret = host->reset_root_port(host, dev); if (ret) - pci_err(dev, "failed to reset slot: %d\n", ret); + pci_err(dev, "failed to reset root port: %d\n", ret); else /* Now restore it on success */ pci_restore_state(dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index 8d7d2a49b76c..ab4f4a668f6d 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -599,7 +599,7 @@ struct pci_host_bridge { void (*release_fn)(struct pci_host_bridge *); int (*enable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); void (*disable_device)(struct pci_host_bridge *bridge, struct pci_dev *dev); - int (*reset_slot)(struct pci_host_bridge *bridge, struct pci_dev *dev); + int (*reset_root_port)(struct pci_host_bridge *bridge, struct pci_dev *dev); void *release_data; unsigned int ignore_reset_delay:1; /* For entire hierarchy */ unsigned int no_ext_tags:1; /* No Extended Tags */
The callback is supposed to reset the root port, hence it should be named as 'reset_root_port'. This also warrants renaming the rest of the instances of 'reset slot' as 'reset root port' in the drivers. Suggested-by: Lukas Wunner <lukas@wunner.de> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- 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 | 6 +++--- include/linux/pci.h | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-)