-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add regulator-boot-on to meson-sm1-odroid and -hc4 to fix power cycle during boot #9217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… during boot Fix power cycle glitch to SD, USB, HDMI, and especially SATA during boot handoff from U-Boot to kernel (causing HDDs emergency head retract, etc.)
📝 WalkthroughWalkthroughAdds the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Does this affect 6.19 only? 6.18 will stay here for a while too I guess since it is latest LTS kernel. |
rpardini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. The HC4 DT has been stable for years.
I'd say send this to the (upstream, linux-amlogic) mailing list to get feedback from actual experts.
In general I don't think this makes sense, no matter how your heads retract -- the device tree describes the hardware, not the behavior.
patch/kernel/archive/meson64-6.19/board-odroid-sm1-regulators-boot-on.patch
Show resolved
Hide resolved
|
While you do your research upstream, it would be acceptable if you did an overlay or an extra DT. That way you can get your changes in, not affect other boards, and have it be optional. |
With all due respect, the device tree absolutely does define behavior. In this case, for instance,
Again, with respect, I don't agree. This is an issue, and it has either been an issue for years, or upstream behavior of the kernel "regulator-fixed" driver has changed since, and they since added the "regulator-boot-on" property to accommodate. Either way it is an appropriate fix.
Yes, it affects the other meson-sm1-odroid boards. They all have 5V regulators controlled by the GPIOH_8 pin, and it is detrimental to cycle it during boot handoff from U-Boot to kernel. I am certain that it is appropriate for all of these boards, and it is in agreement with and complements the existing "regulator-always-on" properties. If you are really worried about it, I can do a simple overlay, but I am certain that it makes more sense to update the dts files. I doubt anybody wants to deal with it upstream, though it belongs there -- and in any case Armbian is the best thing going for these older boards. Hardkernel has more or less moved on and is barely supporting them with linux updates. Please reconsider. Or else, let me know, and I'll put in a PR with a simple overlay. |
It belongs in 6.18 as well. |
That makes sense. Looking at fixed.c: if (config->enabled_at_boot)
gflags = GPIOD_OUT_HIGH;
else
gflags = GPIOD_OUT_LOW;the parameter controls the initial GPIO state, and if it is already enabled in U-Boot, its state will not change — unlike the case without the boot_on flag (where there would be a quick off-on flap). |
|
@ean365 you might be right -- I simply don't know enough -- but have seen these kinds of changes "pile up" on the Armbian side of things for years now. The best place to get feedback is on the mailing list. If you're correct, upstream will accept your changes, if not, good reasons and a fun discussion will emerge. With that, you can add a proper patch, with a lore link, that will make the whole thing clear to someone who looks at this in 5 years and wonders. It also benefits the whole community and not only Armbian. That said, I've half a dozen HC4's running for a few years, and I've no indication of any retracting heads being a problem. |
|
Its just a property that tells the operating system to automatically enable a specific power regulator when the system starts, even if the bootloader didn't turn it on. Seems like a harmless addition to me. With that said, I don't see why the Odroid C4 would need this. Making it specific to the HC4 would make more sense. With all that said; creating an overlay would achieve the same goal and not be so intrusive. |
|
@ean365 seems feedback from my more knowledgeable colleagues proves me wrong here. It is at worst harmless, and at best fixes an issue others might be having, no matter my own experience. Sorry for the initial reaction. My proposal: make this into an "extra-DT" I did some investigation and you could add these trailers: which, if accepted, would allow stable to backport it all the way back to 5.15. |
|
Thanks @rpardini, @adeepn, @EvilOlaf, and @pyavitz (and the rest) for your support, not only for Armbian, but also for giving me benefit of the doubt and taking a second look at this PR (and #9214). It's very much appreciated. I'll elevate upstream to linux-amlogic and hope it gets some good attention and discussion. In the meantime, I'll try to follow up on your interim suggestions to have it supported (optionally) in Armbian. @rpardini, these HC4s are pretty nice for a cheap but capable NAS. If you are running HDDs (spinners, not SSDs) in yours, it is quite possible that you are actually having similar issues and just haven't noticed -- especially if you typically leave them running 24/7/365 and rarely reboot. Try the following on one of your HC4s that has spinning drives and 6.x kernel to confirm (forgive me if you already know all this, but good for others reading the thread)...
@pyavitz, the boot behavior of the GPIOH_8 regulator control pin that this PR attempts to "fix", controls power to the SD Card, USB port, and HDMI on all sm1-odroid boards (at least), C4 included -- and on the HC4, it also controls power to the SATA ports. People may not be "seeing" issues (other than on an HC4 with sensitive HDDs spinning) when power glitches every shutdown/boot, but those are all boot devices (except maybe the HDMI), and glitching their power can not be good nor intentional, especially during the boot process or truly any time -- not harmless unless Thanks again, all. I'll see what I can do. @rpardini, I'll be curious to know if you (or others) confirm the issue on your HC4s... |
|
Nice, thanks.
Being a helium, low-power drive, it spins down (remember IntelliPark?). In this context
|
|
Hmmm. That's strange. See the wikipedia article on SMART and read the description for |
I don't mean this changes anything about |
Actually, see the description for 1920 xC0 |
Agreed. Though, one way or the other, HDDs were meant to be placed in standby (heads parked) prior to power down. The heads "float" on a razor-thin cushion of air (or helium) while the disk platters are spinning. They must be retracted before the platters slow down, or the heads will potentially "crash" into the platter and damage it all. So, when power is removed suddenly, without warning, there is no more power to nicely retract the heads. Instead, the drive has a capacitor that it keeps charged up for just such an emergency, and it has to very rapidly yank the heads off the platters and up onto the "parking deck" before that capacitor runs out of energy. This is typically a lot more "violent" then a normal head retract/park, which is why it's frowned upon, and should be avoided if possible, if you want maximum life and reliability out of an HDD. |
|
BTW, one other |
Fix power cycle glitch to SD, USB, HDMI, and especially SATA during boot handoff from U-Boot to kernel. This was causing the HDDs to power cycle and do emergency head retract, etc., which is violent and will damage the HDDs over time.
The change to meson-sm1-odroid.dts is appropriate for all meson-sm1-odroid boards 5V regulator.
The changes to meson-sm1-odroid-hc4.dts is specific to the 12V SATA regulators on the Odroid HC4.
They are all tied to GPIOH_8 on the CPU, so all controlled from same pin, and should just never be turned off.
These changes were tested successfully on an Odroid HC4.
Note that this PR in conjunction with #9214 are both needed to save the HDDs from doing emergency head retracts during every boot, reboot, and shutdown. (Previously it was causing two emergency retracts each time.) I kept the PRs separate because technically they stand alone.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.