Re: failed to power up MHI controller issue

[Manivannan Sadhasivam <mani@kernel.org>]

On Mon, Apr 15, 2024 at 02:09:35PM +0800, Qiang Yu wrote:
> 
> On 4/13/2024 5:10 PM, Slark Xiao wrote:
> > Hi QiangYu, Mani,
> > My case has a bit difference with yours.
> > In my local case, device can't be recovered except a reboot(mainly a cold
> > reboot). Workaround remove and rescan can't solve it.
> > Also, this issue could be occured in some platform in the first bootup.
> > May I know if this issue related with the power sequence of host or
> > pin settings of the connector?
> Hi Slark
> 
> I don't think it's a mhi host driver issue. From the log you shared, there
> is a
> PERST deassertion followed by a PERST assertion closely, leading to device
> run
> out of order.
> 
> // Here, suppose reboot is triggered on host, so mhi_pci_shutdown is invoked
> 91.474100278 [0x0 mhi_sm_pcie_event_manager] Handling
> EP_PCIE_PM_D3_HOT_EVENT
> event, current states: READY and D0_STATE
> 
> 91.747734185 ep_pcie_handle_perst_irq: PCIe V1711211: No. 1 PERST assertion
> 91.747784342 [0x0 mhi_dev_sm_pcie_handler] received:
> EP_PCIE_PM_D3_COLD_EVENT
> 91.748044394 [0x0 mhi_sm_pcie_event_manager] Handling
> EP_PCIE_PM_D3_COLD_EVENT
> event, current states: READY and D3_HOT_STATE
> 
> //Host should power up, so it deassert perst prepare to do link train.
> 92.475207677 ep_pcie_handle_perst_irq: PCIe V1711211: No. 1 PERST
> deassertion
> 92.475269968 ep_pcie_notify_event: PCIe V1711211: Callback client for event
> 8
> 92.475317729 ep_pcie_core_enable_endpoint: PCIe V1711211: options input are
> 0x2
> 92.475321427 ep_pcie_vreg_init: PCIe V1711211
> 92.475323823 ep_pcie_vreg_init: PCIe V1711211: Vreg vreg-1p8 is being
> enabled
> 
> //Process of perst deassert has not completed, but deassert happen, which is
> not expected
> 92.475366010 ep_pcie_handle_perst_irq: PCIe V1711211: No. 2 PERST assertion

Ok. I think I know what is going on. It is related to how PERST# is handled in
the host.

Rockpro64 SoC defines the PERST# GPIO as below in
arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi:

        ep-gpios = <&gpio2 RK_PD4 GPIO_ACTIVE_HIGH>;

Here, the PERST# GPIO is configured as ACTIVE_HIGH. So whatever the value driver
sets as the logical output for the GPIO, it will be reflected as it is on the
physical line.

In the drivers/pci/controller/pcie-rockchip-host.c driver:

	gpiod_set_value_cansleep(rockchip->ep_gpio, 0); # PERST# assert
	gpiod_set_value_cansleep(rockchip->ep_gpio, 1); # PERST# deassert

So when driver asserts the PERST# GPIO, the physical line will output "low"
corresponding to the driver logical value "0". And vice versa for the deassert.

When host reboot happens, the driver is not doing anything specific for PERST#.
So after the SoC reboot, the physical line goes to "low" state corresponding to
PERST# assert (default state of the GPIO) and this is reflected in the endpoint
log as:

	[    91.747734185] ep_pcie_handle_perst_irq: PCIe V1711211: No. 1 PERST assertion

Then, when the host controller probes, the PERST# GPIO is requested as below:

	rockchip->ep_gpio = devm_gpiod_get_optional(dev, "ep",
                                                            GPIOD_OUT_HIGH);

Here, the GPIO is requested with the initial state of GPIOD_OUT_HIGH. Which
means, the driver sets the logical value of the PERST# GPIO to "1" and the
physical value will be "high" and this is reflected in the endpoint log as:

	[    92.475207677] ep_pcie_handle_perst_irq: PCIe V1711211: No. 1 PERST deassertion	

