dev/ichsmb: use hardware semaphore for smbus_callback #1990
+36
−5
Conversation
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
The current wait loop makes no sense. It has several flaws. First of all, it blocks in non blocking IO mode. When openening the file with O_NONBLOCK, SMB_WAIT is unset and smb_poll returns EWOULDBLOCK which causes our loop to block. Additionally, msleep is called without a timeout. So, it waits until it's waken up. Unfortunately, the SMBus might be used by BIOS/ACPI code which can block the bus. If the callback function returns an error in that case, the wakeup, called by smbus_release_bus will never be called and we're either waiting forever in the SMB_NOINTR case or for a signal in the SMB_INTR case. In order to improve the wait loop, we're inlining smbus_poll to be able to exit early in non blocking IO mode. Additionally, we're setting a timeout for msleep. This will retry the bus request in case it's blocked by BIOS/ACPI code. Note that we don't have to inline our second use of the smbus_poll function. The second call is made after the bus was requested and where it can only be blocked from another call made by ourself. So, we have to wait until the other request is done which will then call wakeup in smbus_release_bus.
It's currently an endless loop waiting for the hardware to become free. Unfortunately, the hardware can be used by BIOS/ACPI code too. In case of a faulty BIOS/ACPI code, the device may not become free and is blocked forever. To avoid a hanging kernel task add a timeout to this wait loop.
As already mentioned in the comment above the interrupt handler, the interrupt for this device can be shared. The semaphore is acquired by simply reading the status register. That's done even for spurious interrupts, so we have to release the semaphore in that case.
ICHs provide a hardware bit which can be used as semaphore. Make use of it to properly implement bus requests and releases and avoid conflicts with BIOS/ACPI code.
kostikbel
reviewed
Jan 27, 2026
|
|
||
| switch (how) { | ||
| case SMB_WAIT | SMB_INTR: | ||
| error = msleep(sc, &sc->lock, SMBPRI|PCATCH, "smbreq", hz / 100); |
bsdimp
added
the
changes-required
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
ICHs provide a hardware bit which can be used as semaphore. Make use of it to
properly implement bus requests and releases and avoid conflicts with BIOS/ACPI
code.