[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c8076e56-4b5a-466c-8702-0a7a596f987e@foss.st.com>
Date: Mon, 18 Aug 2025 14:18:44 +0200
From: Christian Bruel <christian.bruel@...s.st.com>
To: Bjorn Helgaas <helgaas@...nel.org>
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 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@...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.
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
>>
Powered by blists - more mailing lists