Input: adp5588-keys - cache GPIO state before registering the gpiochip - #3500
Open
sipraga wants to merge 1 commit into
Open
Input: adp5588-keys - cache GPIO state before registering the gpiochip#3500sipraga wants to merge 1 commit into
sipraga wants to merge 1 commit into
Conversation
So as not to clobber any pre-programmed GPIO state in the execution of its gpiochip ops, the driver caches things during probe time. However, since those ops can be called both during and immediately after the call to devm_gpiochip_add_data(), it is imperative that things are cached before that. That's not the case right now, so reorder the two steps to prevent any clobbering. In a conrete example which motivated this change, a bootloader was preconfiguring an important GPIO output to HIGH before booting the kernel. Linux would then inadvertently set that output to LOW while configuring a GPIO hog on a discrete GPIO line within the same 8-bit bank (because the cached value was 0=LOW). Fixes: ba9f507 ("Input: adp5588-keys - export unused GPIO pins") Signed-off-by: Alvin Šipraga <alvin.sipraga@analog.com>
nunojsa
approved these changes
Aug 19, 2026
nunojsa
left a comment
nunojsa
left a comment
Collaborator
There was a problem hiding this comment.
We should wait for this to be merged upstream but it should go fairly straight!
Contributor
Author
Sure, I can hold off on merging so that I can later cherry-pick it instead :-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is actually borne from yet more MMC issues I encountered.
submitted upstream: https://lore.kernel.org/all/20260818-adp5588-gpio-cache-v1-1-650a2674fc0d@analog.com/T/#u
Basically I was getting these errors shortly after booting:
Thinking I fixed this, I reapplied the patch that I reverted the other day in #3493, hoping it would fix the issue. And it did. But in the process I looked again at the schematics and realized that the gpio-hogs for the eMMC case should actually be NO-OPs, i.e. u-boot should have already configured them that way, and before that, the pull resistors should have configured the logic gpio8=LOW gpio9=HIGH, thus connecting eMMC and gating SD card. So although I still think the fix is semantically correct, it should only actually have an effect when using an SD card. The only other thing the change does, in practice, is delay the probing of MMC. So why does that help?
I got tired of this problem so I just cracked out the oscilloscope to see if I was mad, and apparently I'm not. This is the trace of gpio9 (controlling the SD card gate switch), which should always be HIGH, about 2 seconds into kernel boot, exactly when the adp5588-keys driver is probing:
Those 320 us is the period between the two register writes in
adp5588_gpio_direction_output(), which led me to make this change, which fixed the errors I was seeing even with the revert from yesterday. And in case you don't believe me, I put an fsleep(1000) inbetween the two writes and captured this afterwards:Actual commit message follows.
So as not to clobber any pre-programmed GPIO state in the execution of its gpiochip ops, the driver caches things during probe time. However, since those ops can be called both during and immediately after the call to devm_gpiochip_add_data(), it is imperative that things are cached before that. That's not the case right now, so reorder the two steps to prevent any clobbering.
In a conrete example which motivated this change, a bootloader was preconfiguring an important GPIO output to HIGH before booting the kernel. Linux would then inadvertently set that output to LOW while configuring a GPIO hog on a discrete GPIO line within the same 8-bit bank (because the cached value was 0=LOW).
Fixes: ba9f507 ("Input: adp5588-keys - export unused GPIO pins")
PR Description
necessary to understand them. List any dependencies required for this change.
any space), or simply check them after publishing the PR.
description and try to push all related PRs simultaneously.
PR Type
PR Checklist