[PATCH] fbcon: Remove struct fbcon_display.inverse

[PATCH] fbcon: Remove struct fbcon_display.inverse

[Thomas Zimmermann]

The field inverse in struct fbcon_display is unused. Remove it.

The field apparently never did anything. Commit c7ef5e285c84 ("video:
fbdev: atari: Fix inverse handling") converted its final user to call
fb_invert_cmaps() instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: c7ef5e285c84 ("video: fbdev: atari: Fix inverse handling")
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Helge Deller <deller@gmx.de>
Cc: <stable@vger.kernel.org> # v6.0+
---
 drivers/video/fbdev/core/fbcon.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
index 1cd10a7faab0..fca14e9b729b 100644
--- a/drivers/video/fbdev/core/fbcon.h
+++ b/drivers/video/fbdev/core/fbcon.h
@@ -30,7 +30,6 @@ struct fbcon_display {
 #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
     u_short scrollmode;             /* Scroll Method, use fb_scrollmode() */
 #endif
-    u_short inverse;                /* != 0 text black on white as default */
     short yscroll;                  /* Hardware scrolling */
     int vrows;                      /* number of virtual rows */
     int cursor_shape;
-- 
2.52.0

Re: [PATCH] fbcon: Remove struct fbcon_display.inverse

[Helge Deller]

On 2/9/26 17:15, Thomas Zimmermann wrote:
> The field inverse in struct fbcon_display is unused. Remove it.

Indeed, seems to be unused.
  
> The field apparently never did anything. Commit c7ef5e285c84 ("video:
> fbdev: atari: Fix inverse handling") converted its final user to call
> fb_invert_cmaps() instead.

That commit seems not to be related, as it touches a static
inverse variable inside the atafb driver only.
Commit e4fc27618b75 (from 2005) touched it last time, but it seems even older.

Patch applied to fbdev, but I dropped the wrong commit reference in the
commit message.

Thanks!
Helge

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: c7ef5e285c84 ("video: fbdev: atari: Fix inverse handling")
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Helge Deller <deller@gmx.de>
> Cc: <stable@vger.kernel.org> # v6.0+
> ---
>   drivers/video/fbdev/core/fbcon.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.h b/drivers/video/fbdev/core/fbcon.h
> index 1cd10a7faab0..fca14e9b729b 100644
> --- a/drivers/video/fbdev/core/fbcon.h
> +++ b/drivers/video/fbdev/core/fbcon.h
> @@ -30,7 +30,6 @@ struct fbcon_display {
>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
>       u_short scrollmode;             /* Scroll Method, use fb_scrollmode() */
>   #endif
> -    u_short inverse;                /* != 0 text black on white as default */
>       short yscroll;                  /* Hardware scrolling */
>       int vrows;                      /* number of virtual rows */
>       int cursor_shape;

Re: [PATCH] fbcon: Remove struct fbcon_display.inverse

[Thomas Zimmermann]

Hi

Am 09.02.26 um 22:31 schrieb Helge Deller:
> On 2/9/26 17:15, Thomas Zimmermann wrote:
>> The field inverse in struct fbcon_display is unused. Remove it.
>
> Indeed, seems to be unused.
>
>> The field apparently never did anything. Commit c7ef5e285c84 ("video:
>> fbdev: atari: Fix inverse handling") converted its final user to call
>> fb_invert_cmaps() instead.
>
> That commit seems not to be related, as it touches a static
> inverse variable inside the atafb driver only.
> Commit e4fc27618b75 (from 2005) touched it last time, but it seems 
> even older.

Oh well. What a stupid mistake...

>
> Patch applied to fbdev, but I dropped the wrong commit reference in the
> commit message.

Thanks a lot.

Best regards,
Thomas

>
> Thanks!
> Helge
>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: c7ef5e285c84 ("video: fbdev: atari: Fix inverse handling")
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: <stable@vger.kernel.org> # v6.0+
>> ---
>>   drivers/video/fbdev/core/fbcon.h | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/core/fbcon.h 
>> b/drivers/video/fbdev/core/fbcon.h
>> index 1cd10a7faab0..fca14e9b729b 100644
>> --- a/drivers/video/fbdev/core/fbcon.h
>> +++ b/drivers/video/fbdev/core/fbcon.h
>> @@ -30,7 +30,6 @@ struct fbcon_display {
>>   #ifdef CONFIG_FRAMEBUFFER_CONSOLE_LEGACY_ACCELERATION
>>       u_short scrollmode;             /* Scroll Method, use 
>> fb_scrollmode() */
>>   #endif
>> -    u_short inverse;                /* != 0 text black on white as 
>> default */
>>       short yscroll;                  /* Hardware scrolling */
>>       int vrows;                      /* number of virtual rows */
>>       int cursor_shape;
>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Claude review: fbcon: Remove struct fbcon_display.inverse

[Claude Code Review Bot]

Overall Series Review

Subject: fbcon: Remove struct fbcon_display.inverse
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 3
Reviewed: 2026-02-11T16:55:05.707001

---

This is a small cleanup patch series removing an unused field from the fbcon subsystem. The patch removes `struct fbcon_display.inverse`, which has been dead code for a considerable time. The series includes the initial patch submission and follow-up discussion where a maintainer (Helge Deller) correctly identifies an error in the commit message attribution and applies the patch with corrections.

**Series Assessment:**
- **Purpose**: Straightforward dead code removal
- **Scope**: Single-line deletion in a header file
- **Quality**: The cleanup is valid, but the initial submission contains an incorrect Fixes tag
- **Process**: The maintainer caught the error and corrected it before merging
- **Impact**: Minimal - purely cosmetic cleanup with no functional changes

---
Generated by Claude Code Patch Reviewer

Claude review: fbcon: Remove struct fbcon_display.inverse

[Claude Code Review Bot]

Patch Review

**From:** Thomas Zimmermann <tzimmermann@suse.de>  
**Subject:** [PATCH] fbcon: Remove struct fbcon_display.inverse  
**Date:** Mon, 9 Feb 2026 17:15:43 +0100

**Summary:**
Removes the unused `inverse` field from `struct fbcon_display` in `drivers/video/fbdev/core/fbcon.h`.

**Technical Review:**

**Change Analysis:**
```c
-    u_short inverse;                /* != 0 text black on white as default */
```

The removal is clean and straightforward. The field is:
- Not referenced anywhere in the current codebase
- A simple structure member removal requiring no other code changes
- Properly reduces structure size

**Positive Aspects:**
1. **Valid cleanup**: Dead code removal improves code maintainability
2. **Minimal scope**: Single-line change reduces risk
3. **Proper structure**: Includes sign-off, Cc to stable

**Issues Identified:**

1. **CRITICAL - Incorrect Fixes tag:**
   ```
   Fixes: c7ef5e285c84 ("video: fbdev: atari: Fix inverse handling")
   ```
   
   As Helge Deller correctly points out in his review (patch-series-filtered.mbx:343-346):
   > That commit seems not to be related, as it touches a static
   > inverse variable inside the atafb driver only.
   > Commit e4fc27618b75 (from 2005) touched it last time, but it seems even older.

   The referenced commit c7ef5e285c84 modified a *different* `inverse` variable (a static local in the atafb driver), not the struct member being removed. This makes the Fixes tag incorrect and misleading.

2. **Questionable stable backport tag:**
   ```
   Cc: <stable@vger.kernel.org> # v6.0+
   ```
   
   This is dead code removal with no functional impact. Stable backports should be reserved for actual fixes. This change:
   - Doesn't fix a bug
   - Doesn't resolve a security issue
   - Doesn't address a regression
   - Provides no user-visible benefit
   
   The stable backport is unnecessary and wastes backport effort.

3. **Commit message could be more thorough:**
   The message doesn't explain:
   - When/why the field became unused
   - What originally used it
   - How it was verified to be completely unused

**Missing Analysis:**
- No explanation of how the field was verified as unused (grep output, code inspection methodology)
- No historical context about what originally populated/read this field
- The commit message states "apparently never did anything" but then references a commit from v6.0, creating confusion

**Testing Considerations:**
- This is a compile-time-only change
- No runtime testing needed
- Should verify structure size change is acceptable for any ABI considerations (though fbcon internals should not be ABI)

**Resolution:**
The maintainer (Helge Deller) handled this correctly:
- Identified the incorrect Fixes tag
- Applied the patch with corrected commit message
- Presumably removed the inappropriate stable tag

**Recommendation:**
The cleanup itself is valid, but the patch metadata was flawed. With the corrections applied by the maintainer, this is acceptable. Future submissions should:
- Verify Fixes tags reference the actual relevant commit
- Reserve stable backports for actual fixes
- Provide more historical context in commit messages

**Final Verdict:** ACCEPTABLE (with maintainer corrections applied)

The dead code removal is correct, but the submission quality could have been better. The maintainer's intervention caught and corrected the errors.

---
Generated by Claude Code Patch Reviewer