We have the helper functions pinctrl_pm_select_default_state and
pinctrl_pm_select_sleep_state.
This patch adds the missing pinctrl_pm_select_init_state function.

The STM32MP2 needs to set the pinctrl to an initial state during
pm_resume, just like in probe. To achieve this, the function
pinctrl_pm_select_init_state is added.

This allows a driver to balance pinctrl_pm_select_sleep_state()
with pinctrl_pm_select_default_state() and
pinctrl_pm_select_init_state() in pm_runtime_suspend and pm_runtime_resume.

Christian Bruel (2):
  pinctrl: Add pinctrl_pm_select_init_state helper function
  PCI: stm32: use pinctrl_pm_select_init_state() in
    stm32_pcie_resume_noirq()

 drivers/pci/controller/dwc/pcie-stm32.c | 10 +++++++---
 drivers/pinctrl/core.c                  | 13 +++++++++++++
 include/linux/pinctrl/consumer.h        | 10 ++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)


base-commit: 5a972a01e24b278f7302a834c6eaee5bdac12843
-- 
2.34.1

On Thu, Jul 17, 2025 at 8:33 AM Christian Bruel
<christian.bruel@foss.st.com> wrote:

> We have the helper functions pinctrl_pm_select_default_state and
> pinctrl_pm_select_sleep_state.
> This patch adds the missing pinctrl_pm_select_init_state function.
>
> The STM32MP2 needs to set the pinctrl to an initial state during
> pm_resume, just like in probe. To achieve this, the function
> pinctrl_pm_select_init_state is added.
>
> This allows a driver to balance pinctrl_pm_select_sleep_state()
> with pinctrl_pm_select_default_state() and
> pinctrl_pm_select_init_state() in pm_runtime_suspend and pm_runtime_resume.
>
> Christian Bruel (2):
>   pinctrl: Add pinctrl_pm_select_init_state helper function
>   PCI: stm32: use pinctrl_pm_select_init_state() in
>     stm32_pcie_resume_noirq()

If Bjorn Helgaas is OK with it I can apply this to the pinctrl tree.

Otherwise I can also just apply patch 1/2, but that doesn't solve
any problem.

What should I do?

Yours,
Linus Walleij

On Wed, Jul 23, 2025 at 01:32:52PM +0200, Linus Walleij wrote:
> On Thu, Jul 17, 2025 at 8:33 AM Christian Bruel
> <christian.bruel@foss.st.com> wrote:
> 
> > We have the helper functions pinctrl_pm_select_default_state and
> > pinctrl_pm_select_sleep_state.
> > This patch adds the missing pinctrl_pm_select_init_state function.
> >
> > The STM32MP2 needs to set the pinctrl to an initial state during
> > pm_resume, just like in probe. To achieve this, the function
> > pinctrl_pm_select_init_state is added.
> >
> > This allows a driver to balance pinctrl_pm_select_sleep_state()
> > with pinctrl_pm_select_default_state() and
> > pinctrl_pm_select_init_state() in pm_runtime_suspend and pm_runtime_resume.
> >
> > Christian Bruel (2):
> >   pinctrl: Add pinctrl_pm_select_init_state helper function
> >   PCI: stm32: use pinctrl_pm_select_init_state() in
> >     stm32_pcie_resume_noirq()
> 
> If Bjorn Helgaas is OK with it I can apply this to the pinctrl tree.
> 
> Otherwise I can also just apply patch 1/2, but that doesn't solve
> any problem.

The stm32 driver has been posted and is on this branch of the PCI
tree:

  https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dwc-stm32&id=5a972a01e24b

but it's not in mainline (or even in pci/next) yet, so you would only
be able to apply patch 2/2 if you took the whole driver, which is
probably more than you would want to do.

I haven't put it in pci/next yet because it doesn't build when
CONFIG_PINCTRL is not defined:

  https://lore.kernel.org/r/20250716192418.GA2550861@bhelgaas

I don't know enough about pinctrl to know why stm32 needs this when
nobody else seems to.  I doubt it's really unique, so maybe it's just
not doing the right thing here.

Bjorn

On 7/23/25 23:07, Bjorn Helgaas wrote:
> On Wed, Jul 23, 2025 at 01:32:52PM +0200, Linus Walleij wrote:
>> On Thu, Jul 17, 2025 at 8:33 AM Christian Bruel
>> <christian.bruel@foss.st.com> wrote:
>>
>>> We have the helper functions pinctrl_pm_select_default_state and
>>> pinctrl_pm_select_sleep_state.
>>> This patch adds the missing pinctrl_pm_select_init_state function.
>>>
>>> The STM32MP2 needs to set the pinctrl to an initial state during
>>> pm_resume, just like in probe. To achieve this, the function
>>> pinctrl_pm_select_init_state is added.
>>>
>>> This allows a driver to balance pinctrl_pm_select_sleep_state()
>>> with pinctrl_pm_select_default_state() and
>>> pinctrl_pm_select_init_state() in pm_runtime_suspend and pm_runtime_resume.
>>>
>>> Christian Bruel (2):
>>>    pinctrl: Add pinctrl_pm_select_init_state helper function
>>>    PCI: stm32: use pinctrl_pm_select_init_state() in
>>>      stm32_pcie_resume_noirq()
>>
>> If Bjorn Helgaas is OK with it I can apply this to the pinctrl tree.
>>
>> Otherwise I can also just apply patch 1/2, but that doesn't solve
>> any problem.
> 
> The stm32 driver has been posted and is on this branch of the PCI
> tree:
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=controller/dwc-stm32&id=5a972a01e24b
> 
> but it's not in mainline (or even in pci/next) yet, so you would only
> be able to apply patch 2/2 if you took the whole driver, which is
> probably more than you would want to do.
> 
> I haven't put it in pci/next yet because it doesn't build when
> CONFIG_PINCTRL is not defined:
> 
>    https://lore.kernel.org/r/20250716192418.GA2550861@bhelgaas
> 
> I don't know enough about pinctrl to know why stm32 needs this when
> nobody else seems to.  I doubt it's really unique, so maybe it's just
> not doing the right thing here.

