public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	Sasha Levin <sashal@kernel.org>,
	linusw@kernel.org, brgl@kernel.org, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 7.0-5.10] gpio: viperboard: normalize return value of gpio_get
Date: Mon, 20 Apr 2026 09:18:41 -0400	[thread overview]
Message-ID: <20260420132314.1023554-127-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

[ Upstream commit c08381ad56a9cc111f893b2b21400ceb468cc698 ]

The GPIO get callback is expected to return 0 or 1 (or a negative error
code). Ensure that the value returned by vprbrd_gpiob_get() in the output
case is normalized to the [0, 1] range.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://patch.msgid.link/20260218190657.2974723-7-dmitry.torokhov@gmail.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Connection lost, reconnecting...
Retry attempt 1...
Now I have all the information needed for a comprehensive analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: PARSE THE SUBJECT LINE
Record: [gpio: viperboard] [normalize] [Normalize return value of
gpio_get callback to [0, 1] range]

### Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- `Signed-off-by: Dmitry Torokhov` - Author
- `Link:` - patch.msgid.link (message ID
  20260218190657.2974723-**7**-dmitry.torokhov@gmail.com - patch 7/7 in
  a series)
- `Signed-off-by: Bartosz Golaszewski` - GPIO subsystem maintainer
  applied it
- No Fixes: tag, no Cc: stable, no Reported-by, no Reviewed-by

Record: Missing Fixes: and Cc: stable tags (expected for candidates).
Maintainer SOB present. Part of a 7-patch series.

### Step 1.3: ANALYZE THE COMMIT BODY TEXT
The commit says the GPIO `.get()` callback must return 0 or 1 (or
negative error). `vprbrd_gpiob_get()` in the output case returns
`gpio->gpiob_val & (1 << offset)`, which for offset > 0 returns values
like 2, 4, 8, etc., violating the API contract.

Record: Bug = API contract violation in GPIO get callback. Symptom =
non-normalized return value (e.g., 32 instead of 1 for offset 5). Root
cause = missing `!!()` normalization.

### Step 1.4: DETECT HIDDEN BUG FIXES
"Normalize" is equivalent to "fix incorrect return value." This is a
real bug fix - the function returns wrong values for the majority of
GPIO offsets (1-15 out of 16).

Record: This IS a bug fix, not cleanup.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: INVENTORY THE CHANGES
- `drivers/gpio/gpio-viperboard.c`: 1 line changed (+1/-1)
- Function modified: `vprbrd_gpiob_get()`
- Scope: single-file surgical fix, one line

Record: 1 file, 1 line changed, single function, minimal scope.

### Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
**Before:** When GPIO B pin is set as output, `return gpio->gpiob_val &
(1 << offset)` returns a bit-masked value (0, 1, 2, 4, 8, ..., 32768).

**After:** `return !!(gpio->gpiob_val & (1 << offset))` returns 0 or 1.

Record: Before = returns arbitrary power-of-2 values. After = returns
normalized 0/1.

### Step 2.3: IDENTIFY THE BUG MECHANISM
Category: **Logic / correctness fix** - incorrect return value from API
callback.

The expression `(1 << offset)` for offset=5 yields 32. If the bit is
set, `gpiob_val & 32` returns 32, not 1. The `!!` operator normalizes
any truthy value to exactly 1.

Record: Logic bug - wrong return value for GPIO offsets > 0. Fix is
`!!()` normalization.

### Step 2.4: ASSESS THE FIX QUALITY
- Obviously correct: `!!` is the standard pattern used throughout the
  kernel for this exact purpose
- Minimal: one line
- The identical fix was applied to `vprbrd_gpioa_get()` in 2015 (commit
  `80776df4f53e8`)
- Zero regression risk

Record: Fix is obviously correct, minimal, proven pattern. Zero
regression risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: BLAME THE CHANGED LINES
The buggy line (291) was introduced in commit `9d5b72de0d162` by Lars
Poeschel on 2012-11-05 - the original driver addition. This bug has
existed since the driver was first added.

Record: Buggy code from 2012, present in all stable trees since ~v3.8.

### Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag present. However, sibling commit `e2fa075d5ce19` (iio:
adc: ti-ads7950, same author, same type of fix) has `Fixes: 86ef402d805d
("gpiolib: sanitize the return value of gpio_chip::get()")` and `Cc:
stable@vger.kernel.org`.

The commit `86ef402d805d` (v6.15-rc1, Feb 2025) made gpiolib reject non-
normalized values by returning `-EBADE`. This was then partially
reverted by `ec2cceadfae72` (Feb 2026, Cc: stable) to normalize + warn
instead of error.

Record: The bug was latent since 2012. It became a functional regression
in v6.15 when `86ef402d805d` started rejecting non-normalized values
with -EBADE.

### Step 3.3: CHECK FILE HISTORY
Commit `80776df4f53e8` (Dec 2015) by Linus Walleij applied the identical
fix to `vprbrd_gpioa_get()` but missed `vprbrd_gpiob_get()`. This commit
completes that work 10 years later.

