Skip to content
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

Conditional booting based POWER Led level #1053

Closed
akarapatis opened this issue Oct 3, 2018 · 28 comments
Closed

Conditional booting based POWER Led level #1053

akarapatis opened this issue Oct 3, 2018 · 28 comments

Comments

@akarapatis
Copy link

For Rpi-3B, is it possible to define conditional booting based on POWER Led level ?

I know that the POWER Led is connected to GPIO port expander on i2c-0 and that you can read it from /sys/devices/platform/leds/leds/led1.

But is there a way to use it in config.txt ?
If not, what would you suggest? ( E.g. modify bootloader)

References

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

I've not tried it, but there may be a way using a config.txt conditional filter based on a GPIO value. The PWR level appears on GPIO 7 on the GPIO expander, which appears as 128+7=GPIO135.

Try adding this (untested) section to config.txt:

[gpio135=0]
dtoverlay=PWR-is-low
[gpio135=1]
dtoverlay=PWR-is-high

Reboot, then run sudo vcdbg log msg |& grep PWD. The dtoverlay lines loading non-existent overlays are just a way to add some logging entries.

Assuming this works, whether it can be used to achieve what you want depends on what your goal is.

@akarapatis
Copy link
Author

I was aware of the conditional filter for GPIO and had already tried something like:

[gpio135=0]
boot_delay=240

[gpio135=1]
boot_delay=1

However, that didn't work (device boots regardless of pin value) and was wondering if there was something I was missing.

P.S
Thx for explaining the GPIO135 as I had seen it and didn't how it was formed

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

Did you try my suggestion to test whether the filter is working?

@akarapatis
Copy link
Author

Running:
sudo vcdbg log msg |& grep PW

Power LED ON during boot

001058.813: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
001254.112: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
002013.051: Failed to load overlay 'PWR-is-low'
002047.163: Failed to load overlay 'PWR-is-high'
002047.222: brfs: File read: /mfs/sd/overlays/PWR-is-high.dtb

Power LED OFF during boot

003612.700: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
003806.099: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
004549.229: Failed to load overlay 'PWR-is-low'
004583.522: Failed to load overlay 'PWR-is-high'
004583.581: brfs: File read: /mfs/sd/overlays/PWR-is-high.dtb

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

OK, thanks - it looks thoroughly broken.

@akarapatis
Copy link
Author

You mean it needs fixing or it is not possible?

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