Then during rockchip_pcie_host_init_port() of driver probe, PERST# is asserted
again to perform register initialization and this is also reflected in the
endpoint log as:

	[    92.475366010] ep_pcie_handle_perst_irq: PCIe V1711211: No. 2 PERST assertion

Once the register initilization is completed, then the PERST# is deasserted:

	[    92.503568354] ep_pcie_handle_perst_irq: PCIe V1711211: No. 2 PERST deassertion

The issue here happens due to the very short time between the first PERST#
deassert during devm_gpiod_get_optional() and then successive PERST# assert
during rockchip_pcie_host_init_port() as Qiang noted.

But the GPIO flag (GPIOD_OUT_HIGH) is what actually culprit. It is supposed to
be GPIOD_OUT_LOW as the driver should not deassert PERST# before configuring the
controller.

@Slark: If you can modify the host platform, then try changing the flag from
GPIOD_OUT_HIGH to GPIOD_OUT_LOW in [1] and see if that fixes the issue during
reboot.

- Mani

[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/pci/controller/pcie-rockchip.c?h=v6.6.3#n123

> > At 2024-04-09 12:35:41, "Qiang Yu" <quic_qianyu@quicinc.com> wrote:
> > >
> > >On 4/8/2024 5:59 PM, Slark Xiao wrote:
> > >> Hi Mani,
> > >> Please see attached log files(both kernel log and IPC log).
> > >>
> > >Hi Mani, Slark
> > >
> > >I ever met similar issue like this, where device treated D3cold,ready -> D0
> > >as illegal transition.
> > >
> > >"EP_PCIE_PM_D0_EVENT: illegal in current MHI state: READY and D3_COLD_STATE"
> > >
> > >In MHI spec, there is not transition path from D3cold,ready->D0, but
> > >in fact, we have this transition in some cases.
> > >
> > >For example, when we remove and reinstall pci generic driver. During
> > >remove,
> > >mhi will reset device and pci framework will put it into D3cold
> > directly >when
> > >rootport driver runtime suspend. When we reinstall driver, device will
> > >see D0
> > >event but its current state is D3cold,ready. The whole state transition is
> > >like:
> > >D0,M0 -> D0,reset -> D0,ready-> D3cold,ready -> D0,ready.
> > >
> > >During reboot, if device doesn't reboot with host, look like we will
> > >also meet
> > >similar transition.
> > >
> > >This illegal state error log may not root cause to this issue. Even
> > >process it
> > >as syserr, we can still recovery and go back to M0.
> > >> At 2024-04-02 12:56:47, "Manivannan Sadhasivam" <mani@kernel.org> wrote:
> > >> >+ MHI list
> > >> >
> > >> >On Thu, Mar 28, 2024 at 08:02:20PM +0800, Slark Xiao wrote:
> > >> >> Hi Mani,
> > >> >>  Hope you are doing well! I got a problem with our sdx65 device in some
> > >> >> >> >> specific platform. MHI driver would report "failed to power
> > up MHI controller"
> > >> >> when device bootup. Actually, I can reproduce this error when host doing a
> > >> >> reboot. It's Rockpro64 with SDX65, and kernel 6.6.3 or 6.7.10.
> > >> >> >> >> So I add some logs and change dbg level log to info for more
> > print. You can
> > >> >> see my attachments for reference.
> > >> >> It seems the host didn't receive the event of "MISSION MODE" and then
> > >> >> power down the device.
> > >> >> BTW, there are some extra log prints were added in function
> > >> >> mhi_sync_power_up(). You can find it with mask "##shawn##".
> > >> >> Do you have any idea to debug it?
> > >> >> >> >
> > >> >Looks like something gone wrong with the device firmware. Is it possible to get
> > >> >the logs from the device?
> > >> >
> > >> >This could be due to the way the PCIe controller driver on the host handling
> > >> >reboot.
> > >> >
> > >> >But let's get the device logs first to debug further.
> > >> >
> > >> >- Mani
> > >> >
> > >> >-- >> >மணிவண்ணன் சதாசிவம்
> 

-- 
மணிவண்ணன் சதாசிவம்