On Wed, Jun 25, 2025 at 12:18:16PM +0200, Christian Bruel wrote: > > > On 6/25/25 06:00, Manivannan Sadhasivam wrote: > > On Tue, Jun 24, 2025 at 05:22:06PM -0500, Bjorn Helgaas wrote: > > > On Mon, Jun 23, 2025 at 06:13:07AM -0600, Manivannan Sadhasivam wrote: > > > > On Tue, 10 Jun 2025 11:07:05 +0200, Christian Bruel wrote: > > > > > Changes in v12; > > > > > Fix warning reported by kernel test robot <lkp@xxxxxxxxx> > > > > > > > > > > Changes in v11; > > > > > Address comments from Manivanna: > > > > > - RC driver: Do not call pm_runtime_get_noresume in probe > > > > > More uses of dev_err_probe > > > > > - EP driver: Use level triggered PERST# irq > > > > > > > > > > [...] > > > > > > > > Applied, thanks! > > > > > > > > [1/9] dt-bindings: PCI: Add STM32MP25 PCIe Root Complex bindings > > > > commit: 41d5cfbdda7a61c5d646a54035b697205cff1cf0 > > > > [2/9] PCI: stm32: Add PCIe host support for STM32MP25 > > > > commit: f6111bc2d8fe6ffc741661126a2174523124dc11 > > > > [3/9] dt-bindings: PCI: Add STM32MP25 PCIe Endpoint bindings > > > > commit: 203cfc4a23506ffb9c48d1300348c290dbf9368e > > > > [4/9] PCI: stm32: Add PCIe Endpoint support for STM32MP25 > > > > commit: 8869fb36a107a9ff18dab8c224de6afff1e81dec > > > > [5/9] MAINTAINERS: add entry for ST STM32MP25 PCIe drivers > > > > commit: 003902ed7778d62083120253cd282a9112674986 > > > > > > This doesn't build for me with the attached config: > > > > > > $ make drivers/pci/controller/dwc/pcie-stm32.o > > > CALL scripts/checksyscalls.sh > > > DESCEND objtool > > > INSTALL libsubcmd_headers > > > CC drivers/pci/controller/dwc/pcie-stm32.o > > > drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_suspend_noirq’: > > > drivers/pci/controller/dwc/pcie-stm32.c:83:16: error: implicit declaration of function ‘pinctrl_pm_select_sleep_state’ [-Werror=implicit-function-declaration] > > > 83 | return pinctrl_pm_select_sleep_state(dev); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > drivers/pci/controller/dwc/pcie-stm32.c: In function ‘stm32_pcie_resume_noirq’: > > > drivers/pci/controller/dwc/pcie-stm32.c:96:24: error: ‘structdevice’ has no member named ‘pins’ > > > 96 | if (!IS_ERR(dev->pins->init_state)) > > > | ^~ > > > drivers/pci/controller/dwc/pcie-stm32.c:97:23: error: implicit declaration of function ‘pinctrl_select_state’ [-Werror=implicit-function-declaration] > > > 97 | ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state); > > > | ^~~~~~~~~~~~~~~~~~~~ > > > drivers/pci/controller/dwc/pcie-stm32.c:97:47: error: ‘structdevice’ has no member named ‘pins’ > > > 97 | ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state); > > > | ^~ > > > drivers/pci/controller/dwc/pcie-stm32.c:97:61: error: ‘structdevice’ has no member named ‘pins’ > > > 97 | ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state); > > > | ^~ > > > drivers/pci/controller/dwc/pcie-stm32.c:99:23: error: implicit declaration of function ‘pinctrl_pm_select_default_state’ [-Werror=implicit-function-declaration] > > > 99 | ret = pinctrl_pm_select_default_state(dev); > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > > Hmm... I see two issues here. First is, wrong pinctrl header used. The correct > > one is: > > > > #include <linux/pinctrl/consumer.h> > > ah yes, the missing pinctrl_pm_select_default_state() should indeed be fixed > by using the correct header. > I've fixed it in the branch. > > > > Second issue is the driver accessing "struct device::pins" directly. The "pins" > > member won't be available if CONFIG_PINCTRL is not set (which is what your > > .config has). So either the member should not be accessed directly or the > > driver has to depend on CONFIG_PINCTRL. The latter one is not acceptable.It > > also looks weird that only this driver is accessing the "pins" member directly > > apart from the pinctrl core. So I think this part needs a revisit. > > > > Christian? > The pinctrl "init" and "default" configurations are managed effectively by > the probing code. The same approach is required in > stm32_pcie_resume_noirq(). > > In this case, would introducing a new helper function, > pinctrl_pm_select_init_state(), be preferable, even if we are the only > consumer? > Sounds reasonable. If you end up creating an API, get it acked by the pinctrl maintainer so that I can merge it (and the use of it in this driver) through the PCI tree to avoid cross tree dependency. - Mani -- மணிவண்ணன் சதாசிவம்
