Re: imx8mp: pci enumeration fails with current HEAD

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 17/04/2025 at 16:46:13 +02, Heiko Schocher <hs@denx.de> wrote:

> Hello Miquel,
>
> On 17.04.25 16:37, Heiko Schocher wrote:
>> Hi Miquel,
>> I have here an imx8mp based board for which I just prepare mainlining.
>> pci enumeration works fine with U-Boot 2025.04
>> u-boot=> pci enum
>> PCIE-0: Link up (Gen1-x1, Bus0)
>> u-boot=>
>> and tftp through the ethernet interface works fine.
>> Using current HEAD
>> * 5b4ae0f3f04 - (origin/master, origin/HEAD) mailmap: update my name
>> and email (vor 2 Tagen) <Casey Connolly>
>> It breaks with:
>> u-boot=> pci enum
>> nxp_imx8_pcie_phy pcie-phy@32f00000: PHY: Failed to power on pcie-phy@32f00000: -110.
>> pcie_dw_imx pcie@33800000: failed to power on PHY
>> u-boot=>
>> git bisect dropped your patch:
>> 197376fbf300e92afa0a1583815d9c9eb52d613a is the first bad commit
>> commit 197376fbf300e92afa0a1583815d9c9eb52d613a
>> Author: Miquel Raynal <miquel.raynal@bootlin.com>
>> Date:   Thu Apr 3 09:39:05 2025 +0200
>>      power-domain: Add refcounting
>>      It is very surprising that such an uclass, specifically designed
>> to
>>      handle resources that may be shared by different devices, is not keeping
>>      the count of the number of times a power domain has been
>>      enabled/disabled to avoid shutting it down unexpectedly or disabling it
>>      several times.
>>      Doing this causes troubles on eg. i.MX8MP because disabling power
>>      domains can be done in recursive loops were the same power domain
>>      disabled up to 4 times in a row. PGCs seem to have tight FSM internal
>>      timings to respect and it is easy to produce a race condition that puts
>>      the power domains in an unstable state, leading to ADB400 errors and
>>      later crashes in Linux.
>>      CI tests using power domains are slightly updated to make sure
>> the count
>>      of on/off calls is even and the results match what we *now* expect.
>>      As we do not want to break existing users while stile getting
>>      interesting error codes, the implementation is split between:
>>      - a low-level helper reporting error codes if the requested transition
>>        could not be operated,
>>      - a higher-level helper ignoring the "non error" codes, like EALREADY and
>>        EBUSY.
>>      Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> reverting this patch, and it works again fine for me!
>> Do you have a imx8mp based hardware with pci?
>> Can you (or anyone else with an imx8mp based hardware with pci)
>> approve
>> this on a similiar HW?
>> I try to dig into it, but may you have a fast idea!
>> Okay, thought about it ... it tries to power on "blk-ctrl@32f10000"
>> driver:/drivers/power/domain/imx8mp-hsiomix.c
>> imx8mp.dtsi:
>>                          hsio_blk_ctrl: blk-ctrl@32f10000 {
>>                                  compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
>> which has several different power domains ... and your patch prevents
>> to
>> enable more than one of them ... adding some debugs shows:
>> u-boot=> pci enum
>> [...]
>> power_domain_on_lowlevel(power_domain=00000000fe5942f8) blk-ctrl@32f10000 priv->on_count: 0
>> power_domain_on_lowlevel(power_domain=00000000fe5942f8) blk-ctrl@32f10000 power_domain->id: 4
>> Here pwoer domain id
>> #define IMX8MP_HSIOBLK_PD_PCIE_PHY                      4
>> gets enabled and on_count increases...
>> imx8mp_hsiomix_set: ------------ power_domain->id: 4
>> IMX8MP_HSIOBLK_PD_PCIE: 3 4
>> power_domain_on_lowlevel(power_domain=00000000fe5d2408) power-domain@17 priv->on_count: 0
>> power_domain_on_lowlevel(power_domain=00000000fe5d2408) power-domain@17 power_domain->id: 0
>> power_domain_on_lowlevel: ret: 0
>> power_domain_on_lowlevel(power_domain=00000000fe5d2480) power-domain@1 priv->on_count: 0
>> power_domain_on_lowlevel(power_domain=00000000fe5d2480) power-domain@1 power_domain->id: 0
>> power_domain_on_lowlevel: ret: 0
>> imx8mp_hsiomix_set: ------------ power_domain->id: 4 IMX8MP_HSIOBLK_PD_PCIE: 3 4 END OKAY
>> power_domain_on_lowlevel: ret: 0
>> power_domain_get_by_index(dev=00000000fe5b2410, power_domain=00000000fe594458)
>> power_domain_on_lowlevel(power_domain=00000000fe594458) blk-ctrl@32f10000 priv->on_count: 1
>> power_domain_on_lowlevel(power_domain=00000000fe594458) blk-ctrl@32f10000 power_domain->id: 3
>
> okay, the "power_domain=00000000fe594458" for the IMX8MP_HSIOBLK_PD_PCIE case
> is different as the one for the IMX8MP_HSIOBLK_PD_PCIE_PHY case ... so priv pointer
> should also be different...

That's part of the problem. The API is unusual, you probably allocate a
structure (either on the stack or dynamically) and pass its pointer to
be filled by the uclass.

In general the approach is that you ask the uclass to give you a pointer
towards a unique version of a structure that defines an opaque object
you want to manipulate.

So I do not think the printing is relevant, what is more relevant is the
node the udev targets. In both cases it is blk-ctrl@32f10000, so the
second time on_count is already set.

> so that should be fine ... I do not see any debug output
> where this "power_domain=00000000fe594458" gets enabled (or disabled before) ... I
> have to debug deeper into it... sorry. But may you have an idea...

Let me know if you think my theory does not stand.

Thanks,
Miquèl