The gpio filter could conceivably be fixed (until we find the problem I can't promise anything), but I don't see how a fixed-length delay is going to do anything useful for you unless your configuration guarantees to have power eventually.

@akarapatis
Copy link
Author

A delay is necessary in our case as we are building up power gradually and when devices are connected on the USB hub it fails to reach the necessary 5V. So we added a static delay to let our system build up its necessary power

@6by9
Copy link

6by9 commented Oct 3, 2018

I had 2 minutes with the debugger up.
The relevant board traits aren't set at the point that the config.txt tries parsing the [gpioX=Y] tags, so it doesn't know how to get to the GPIO expander.

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

I was expecting that to be the case for the first pass, but the dtoverlay parsing is done in a second, later pass using (I thought) the same rules.

@6by9
Copy link

6by9 commented Oct 3, 2018

OK, I'll check again tomorrow.

@6by9
Copy link

6by9 commented Oct 3, 2018

Actually I had seen this whilst debugging. The gpio expander open error path via gpioman is flawed and leaves it thinking the expander is open & initialised when it isn't. Fixing that may mean that it initialises correctly on that second pass.

@pelwell
Copy link
Contributor

pelwell commented Oct 3, 2018

That's definitely the problem area, but the error path is never used because the incomplete configuration selects the dummy driver which always succeeds. It needs either a way to reset the erroneous latched state or a way of deferring the choice of driver until the configuration is complete.

@6by9
Copy link

6by9 commented Oct 4, 2018

I'm getting the correct driver selected, but it then checks SMPS traits and doesn't fail if neither are selected, just leaving things in a bad state.
Fixed that up and we get the correct behaviour - I'll create a merge request.

It is worth noting that the SoC is pulling very little power at the point that config.txt is parsed, so I had to drop the voltage significantly further than expected.

popcornmix added a commit that referenced this issue Oct 9, 2018
2ndstage: Increase eth_open timeout to 5 seconds
See: #1041

firmware: video_encode: Use default values on invalid nStride or nSliceHeight
See: #1051

firmware: gpioman/FXL6408: Handle open failing sensibly
See: #1053

firmware: Delay backlight coming on
See: #1052

firmware: LCD driver close fixes

2ndstage: ignore autoboot.txt if boot partition is already set
See: raspberrypi/noobs#508
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this issue Oct 9, 2018
2ndstage: Increase eth_open timeout to 5 seconds
See: raspberrypi/firmware#1041

firmware: video_encode: Use default values on invalid nStride or nSliceHeight
See: raspberrypi/firmware#1051

firmware: gpioman/FXL6408: Handle open failing sensibly
See: raspberrypi/firmware#1053

firmware: Delay backlight coming on
See: raspberrypi/firmware#1052

firmware: LCD driver close fixes

2ndstage: ignore autoboot.txt if boot partition is already set
See: raspberrypi/noobs#508
@popcornmix
Copy link
Contributor

Fix should be in latest rpi-update firmware.

@6by9
Copy link

6by9 commented Oct 20, 2018

@akarapatis Please test and report back.

@6by9
Copy link

6by9 commented Oct 30, 2018

Just to confirm, are you on a 3B+, or a 3B? It makes a big difference.
And what exactly is in your config.txt? I don't see why you'd get both lines logged with LED on.

@akarapatis
Copy link
Author

uname -r returns 4.14.78-v7+

Running:
sudo vcdbg log msg |& grep PW

Power LED OFF during boot

001025.692: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
001221.768: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
002000.439: Failed to load overlay 'PWR-is-high'
002000.495: brfs: File read: /mfs/sd/overlays/PWR-is-high.dtb

Power LED ON during boot

001070.591: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
001268.543: gpioman: gpioman_get_pin_num: pin LEDS_PWR_OK not defined
002031.376: Failed to load overlay 'PWR-is-low'
002031.708: brfs: File read: /mfs/sd/overlays/PWR-is-low.dtb

@akarapatis
Copy link
Author

@6by9 I checked again the 2 scenarios. Also I am running it on 3B

@pelwell
Copy link
Contributor

pelwell commented Oct 30, 2018

[ I think you would get rid of the overlay errors if you named the overlays "*.dtbo", like all the rest. ]

@akarapatis
Copy link
Author

akarapatis commented Oct 30, 2018

In addition, my config.txt contains

[gpio135=0]
dtoverlay=PWR-is-low

[gpio135=1]
dtoverlay=PWR-is-high

It was the config.txt suggested to see whether the POWER LED pin is read or not

@6by9
Copy link

6by9 commented Oct 30, 2018

That all looks fine.
You get dtoverlay=PWR-is-high run (and failing as the file doesn't exist) in one condition, and dtoverlay=PWR-is-low run (and failing) in the other condition.
(It's not what your initial post had said which is why I was confused).

It does appear that the logic is inverted, so [gpio135=0] is when there is no low voltage condition, but apart from that it looks correct.

@akarapatis
Copy link
Author

akarapatis commented Oct 30, 2018

@6by9 Forgive me about the original post. I thought I had tested it properly. Indeed it seems inverted.

However, one more question:
When I add boot_delay=120 rule in the case where power led is OFF, it seems to be taken into account even when power led is ON

so my config.txt now looks

[gpio135=0]
dtoverlay=PWR-is-low

[gpio135=1]
boot_delay=120
dtoverlay=PWR-is-high

@6by9
Copy link

6by9 commented Oct 30, 2018

However, one question when I add boot_delay=120 rule in the case where power led is OFF it is ignored

It looks like it is just the order that things are parsed. As I think Phil had said, things are done in phases, and I think the dtoverlay phase that we were using as a canary is done later than the boot_delay (which is pretty early on).
IIRC you can insert things onto the Linux command line via dtoverlays, so if you are only needing to slow Linux booting then that may be one answer.

	fragment@0 {
		target-path = "/chosen";
		__overlay__ {
			bootargs = "cma=256M";
		};
	};

is part of vc4-kms-v3d-overlay.dts to select an increased CMA heap.
I suspect bootargs = "bootdelay=5"; would add a 5 second delay. Amend as required.

@akarapatis
Copy link
Author

@6by9 Thank you for help.

Indeed it seemed that boot_delay was evaluated before the conditional booting.

However, if we edit the overlay this might be overwritten by an update and if we create a new one we need to keep updating it ourselves.

In any case, it is very helpful

@6by9
Copy link

6by9 commented Nov 1, 2018

However, if we edit the overlay this might be overwritten by an update and if we create a new one we need to keep updating it ourselves.

I was suggesting creating your own overlay, eg boot-delay-overlay.dts. Whilst there is a chance that someone else will create an overlay with the same name which could overwrite yours, the likelihood is very low. None of the update mechanisms should ever delete overlays they don't know about.

@pelwell
Copy link
Contributor

pelwell commented Nov 1, 2018

You can always submit your overlay to be added to the standard kernel. I would suggest making it a generic kernel-param overlay:

dtoverlay=kernel-param=bootdelay=5

This would be more generally useful, and would get around the need for any string manipulation (which overlays can't do).

@akarapatis
Copy link
Author

I guess this issue can close. Thank you for fixing the bug and helping me finding a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants