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

[Christian Bruel <christian.bruel@foss.st.com>]

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
>>