Re: [PATCH v12 2/9] PCI: stm32: Add PCIe host support for STM32MP25

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

On 8/8/25 18:45, Bjorn Helgaas wrote:
> On Fri, Aug 08, 2025 at 04:55:52PM +0200, Christian Bruel wrote:
>> On 8/7/25 20:09, Bjorn Helgaas wrote:
>>> [+to Linus for pinctrl usage question below]
>>>
>>> On Tue, Jun 10, 2025 at 11:07:07AM +0200, Christian Bruel wrote:
>>>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>>>> controller based on the DesignWare PCIe core.
> 
>>>> +	return pinctrl_pm_select_sleep_state(dev);
>>>
>>> Isn't there some setup required before we can use
>>> pinctrl_select_state(), pinctrl_pm_select_sleep_state(),
>>> pinctrl_pm_select_default_state(), etc?
>>>
>>> I expected something like devm_pinctrl_get() in the .probe() path, but
>>> I don't see anything.  I don't know how pinctrl works, but I don't see
>>> how dev->pins gets set up.
>>
>> Linus knows better, but the dev->pins states are attached to the dev struct
>> before probe by the pinctrl driver
>>
>> /**
>>   * pinctrl_bind_pins() - called by the device core before probe
>>   * @dev: the device that is just about to probe
>>   */
>> int pinctrl_bind_pins(struct device *dev)
> 
> Thanks for the pointer.  Might be worthy of a mention in
> Documentation/driver-api/pin-control.rst.  Maybe pinctrl/consumer.h
> could even have a bread crumb to that effect since drivers use all
> those interfaces that rely in the implicit initialization done before
> their .probe().
> 
> pin-control.rst mentions pinctrl_get_select_default() being called
> just before the driver probe, but that's now unused and it looks like
> pinctrl_bind_pins() does something similar:
> 
>    really_probe
>      pinctrl_bind_pins
>        dev->pins = devm_kzalloc()
>        devm_pinctrl_get
>        pinctrl_lookup_state(PINCTRL_STATE_DEFAULT)
>        pinctrl_lookup_state(PINCTRL_STATE_INIT)
>        pinctrl_select_state(init)      # if present, else default
>      call_driver_probe

Yeah, and state_init is not mentioned in the documentation even for the 
'normal probe'. The only doc I see from the original commit 
ef0eebc05130b0d22b0ea65c0cd014ee16fc89c7

" 

     Let's introudce a new "init" state.  If this is defined we'll set
     pinctrl to this state before probe and then "default" after probe
     (unless the driver explicitly changed states already). 

"

I will propose something in pin-control.rst, with maybe some code-block 
for the pm part and respin [PATCH 0/2] Add pinctrl_pm_select_init_state 
helper function...

Christian

> 
> Bjorn