CPUIDLE not working on pinephone

Basically, that’s the wrong place to add the PSCI CPUIDLE stuff to the device tree. Instead, that’s already performed dynamically by TF-A, BL31 to be precise, when the device is booted, but the additions are actually lost.

It’s BL31’s job to actually perform the PSCI operations, so it detects dynamically on boot whether CRUST is there, etc., and adds the CPUIDLE stuff to the device tree if all of the required “ingredients” are detected to be there.

In other words, having that kernel patch in place is, basically, a wrong workaround. I’d suggest that we keep the topic unresolved, as I’m working on a proper fix.

I did mark it unsolved, but i would love to hear your ideas going forward. Seeing similar arguments as you raised were raised in the linux kernel thread, and no solution was found. Were they even ultimately equivalent, or just my impression?

edit: since megi does it like this, and it was proposed to kernel, is it truly an unwanted method? Is there even a kernel interface to do it as you propose, or why is it “lost” as you say? Do you know that your method will be THE proper fix? Are you saying this build that now works should not be pushed to stable?

Thanks for marking the issue as unresolved.

The final result will eventually be the same as with the workaround kernel patch, it’s just about where the required amendments to the device tree are performed. The right place is the BL31 itself, in a dynamic manner, which is already performed as visible below in a TF-A console message, but the amendments are actually lost.

INFO: PSCI: Suspend is available via SCPI

Of course, we can keep the workaround kernel patch in place for now, until there’s a proper fix available through the U-Boot package.

I’m quite close to the proper fix, but I’m unfortunately having some health issues that have already pinned me to the bed for about a week. I’m having some difficulties even while sitting to write this, :slight_smile: but I should get better in about a week or so, according to the doctor. Hopefully.

1 Like

Yes, it’s truly an unwanted way to fix this. For example, if there’s no CRUST available, the kernel will likely crash or error out while attempting to use the feature defined trough the device tree, but actually unavailable at runtime.

The kernel interface is to have the CPUIDLE “stuff” defined though the device tree, from the kernel’s viewpoint, but the actual device tree nodes aren’t supposed to be coming from the static device tree, but are added dynamically in BL31, as part of the boot loader booting up and the device firmware initializing, as I already described above.

I’m sure that the method I’m working on is the right way to fix this issue, simply because the BL31 already adds the CPUIDLE device tree nodes dynamically, by detecting the required runtime support, etc. Why the performed device tree amendments are actually lost is something I still need to determine and fix.

The patched kernel can be pushed to the stable branch as a temporary workaround. It should work just fine, but that’s simply the wrong place for a fix.

Nice to hear you are somewhat on top of this, but sad to hear you still have no idea wtf happens where things just get lost. Maybe megi wrestled with this also, was unable to decode it, and just hard patched it in?

Please, keep in mind that I’ve had very little opportunity to actually work on this in the last week or so, because I had to remain pinned to my bed and I wasn’t able to think straight most of the time, as a result of my current health isssues. Things happen. :slight_smile:

I intend to keep working until there’s a proper fix available. Also, the above-linked kernel patch is actually from Samuel Holland, back from 2021, who, as we know, implemented CRUST and the associated TF-A code. See also commit e2b18771 in the TF-A source code, back from 2022, also coming from Samuel.

I was not trying to criticize you or your work in any way. Maybe saying you have no idea wtf the issue is was too strong of an expression and you took it the wrong way, i did not mean it like that. Just that even if you have no idea what the real issue is at this point, i think this proposed kernel patch/what philm merged is good to push to production, as you stated as temporary fix.

Ah, no worries, everything is perfectly fine. Actually, I was just trying to explain why I haven’t produced any results recently. :slight_smile:

Yes, the proposed kernel patch should be fine as a temporary workaround. Once the fixed U-Boot package is available, we’ll have it fixed the right way and remove the workaround; until then, the workaround is fine for the stable branch.

1 Like

For now the fix only exists in the unstable branch and will get its normal way to stable at some point. The patch is not part of the Megi kernel at all. More or less Mobian is maintaining their own stuff and had added it there.

Yes, Mobian has been using their own set of kernel patches, which were reportedly incomplete and caused quite a few random issues on the PinePhone, including the issues related to the communication with USB chargers.

Though, I don’t know is that still the case, or have the kernel patches applied by Mobian become more complete.

Here’s good news: I think I’ve figured out why the amendments to the device tree performed in BL31 become lost, but I still need to verify all that. Of course, I’ll describe everything in detail once I’ve completed the verification.

The bad news is that having it fixed properly won’t be easy. It may actually require an entire redesign of the way BL31 performs the device tree amendments, which will also be a whole lot of fun to upstream.

Which means the “workaround” fix will stay a little longer :smile:

That’s correct, but we’ve got a functional workaround for now. :slight_smile:

However, the proper fix will actually bring nice improvements to the way device tree information is handled inside and between TF-A and U-Boot. Right now, it overlooks all but a single, most simple way of handling the device trees, which is to use built-in device trees. That isn’t a very good option in general.

Also, it should fix some other longstanding issues, such as passing the serial console parameters around correctly. Right now, that seemingly isn’t performed correctly for Rockchip SoCs, which usually causes TF-A messages to be lost on the serial console. That obviously isn’t good on mutiple levels.

Edit: On second thought, it actually shouldn’t solve the issue with the serial console parameters. However, solving that issue has been on my TO-DO list for a while, and I intend to get that resolved as well at some point in time.

Here’s just a brief update and some further insights…

My intended implementation of the proper fix, fingers crossed, :slight_smile: should be fine when it comes to having it upstreamed. Moreover, there are already additional “consumers” of the current “amend the device tree in BL31” concept that should benefit from the improvements brought in by the proper fix, which should help with the upstreaming.

The possible obstacles, besides the involved complexity of the implementation, are the need to have the patches upstreamed to both TF-A and U-Boot, and the well-known tediousness when it comes to TF-A accepting patches of pretty much any kind.

Speaking of the above-mentioned already existing “consumers”, an example is the way that BL31 for the sunxi and rpi4 platforms passes the reserved areas of memory through the device tree amendments, i.e. through the reserved-memory node defined dynamically within the device tree.

Those amendments to the device tree, performed in BL31, are currently lost, at least on the sunxi boards and devices supported by Manjaro ARM. Having the amendments not present within the device tree data as seen by the Linux kernel is obviously very wrong. It also makes me wonder could that be the reason for mysterious system crashes I’ve observed occasionally on the PinePhone.

i have almost never seen a system crash for more than a year now, but the UI has crashed maybe once per 3 months and kicked me back to the login screen.

You’re totally right about the random GUI crashes being much, much more often on the PinePhone. At least in my experience.

I think it would be a good time to make a pp image release with this fix applied, if not for any other reason than to advertise the power management improvement? At least i have not seen any regressions…

@dsimic have you made any progress in the proper implementation?

There should be no regressions with the workaround applied to the kernel package, unless someone uses a U-Boot build that, for some reason, actually has no CRUST support built into it. Obviously, such cases should be quite rare, if any.

I’ve made some progress with the proper implementation, but unfortunately my recent health issues reared their ugly head again, so I’ve remained in my bed pretty much all the time in the last three weeks or so. On the bright side, I’m taking drugs again, as prescribed this time, and I’m feeling much better now, thankfully, so I should be back to doing some work quite soon. I got really tired of doing almost nothing. :slight_smile:

Sad to hear about your health issues, i really hope you get well soon!

1 Like

Thank you very much! I hope I’ll end up with no more prescribed drugs after the current prescription is completed in about two weeks, at which point another medical test will be performed to assess the end result of the current prescription.