Record: Prior art exists in the same file - identical fix applied to
gpioa in 2015, gpiob was missed.

### Step 3.4: CHECK THE AUTHOR
Dmitry Torokhov is a prolific kernel contributor (input subsystem
maintainer, Google). He authored the gpiolib normalization series fixing
this same class of bug across 7+ GPIO drivers.

Record: Author is an experienced, well-known kernel developer.

### Step 3.5: CHECK FOR DEPENDENCIES
This is patch 7/7 in a series of independent per-driver fixes. Each
patch is completely standalone. No dependencies.

Record: Fully standalone, no dependencies.

---

## PHASE 4: MAILING LIST RESEARCH

### Step 4.1: ORIGINAL PATCH DISCUSSION
Found via b4 am: This is patch 7/7 in a series:
1. gpio: bd9571mwv: normalize return value of gpio_get
2. gpio: cgbc: normalize return value of gpio_get
3. gpio: da9055: normalize return value of gpio_get
4. gpio: lp873x: normalize return value of gpio_get
5. gpio: stp-xway: normalize return value of gpio_get
6. gpio: tps65086: normalize return value of gpio_get
7. gpio: viperboard: normalize return value of gpio_get (this one)

No review comments were found in the mbox (no replies). The maintainer
(Bartosz Golaszewski) applied the patches directly.

Record: Part of a 7-patch series. No review discussion found. Applied by
GPIO maintainer.

### Step 4.2: REVIEWERS
The sibling iio commit had `Reviewed-by: Andy Shevchenko`, `Reviewed-by:
Bartosz Golaszewski`, and `Reviewed-by: Linus Walleij`. This GPIO series
was applied directly by the maintainer.

Record: Applied by subsystem maintainer Bartosz Golaszewski.

### Step 4.3-4.5: BUG REPORT AND STABLE HISTORY
The root cause was Dmitry Torokhov's report that `86ef402d805d` broke
many GPIO drivers. This led to both the gpiolib normalization fix
(ec2cceadfae72, Cc: stable) and the per-driver cleanup series.

Record: Dmitry Torokhov reported the broader issue, leading to both
framework and per-driver fixes.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1-5.4: FUNCTION ANALYSIS
`vprbrd_gpiob_get()` is registered as the `.get` callback for GPIO chip
B in `vprbrd_gpio_probe()` (line 428). It's called by the gpiolib
framework via `gpiochip_get()` → `gc->get()` whenever any GPIO consumer
reads a pin value on viperboard GPIO B.

The affected path is specifically the "output cache" early return at
line 290-291, which is taken whenever a pin configured as output is
read.

Record: Called via gpiolib framework from any GPIO consumer reading pin
values. Affected path = output pin value reads.

### Step 5.5: SIMILAR PATTERNS
The exact same bug pattern (`val & BIT(offset)` instead of `!!(val &
BIT(offset))`) was fixed in 7 drivers in this series alone, plus the
prior 2015 fix for gpioa in the same file.

Record: Systematic pattern across many GPIO drivers. Well-understood,
well-tested fix pattern.

---

## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS

### Step 6.1: BUGGY CODE IN STABLE?
The buggy line was introduced in 2012. It exists in ALL active stable
trees.

### Step 6.2: BACKPORT COMPLICATIONS
One-line change to ancient code. Will apply cleanly to all stable trees.

### Step 6.3: RELATED FIXES IN STABLE
`ec2cceadfae72` (gpiolib: normalize the return value on behalf of buggy
drivers) is Cc: stable and will be backported. This provides a
framework-level safety net, but the per-driver fix eliminates the
warning.

Record: gpiolib framework fix will be in stable too, but per-driver fix
prevents warnings.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: SUBSYSTEM CRITICALITY
Subsystem: drivers/gpio - device driver, GPIO subsystem.
Criticality: PERIPHERAL - viperboard is an obscure USB GPIO device.

### Step 7.2: SUBSYSTEM ACTIVITY
Moderate activity in GPIO subsystem overall, but viperboard itself has
minimal activity (last change was commit `d9d87d90cc0b1`).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: WHO IS AFFECTED
Users of Nano River Technologies Viperboard USB GPIO hardware - a very
small population.

### Step 8.2: TRIGGER CONDITIONS
Any read of a GPIO B pin that is configured as output, at offsets 1-15.
Common operation for any viperboard GPIO user.

### Step 8.3: FAILURE MODE SEVERITY
- **On stable with 86ef402d805d but not ec2cceadfae72**: Returns -EBADE
  error instead of pin value - **MEDIUM** (functional regression)
- **On stable with both**: Produces kernel WARNING - **LOW**
- **On older stable without either**: GPIO consumers get wrong values
  (e.g., 32 instead of 1) - **MEDIUM** (incorrect behavior)

### Step 8.4: RISK-BENEFIT RATIO
**BENEFIT**: Fixes incorrect return value for all viperboard GPIO B
users. Eliminates kernel warnings.
**RISK**: Effectively zero - one-line `!!()` change, same pattern used
everywhere.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: EVIDENCE COMPILATION
**FOR backporting:**
- Real bug fix: incorrect API return value, present since 2012
- One-line, obviously correct (identical to fix applied to gpioa in
  2015)
