[PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume_noirq()

[PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume_noirq()

[Christian Bruel]

Replace direct access to dev->pins->init_state with the new helper
pinctrl_pm_select_init_state() to select the init pinctrl state.
This fixes build issues when CONFIG_PINCTRL is not defined.

Depends-on: <20250813081139.93201-3-christian.bruel@foss.st.com>
Reported-by: Bjorn Helgaas <bhelgaas@google.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506260920.bmQ9hQ9s-lkp@intel.com/
Fixes: 633f42f48af5 ("PCI: stm32: Add PCIe host support for STM32MP25")
Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
Changes in v1:
 - pinctrl_pm_select_init_state() return 0 if the state is not defined.
   No need to test as pinctrl_pm_select_default_state() is called.
---
 drivers/pci/controller/dwc/pcie-stm32.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
index 50fae5f5ced2..8501b9ed0633 100644
--- a/drivers/pci/controller/dwc/pcie-stm32.c
+++ b/drivers/pci/controller/dwc/pcie-stm32.c
@@ -90,14 +90,10 @@ static int stm32_pcie_resume_noirq(struct device *dev)
 
 	/*
 	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
-	 * thus if no device is present, must force it low with an init pinmux
-	 * to be able to access the DBI registers.
+	 * thus if no device is present, must deassert it with a GPIO from
+	 * pinctrl pinmux before accessing the DBI registers.
 	 */
-	if (!IS_ERR(dev->pins->init_state))
-		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
-	else
-		ret = pinctrl_pm_select_default_state(dev);
-
+	ret = pinctrl_pm_select_init_state(dev);
 	if (ret) {
 		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
 		return ret;
-- 
2.34.1

Re: [PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume_noirq()

[Bjorn Helgaas]

On Wed, Aug 13, 2025 at 01:53:19PM +0200, Christian Bruel wrote:
> Replace direct access to dev->pins->init_state with the new helper
> pinctrl_pm_select_init_state() to select the init pinctrl state.
> This fixes build issues when CONFIG_PINCTRL is not defined.
> 
> Depends-on: <20250813081139.93201-3-christian.bruel@foss.st.com>
> Reported-by: Bjorn Helgaas <bhelgaas@google.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506260920.bmQ9hQ9s-lkp@intel.com/
> Fixes: 633f42f48af5 ("PCI: stm32: Add PCIe host support for STM32MP25")
> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>

I can't merge 633f42f48af5 as-is because of the build issue.

Pinctrl provides stubs for the non-CONFIG_PINCTRL case; the issue is
that 633f42f48af5 uses dev->pins, which only exists when
CONFIG_PINCTRL is enabled.  

The possibilities I see are:

  1) Merge initial stm32 without suspend/resume support via PCI, merge
     pinctrl_pm_select_init_state() via pinctrl, then add stm32
     suspend/resume support.  pinctrl_pm_select_init_state() and stm32
     (without suspend/resume) would appear in v6.18, and stm32
     suspend/resume would be added in v6.19.

  2) Temporarily #ifdef the dev->pins use.  pinctrl_pm_select_init_state()
     and stm32 (with #ifdef) would appear in v6.18, follow-on patch to
     replace #ifdef with pinctrl_pm_select_init_state() would appear
     in v6.19.

  3) Merge your [1] to add pinctrl_pm_select_init_state() via PCI with
     Linus's ack, followed by the stm32 series with the change below
     squashed in.  Everything would appear in v6.18.

I'm OK with any of these.

[1] https://lore.kernel.org/r/20250813081139.93201-1-christian.bruel@foss.st.com

> ---
> Changes in v1:
>  - pinctrl_pm_select_init_state() return 0 if the state is not defined.
>    No need to test as pinctrl_pm_select_default_state() is called.
> ---
>  drivers/pci/controller/dwc/pcie-stm32.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
> index 50fae5f5ced2..8501b9ed0633 100644
> --- a/drivers/pci/controller/dwc/pcie-stm32.c
> +++ b/drivers/pci/controller/dwc/pcie-stm32.c
> @@ -90,14 +90,10 @@ static int stm32_pcie_resume_noirq(struct device *dev)
>  
>  	/*
>  	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
> -	 * thus if no device is present, must force it low with an init pinmux
> -	 * to be able to access the DBI registers.
> +	 * thus if no device is present, must deassert it with a GPIO from
> +	 * pinctrl pinmux before accessing the DBI registers.
>  	 */
> -	if (!IS_ERR(dev->pins->init_state))
> -		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> -	else
> -		ret = pinctrl_pm_select_default_state(dev);
> -
> +	ret = pinctrl_pm_select_init_state(dev);
>  	if (ret) {
>  		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
>  		return ret;
> -- 
> 2.34.1
> 

Re: [PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume_noirq()

[Christian Bruel]

On 8/13/25 21:06, Bjorn Helgaas wrote:
> On Wed, Aug 13, 2025 at 01:53:19PM +0200, Christian Bruel wrote:
>> Replace direct access to dev->pins->init_state with the new helper
>> pinctrl_pm_select_init_state() to select the init pinctrl state.
>> This fixes build issues when CONFIG_PINCTRL is not defined.
>>
>> Depends-on: <20250813081139.93201-3-christian.bruel@foss.st.com>
>> Reported-by: Bjorn Helgaas <bhelgaas@google.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202506260920.bmQ9hQ9s-lkp@intel.com/
>> Fixes: 633f42f48af5 ("PCI: stm32: Add PCIe host support for STM32MP25")
>> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
> 
> I can't merge 633f42f48af5 as-is because of the build issue.
> 
> Pinctrl provides stubs for the non-CONFIG_PINCTRL case; the issue is
> that 633f42f48af5 uses dev->pins, which only exists when
> CONFIG_PINCTRL is enabled.
> 
> The possibilities I see are:
> 
>    1) Merge initial stm32 without suspend/resume support via PCI, merge
>       pinctrl_pm_select_init_state() via pinctrl, then add stm32
>       suspend/resume support.  pinctrl_pm_select_init_state() and stm32
>       (without suspend/resume) would appear in v6.18, and stm32
>       suspend/resume would be added in v6.19.
> 
>    2) Temporarily #ifdef the dev->pins use.  pinctrl_pm_select_init_state()
>       and stm32 (with #ifdef) would appear in v6.18, follow-on patch to
>       replace #ifdef with pinctrl_pm_select_init_state() would appear
>       in v6.19.
>    3) Merge your [1] to add pinctrl_pm_select_init_state() via PCI with
>       Linus's ack, followed by the stm32 series with the change below
>       squashed in.  Everything would appear in v6.18.

or 4) Linus applies it in his PINCTRL branch and Mani cherry-pick it in 
PCI. But that will be a conflict to solve when both comes to mainline.

Personally I prefer 3) if Linus Acks, to break the dependency

