On 17/04/2025 at 16:46:13 +02, Heiko Schocher <h...@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.ray...@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.ray...@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