- Zero regression risk
- Part of a series where sibling commits were explicitly tagged Cc:
  stable
- Fixes a functional regression on v6.15+ (where gpiolib returns -EBADE
  for non-normalized values)
- Standalone, no dependencies, applies cleanly to all stable trees

**AGAINST backporting:**
- Obscure hardware with very few users
- gpiolib framework fix (ec2cceadfae72, Cc: stable) already handles
  normalization at the framework level
- Severity is LOW-MEDIUM (worst case is a kernel WARNING with the
  framework fix in stable)
- No Fixes: or Cc: stable tags on this specific commit

### Step 9.2: STABLE RULES CHECKLIST
1. Obviously correct and tested? **YES** - `!!()` pattern, identical to
   prior gpioa fix
2. Fixes a real bug? **YES** - incorrect return values from GPIO get
   callback
3. Important issue? **BORDERLINE** - functional regression on v6.15+,
   wrong values on older
4. Small and contained? **YES** - 1 line, 1 file
5. No new features? **YES**
6. Can apply to stable? **YES** - cleanly applies to all trees

### Step 9.3: EXCEPTION CATEGORIES
Not an exception category - this is a standard correctness fix.

### Step 9.4: DECISION
This is a small, safe, obviously correct fix for a real API contract
violation that has existed since 2012. While the hardware is obscure,
the fix is zero-risk and the same class of fix was explicitly marked for
stable in sibling commits. The `!!()` pattern is well-established and
was already applied to the other get function in this same file.

---

## Verification

- [Phase 1] Parsed tags: Signed-off-by by author (Torokhov) and
  maintainer (Golaszewski), Link to lore.
- [Phase 2] Diff analysis: Single line change `return gpio->gpiob_val &
  (1 << offset)` → `return !!(gpio->gpiob_val & (1 << offset))` in
  vprbrd_gpiob_get().
- [Phase 3] git blame: Buggy code introduced in 9d5b72de0d162
  (2012-11-05, original driver addition).
- [Phase 3] git show 80776df4f53e8: Confirmed identical fix was applied
  to vprbrd_gpioa_get() in Dec 2015 by Linus Walleij, but gpiob_get was
  missed.
- [Phase 3] git show 86ef402d805d: Confirmed this commit (v6.15-rc1)
  made gpiolib return -EBADE for non-normalized values.
- [Phase 3] git show ec2cceadfae72: Confirmed this commit (Cc: stable)
  partially reverts to normalize+warn instead of error.
- [Phase 3] git show e2fa075d5ce19: Confirmed sibling commit (iio: ti-
  ads7950) has Fixes: 86ef402d805d and Cc: stable.
- [Phase 4] b4 am: Confirmed this is patch 7/7 in a series of 7
  identical fixes across GPIO drivers.
- [Phase 4] Mbox review: No review comments found; applied directly by
  maintainer.
- [Phase 5] Verified vprbrd_gpiob_get registered as .get callback at
  line 428 of gpio-viperboard.c.
- [Phase 5] Verified gpiolib framework calls gc->get() via
  gpiochip_get() in gpiolib.c:3277.
- [Phase 6] Bug exists in all stable trees (code from 2012). Patch
  applies cleanly.
- [Phase 6] Confirmed ec2cceadfae72 is Cc: stable and provides
  framework-level normalization.
- [Phase 8] Risk: effectively zero for a `!!()` one-liner.

**YES**

 drivers/gpio/gpio-viperboard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-viperboard.c b/drivers/gpio/gpio-viperboard.c
index 15e495c109d2a..89087fd48a819 100644
--- a/drivers/gpio/gpio-viperboard.c
+++ b/drivers/gpio/gpio-viperboard.c
@@ -288,7 +288,7 @@ static int vprbrd_gpiob_get(struct gpio_chip *chip,
 
 	/* if io is set to output, just return the saved value */
 	if (gpio->gpiob_out & (1 << offset))
-		return gpio->gpiob_val & (1 << offset);
+		return !!(gpio->gpiob_val & (1 << offset));
 
 	mutex_lock(&vb->lock);
 
-- 
2.53.0


  parent reply	other threads:[~2026-04-20 13:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-6.12] pinctrl: realtek: Fix return value and silence log for unsupported configs Sasha Levin
2026-04-20 13:17 ` [PATCH AUTOSEL 7.0-5.10] gpio: tps65086: normalize return value of gpio_get Sasha Levin
2026-04-20 13:18 ` Sasha Levin [this message]
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] pinctrl: amd: Support new ACPI ID AMDI0033 Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] gpio: da9055: normalize return value of gpio_get Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-5.10] gpio: bd9571mwv: " Sasha Levin
2026-04-20 13:21 ` [PATCH AUTOSEL 6.18] gpio: tegra: fix irq_release_resources calling enable instead of disable Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260420132314.1023554-127-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox