linux-kernel - Re: [PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in stm32_pcie_resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250813190633.GA284987@bhelgaas>
Date: Wed, 13 Aug 2025 14:06:33 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Christian Bruel <christian.bruel@...s.st.com>
Cc: lpieralisi@...nel.org, kwilczynski@...nel.org, mani@...nel.org,
	robh@...nel.org, bhelgaas@...gle.com, mcoquelin.stm32@...il.com,
	alexandre.torgue@...s.st.com, linus.walleij@...aro.org,
	linux-pci@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-gpio@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v1] PCI: stm32: use pinctrl_pm_select_init_state() in
 stm32_pcie_resume_noirq()

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@...s.st.com>
> Reported-by: Bjorn Helgaas <bhelgaas@...gle.com>
> Reported-by: kernel test robot <lkp@...el.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@...s.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
>