Thank you

Christian

> 
> I'm OK with any of these.
> 
> [1] https://lore.kernel.org/r/20250813081139.93201-1-christian.bruel@foss.st.com
> 
>> ---
>> Changes in v1:
>>   - pinctrl_pm_select_init_state() return 0 if the state is not defined.
>>     No need to test as pinctrl_pm_select_default_state() is called.
>> ---
>>   drivers/pci/controller/dwc/pcie-stm32.c | 10 +++-------
>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
>> index 50fae5f5ced2..8501b9ed0633 100644
>> --- a/drivers/pci/controller/dwc/pcie-stm32.c
>> +++ b/drivers/pci/controller/dwc/pcie-stm32.c
>> @@ -90,14 +90,10 @@ static int stm32_pcie_resume_noirq(struct device *dev)
>>   
>>   	/*
>>   	 * The core clock is gated with CLKREQ# from the COMBOPHY REFCLK,
>> -	 * thus if no device is present, must force it low with an init pinmux
>> -	 * to be able to access the DBI registers.
>> +	 * thus if no device is present, must deassert it with a GPIO from
>> +	 * pinctrl pinmux before accessing the DBI registers.
>>   	 */
>> -	if (!IS_ERR(dev->pins->init_state))
>> -		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
>> -	else
>> -		ret = pinctrl_pm_select_default_state(dev);
>> -
>> +	ret = pinctrl_pm_select_init_state(dev);
>>   	if (ret) {
>>   		dev_err(dev, "Failed to activate pinctrl pm state: %d\n", ret);
>>   		return ret;
>> -- 
>> 2.34.1
>>