[RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[Christian Bruel]

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

[RESEND PATCH 1/2] pinctrl: Add pinctrl_pm_select_init_state helper function

[Christian Bruel]

If a platform requires an initial state during probing, this helper function
provides the client with access to the same initial state used to balance
from a pinctrl_pm_select_sleep_state .
eg:

 xxx_suspend_noirq
    pinctrl_pm_select_sleep_state

 xxx resume_noirq
    pinctrl_pm_select_init_state
    ...
    pinctrl_pm_select_default_state

Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
---
 drivers/pinctrl/core.c           | 13 +++++++++++++
 include/linux/pinctrl/consumer.h | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 9046292d1360..80cf9f20f626 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1655,6 +1655,19 @@ int pinctrl_pm_select_default_state(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(pinctrl_pm_select_default_state);
 
+/**
+ * pinctrl_pm_select_init_state() - select init pinctrl state for PM
+ * @dev: device to select init state for
+ */
+int pinctrl_pm_select_init_state(struct device *dev)
+{
+	if (!dev->pins)
+		return 0;
+
+	return pinctrl_select_bound_state(dev, dev->pins->init_state);
+}
+EXPORT_SYMBOL_GPL(pinctrl_pm_select_init_state);
+
 /**
  * pinctrl_pm_select_sleep_state() - select sleep pinctrl state for PM
  * @dev: device to select sleep state for
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 73de70362b98..63ce16191eb9 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -48,6 +48,7 @@ int pinctrl_select_default_state(struct device *dev);
 
 #ifdef CONFIG_PM
 int pinctrl_pm_select_default_state(struct device *dev);
+int pinctrl_pm_select_init_state(struct device *dev);
 int pinctrl_pm_select_sleep_state(struct device *dev);
 int pinctrl_pm_select_idle_state(struct device *dev);
 #else
@@ -55,6 +56,10 @@ static inline int pinctrl_pm_select_default_state(struct device *dev)
 {
 	return 0;
 }
+static inline int pinctrl_pm_select_init_state(struct device *dev)
+{
+	return 0;
+}
 static inline int pinctrl_pm_select_sleep_state(struct device *dev)
 {
 	return 0;
@@ -143,6 +148,11 @@ static inline int pinctrl_pm_select_default_state(struct device *dev)
 	return 0;
 }
 
+static inline int pinctrl_pm_select_init_state(struct device *dev)
+{
+	return 0;
+}
+
 static inline int pinctrl_pm_select_sleep_state(struct device *dev)
 {
 	return 0;
-- 
2.34.1

[RESEND PATCH 2/2] 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.

Signed-off-by: Christian Bruel <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")
---
 drivers/pci/controller/dwc/pcie-stm32.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
index 50fae5f5ced2..c1d803dc3778 100644
--- a/drivers/pci/controller/dwc/pcie-stm32.c
+++ b/drivers/pci/controller/dwc/pcie-stm32.c
@@ -28,6 +28,7 @@ struct stm32_pcie {
 	struct clk *clk;
 	struct gpio_desc *perst_gpio;
 	struct gpio_desc *wake_gpio;
+	bool   have_pinctrl_init;
 };
 
 static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
@@ -91,10 +92,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.
+	 * if present to be able to access the DBI registers.
 	 */
-	if (!IS_ERR(dev->pins->init_state))
-		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
+	if (stm32_pcie->have_pinctrl_init)
+		ret = pinctrl_pm_select_init_state(dev);
 	else
 		ret = pinctrl_pm_select_default_state(dev);
 
@@ -274,6 +275,9 @@ static int stm32_pcie_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
 				     "Failed to get PCIe reset\n");
 
+	if (device_property_match_string(dev, "pinctrl-names", PINCTRL_STATE_INIT) >= 0)
+		stm32_pcie->have_pinctrl_init = true;
+
 	ret = stm32_pcie_parse_port(stm32_pcie);
 	if (ret)
 		return ret;
-- 
2.34.1

Re: [RESEND PATCH 2/2] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume_noirq()

[Bjorn Helgaas]

On Thu, Jul 17, 2025 at 08:30:42AM +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.
> 
> Signed-off-by: Christian Bruel <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")

633f42f48af5 is still on pci/controller/dwc-stm32, but only for
reference.  After v6.17-rc1, we will need to rebase to it and figure
out the merge strategy.

Part of that will be to restructure 633f42f48af5 and the material
below such that there is no build issue at any point in the series.

> ---
>  drivers/pci/controller/dwc/pcie-stm32.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
> index 50fae5f5ced2..c1d803dc3778 100644
> --- a/drivers/pci/controller/dwc/pcie-stm32.c
> +++ b/drivers/pci/controller/dwc/pcie-stm32.c
> @@ -28,6 +28,7 @@ struct stm32_pcie {
>  	struct clk *clk;
>  	struct gpio_desc *perst_gpio;
>  	struct gpio_desc *wake_gpio;
> +	bool   have_pinctrl_init;
>  };
>  
>  static void stm32_pcie_deassert_perst(struct stm32_pcie *stm32_pcie)
> @@ -91,10 +92,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.
> +	 * if present to be able to access the DBI registers.
>  	 */
> -	if (!IS_ERR(dev->pins->init_state))
> -		ret = pinctrl_select_state(dev->pins->p, dev->pins->init_state);
> +	if (stm32_pcie->have_pinctrl_init)
> +		ret = pinctrl_pm_select_init_state(dev);
>  	else
>  		ret = pinctrl_pm_select_default_state(dev);
>  
> @@ -274,6 +275,9 @@ static int stm32_pcie_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(stm32_pcie->rst),
>  				     "Failed to get PCIe reset\n");
>  
> +	if (device_property_match_string(dev, "pinctrl-names", PINCTRL_STATE_INIT) >= 0)
> +		stm32_pcie->have_pinctrl_init = true;
> +
>  	ret = stm32_pcie_parse_port(stm32_pcie);
>  	if (ret)
>  		return ret;
> -- 
> 2.34.1
> 

Re: [RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[Linus Walleij]

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

Re: [RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[Bjorn Helgaas]

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

Re: [RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[Christian Bruel]

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

Re: [RESEND PATCH 0/2] Add pinctrl_pm_select_init_state helper function

[Christian Bruel]

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
>