The STM32MP2 is unique because the core clock is gated on CLKREQ#. 
Consequently, it is not possible to access the core registers from DBI 
when no card is attached, causing the board to freeze. I don't know 
another platform with this limitation

To fix this, we use a GPIO to de-assert CLKREQ# during probe and restore 
the pin to its default AF mode afterward. This works perfectly for 
probe, but we lack functionality for PM resume unless we explicitly 
select the state with pinctrl_pm_select_XXX_state().

For reference, the init_state functionality was introduced in
https://lkml.org/lkml/2015/10/21/1

If we prefer not to extend the pinctrl API in patch 1/2, I can fix the 
case in patch 2/2 only with something like:

in stm32_pcie_probe()
      pinctrl = devm_pinctrl_get(dev);

      if(pinctrl!= -ENODEV) // PINCTRL is defined
           pinctrl_init = pinctrl_lookup_state(stm32_pcie>pinctrl, 
PINCTRL_STATE_IN

in stm32_pcie_resume_noirq()
    if (pinctrl) {
           ret = pinctrl_select_state(stm32_pcie->pinctrl, 
stm32_pcie->pinctrl_init);

What do you advise ?

thank you

Christian



> 
> Bjorn

Gentle ping,

Maybe the best for Linus is to apply only 1/2 on his pinctrl tree.

I'll rebase 2/2 stm32 PCIe part after the merge to mainline and respin 
for the PCI tree

Best Regards

Christian

On 7/24/25 15:36, Christian Bruel wrote:
> 
> 
> On 7/23/25 23:07, Bjorn Helgaas wrote:
>> On Wed, Jul 23, 2025 at 01:32:52PM +0200, Linus Walleij wrote:
>>> On Thu, Jul 17, 2025 at 8:33 AM Christian Bruel
>>> <christian.bruel@foss.st.com> wrote:
>>>
>>>> We have the helper functions pinctrl_pm_select_default_state and
>>>> pinctrl_pm_select_sleep_state.
>>>> This patch adds the missing pinctrl_pm_select_init_state function.
>>>>
>>>> The STM32MP2 needs to set the pinctrl to an initial state during
>>>> pm_resume, just like in probe. To achieve this, the function
>>>> pinctrl_pm_select_init_state is added.
>>>>
>>>> This allows a driver to balance pinctrl_pm_select_sleep_state()
>>>> with pinctrl_pm_select_default_state() and
>>>> pinctrl_pm_select_init_state() in pm_runtime_suspend and 
>>>> pm_runtime_resume.
>>>>
>>>> Christian Bruel (2):
>>>>    pinctrl: Add pinctrl_pm_select_init_state helper function
>>>>    PCI: stm32: use pinctrl_pm_select_init_state() in
>>>>      stm32_pcie_resume_noirq()
>>>
>>> If Bjorn Helgaas is OK with it I can apply this to the pinctrl tree.
>>>
>>> Otherwise I can also just apply patch 1/2, but that doesn't solve
>>> any problem.
>>
>> The stm32 driver has been posted and is on this branch of the PCI
>> tree:
>>
>>    https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/? 
>> h=controller/dwc-stm32&id=5a972a01e24b
>>
>> but it's not in mainline (or even in pci/next) yet, so you would only
>> be able to apply patch 2/2 if you took the whole driver, which is
>> probably more than you would want to do.
>>
>> I haven't put it in pci/next yet because it doesn't build when
>> CONFIG_PINCTRL is not defined:
>>
>>    https://lore.kernel.org/r/20250716192418.GA2550861@bhelgaas
>>
>> I don't know enough about pinctrl to know why stm32 needs this when
>> nobody else seems to.  I doubt it's really unique, so maybe it's just
>> not doing the right thing here.
> 
> The STM32MP2 is unique because the core clock is gated on CLKREQ#. 
> Consequently, it is not possible to access the core registers from DBI 
> when no card is attached, causing the board to freeze. I don't know 
> another platform with this limitation
> 
> To fix this, we use a GPIO to de-assert CLKREQ# during probe and restore 
> the pin to its default AF mode afterward. This works perfectly for 
> probe, but we lack functionality for PM resume unless we explicitly 
> select the state with pinctrl_pm_select_XXX_state().
> 
> For reference, the init_state functionality was introduced in
> https://lkml.org/lkml/2015/10/21/1
> 
> If we prefer not to extend the pinctrl API in patch 1/2, I can fix the 
> case in patch 2/2 only with something like:
> 
> in stm32_pcie_probe()
>       pinctrl = devm_pinctrl_get(dev);
> 
>       if(pinctrl!= -ENODEV) // PINCTRL is defined
>            pinctrl_init = pinctrl_lookup_state(stm32_pcie>pinctrl, 
> PINCTRL_STATE_IN
> 
> in stm32_pcie_resume_noirq()
>     if (pinctrl) {
>            ret = pinctrl_select_state(stm32_pcie->pinctrl, stm32_pcie- 
>  >pinctrl_init);
> 
> What do you advise ?
> 
> thank you
> 
> Christian
> 
> 
> 
>>
>> Bjorn
>