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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux