[PATCH AUTOSEL 6.18-5.10] drm/tegra: hdmi: sor: Fix error: variable ‘j’ set but not used

[PATCH AUTOSEL 6.18-5.10] drm/tegra: hdmi: sor: Fix error: variable ‘j’ set but not used

[Sasha Levin]

From: Brahmajit Das <listout@listout.xyz>

[ Upstream commit 1beee8d0c263b3e239c8d6616e4f8bb700bed658 ]

The variable j is set, however never used in or outside the loop, thus
resulting in dead code.
Building with GCC 16 results in a build error due to
-Werror=unused-but-set-variable= enabled by default.
This patch clean up the dead code and fixes the build error.

Example build log:
drivers/gpu/drm/tegra/sor.c:1867:19: error: variable ‘j’ set but not used [-Werror=unused-but-set-variable=]
 1867 |         size_t i, j;
      |                   ^

Signed-off-by: Brahmajit Das <listout@listout.xyz>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Link: https://lore.kernel.org/r/20250901212020.3757519-1-listout@listout.xyz
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of Commit: drm/tegra: hdmi: sor: Fix error: variable 'j' set
but not used

### 1. Commit Message Analysis

The commit message explicitly states this is a **build fix** for GCC 16,
which enables `-Werror=unused-but-set-variable=` by default. The commit
removes dead code (an unused variable `j`) to fix a compilation error.

Key indicators:
- "Fix error" - indicates a build failure
- "Building with GCC 16 results in a build error" - explicit mention of
  build breakage
- Provides concrete build log showing the error

### 2. Code Change Analysis

The changes are minimal and purely mechanical:

**In `drivers/gpu/drm/tegra/hdmi.c`:**
- Line 660: Changes `size_t i, j;` to `size_t i;`
- Line 693: Changes `for (i = 3, j = 0; i < size; i += 7, j += 8)` to
  `for (i = 3; i < size; i += 7)`

**In `drivers/gpu/drm/tegra/sor.c`:**
- Line 1866: Changes `size_t i, j;` to `size_t i;`
- Line 1899: Changes `for (i = 3, j = 0; i < size; i += 7, j += 8)` to
  `for (i = 3; i < size; i += 7)`

The variable `j` was:
1. Declared but never used anywhere
2. Incremented in the loop (`j += 8`) but the value was never read
3. Pure dead code that has no functional impact

### 3. Classification

This is a **build fix** - one of the explicit exception categories that
ARE allowed in stable:

> **BUILD FIXES:**
> - Fixes for compilation errors or warnings, Kconfig dependency fixes,
include file fixes
> - These are critical for users who need to build the kernel

### 4. Scope and Risk Assessment

- **Lines changed:** ~4 lines total (2 per file)
- **Files affected:** 2 files in the same subsystem (Tegra DRM driver)
- **Risk level:** Extremely low
- **Complexity:** None - purely removes unused code
- **Behavioral change:** None - the variable was never used

This is one of the lowest-risk changes possible:
- No logic changes
- No control flow changes
- No memory management changes
- Simply removing dead code that was never executed meaningfully

### 5. User Impact

**Who is affected:**
- Anyone trying to build the kernel with GCC 16
- Tegra (NVIDIA) platform users who need HDMI output

**Severity:**
- Without this fix, the kernel fails to compile with newer GCC versions
- This blocks users from building the kernel entirely

### 6. Stability Indicators

- Signed-off by the subsystem maintainer (Thierry Reding from NVIDIA)
- Link to the mailing list discussion provided
- The change is trivial and obviously correct

### 7. Dependency Check

- No dependencies on other commits
- The affected code (Tegra DRM HDMI/SOR drivers) exists in stable trees
- The fix is self-contained

### Conclusion

This commit meets all stable kernel criteria for a **build fix**:

1. **Obviously correct:** The variable `j` was declared, incremented,
   but never read - pure dead code
2. **Fixes a real bug:** Build failure with GCC 16
3. **Small and contained:** Only 4 lines changed across 2 related files
4. **No new features:** Removes unused code
5. **No risk of regression:** Cannot change behavior since the removed
   code was never used

Build fixes are explicitly allowed in stable trees because they are
critical for users who need to build the kernel. As GCC 16 becomes more
widely used (and is now the default compiler in some distributions),
this fix will become essential for anyone building the kernel.

The fix is trivial, obviously correct, and fixes a real build failure.
It has zero risk of causing runtime regressions since it only removes
dead code.

**YES**

 drivers/gpu/drm/tegra/hdmi.c | 4 ++--
 drivers/gpu/drm/tegra/sor.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 8cd2969e7d4bf..c4820f5e76581 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -658,7 +658,7 @@ static void tegra_hdmi_write_infopack(struct tegra_hdmi *hdmi, const void *data,
 {
 	const u8 *ptr = data;
 	unsigned long offset;
-	size_t i, j;
+	size_t i;
 	u32 value;
 
 	switch (ptr[0]) {
@@ -691,7 +691,7 @@ static void tegra_hdmi_write_infopack(struct tegra_hdmi *hdmi, const void *data,
 	 * - subpack_low: bytes 0 - 3
 	 * - subpack_high: bytes 4 - 6 (with byte 7 padded to 0x00)
 	 */
-	for (i = 3, j = 0; i < size; i += 7, j += 8) {
+	for (i = 3; i < size; i += 7) {
 		size_t rem = size - i, num = min_t(size_t, rem, 4);
 
 		value = tegra_hdmi_subpack(&ptr[i], num);
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 21f3dfdcc5c95..bc7dd562cf6b6 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1864,7 +1864,7 @@ static void tegra_sor_hdmi_write_infopack(struct tegra_sor *sor,
 {
 	const u8 *ptr = data;
 	unsigned long offset;
-	size_t i, j;
+	size_t i;
 	u32 value;
 
 	switch (ptr[0]) {
@@ -1897,7 +1897,7 @@ static void tegra_sor_hdmi_write_infopack(struct tegra_sor *sor,
 	 * - subpack_low: bytes 0 - 3
 	 * - subpack_high: bytes 4 - 6 (with byte 7 padded to 0x00)
 	 */
-	for (i = 3, j = 0; i < size; i += 7, j += 8) {
+	for (i = 3; i < size; i += 7) {
 		size_t rem = size - i, num = min_t(size_t, rem, 4);
 
 		value = tegra_sor_hdmi_subpack(&ptr[i], num);
-- 
2.51.0

Claude review: drm/tegra: hdmi: sor: Fix error: variable ‘j’ set but not used

[Claude Code Review Bot]

Overall Series Review

Subject: =?UTF-8?q?drm/tegra:=20hdmi:=20sor:=20F?=
 =?UTF-8?q?ix=20error:=20variable=20=E2=80=98j=E2=80=99=20set=20but=20not?=
 =?UTF-8?q?=20used?=
Author: Sasha Levin <sashal@kernel.org>
Patches: 1
Reviewed: 2026-02-11T17:06:48.090816

---

This is a single-patch submission for stable kernel backport (AUTOSEL 6.18-5.10) that removes unused variables from the Tegra DRM driver. The patch addresses a GCC 16 build failure caused by the `-Werror=unused-but-set-variable` flag being enabled by default.

**Summary Assessment:**
- **Type:** Build fix
- **Scope:** Minimal - removes dead code only
- **Risk:** Very low - no functional changes
- **Correctness:** The fix is correct but reveals a potential code smell

**Key Concerns:**
1. The variable `j` was being incremented by 8 in a loop that processes 7 bytes at a time, suggesting possible original intent for tracking bit positions
2. The commit message and "LLM Generated explanations" section is unusual for kernel patches
3. This appears to be an AUTOSEL (automatically selected) stable backport

**Recommendation:** The patch is technically correct as a build fix, but warrants investigation into whether the original code was buggy or incomplete.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/tegra: hdmi: sor: Fix error: variable ‘j’ set but not used

[Claude Code Review Bot]

Patch Review

**Commit Metadata:**
- Author: Brahmajit Das <listout@listout.xyz>
- Upstream: 1beee8d0c263b3e239c8d6616e4f8bb700bed658
- Maintainer: Thierry Reding (NVIDIA)
- Target: Stable backport to 6.18-5.10

**Commit Message Review:**

The commit message is functional but has issues:

```
The variable j is set, however never used in or outside the loop, thus
resulting in dead code.
Building with GCC 16 results in a build error due to
-Werror=unused-but-set-variable= enabled by default.
This patch clean up the dead code and fixes the build error.
```

Issues:
- Grammar: "This patch clean up" should be "This patch cleans up"
- The message doesn't explain WHY `j` existed or whether this reveals a bug
- Contains "LLM Generated explanations" section (lines 92-195) which is highly unusual for kernel patches and should not be in the commit

**Code Review:**

**File 1: drivers/gpu/drm/tegra/hdmi.c**

Original code:
```c
size_t i, j;
...
for (i = 3, j = 0; i < size; i += 7, j += 8) {
```

Modified code:
```c
size_t i;
...
for (i = 3; i < size; i += 7) {
```

**Analysis:**
- Loop processes data in 7-byte chunks (i += 7)
- Variable `j` was incrementing by 8 (j += 8) - one byte more than the chunk size
- The comment at hdmi.c:686-692 explains the data packing format but doesn't mention why chunks are 7 bytes

**Concern:** The mismatch between incrementing by 7 (bytes) and incrementing by 8 (potentially bits * byte?) suggests `j` might have been intended for bit offset tracking but was never completed. This could indicate:
1. Dead code from refactoring
2. Incomplete implementation
3. Copy-paste error

**File 2: drivers/gpu/drm/tegra/sor.c**

Identical changes to hdmi.c:
```c
size_t i, j;
...  
for (i = 3, j = 0; i < size; i += 7, j += 8) {
```

Changed to:
```c
size_t i;
...
for (i = 3; i < size; i += 7) {
```

**Same concern applies** - the code appears to be duplicated between hdmi.c and sor.c with the same potential issue.

**Technical Issues:**

1. **Code Duplication:** The functions `tegra_hdmi_write_infopack()` and `tegra_sor_hdmi_write_infopack()` appear to have identical logic. This duplication should be refactored into a common helper function.

2. **Missing Investigation:** The patch doesn't explain:
   - When was `j` last used (if ever)?
   - Was this a regression or always dead code?
   - Does the infopack packing work correctly without this variable?

3. **Build vs Runtime:** While this fixes the build, we should verify:
   - Are HDMI infopacks being written correctly?
   - Should there be any validation that doesn't happen now?

**Positive Aspects:**

1. **Obviously Safe:** Removing truly unused variables cannot introduce runtime bugs
2. **Fixes Real Problem:** GCC 16 build breakage is legitimate
3. **Proper Signoff Chain:** Has author SoB and maintainer SoB
4. **Targeted:** Changes only what's necessary

**Questions for Maintainer:**

1. Can you verify HDMI infopack transmission works correctly on affected hardware?
2. Was `j` intended for debugging/validation that was never implemented?
3. Should these two functions be consolidated into shared code?
4. Why is there "LLM Generated explanations" in the commit message?

**Verdict:**

**Acked-by conditionally** - The code change is correct for fixing the build, but:

- The commit message should be cleaned up (remove LLM section, fix grammar)
- Investigation should be done into whether the original code was incomplete
- Consider filing a follow-up to deduplicate the hdmi.c and sor.c functions

**Risk Assessment:** Low risk for stable backport - this is purely removing dead code.

**Testing Recommendation:** Verify HDMI output on Tegra platforms (ideally Jetson boards) to ensure infopack handling remains correct.

---
Generated by Claude Code Patch Reviewer