public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* Claude review: drm/panel: Enable GPIOLIB for panels which uses functions from it
  2026-02-26 11:57 ` [PATCH 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
@ 2026-02-27  2:17   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-02-27  2:17 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Author:** David Heidelberg

Adds `depends on GPIOLIB` (via `depends on OF && GPIOLIB`) to five panel configs:
- `DRM_PANEL_EBBG_FT8719`
- `DRM_PANEL_LG_SW43408`
- `DRM_PANEL_NOVATEK_NT36672A`
- `DRM_PANEL_NOVATEK_NT36672E`
- `DRM_PANEL_VISIONOX_RM69299`

I verified all five driver source files use `devm_gpiod_get()` and `gpiod_set_value()` / `gpiod_set_value_cansleep()`. All five changes are correct.

**Minor grammar nit in subject:** "which uses" should be "which use" (plural).

**No functional issues.** Looks good.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup
@ 2026-04-22  9:48 David Heidelberg via B4 Relay
  2026-04-22  9:48 ` [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies David Heidelberg via B4 Relay
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-04-22  9:48 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov,
	Dzmitry Sankouski, Sam Ravnborg
  Cc: Casey Connolly, marijn.suijten, dri-devel, linux-kernel,
	phone-devel, David Heidelberg

Inspired by looking at Marijn patch. Since Marijn patch is not merged yet,
appending patch to my series.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
David Heidelberg (3):
      drm/panel: Clean up S6E3FC2X01 config dependencies
      drm/panel: Clean up S6E3HA2 config dependencies and fill help text
      drm/panel: Enable GPIOLIB for panels which uses functions from it

Marijn Suijten (1):
      drm/panel: Clean up SOFEF00 config dependencies

 drivers/gpu/drm/panel/Kconfig | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
---
base-commit: 877552aa875839314afad7154b5a561889e87ea9
change-id: 20260224-panel-clean-up-kconfig-dep-7dc39c351ef8

Best regards,
-- 
David Heidelberg <david@ixit.cz>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies
  2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
@ 2026-04-22  9:48 ` David Heidelberg via B4 Relay
  2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
  2026-04-22  9:48 ` [PATCH RESEND 2/4] drm/panel: Clean up S6E3FC2X01 " David Heidelberg via B4 Relay
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-04-22  9:48 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov,
	Dzmitry Sankouski, Sam Ravnborg
  Cc: Casey Connolly, marijn.suijten, dri-devel, linux-kernel,
	phone-devel, David Heidelberg

From: Marijn Suijten <marijn.suijten@somainline.org>

As per the config name this Display IC features a DSI command-mode
interface (or the command to switch to video mode is not
known/documented) and does not use any of the video-mode helper
utilities, hence should not select VIDEOMODE_HELPERS.  In addition it
uses devm_gpiod_get() and related functions from GPIOLIB.

Fixes: 5933baa36e26 ("drm/panel/samsung-sofef00: Add panel for OnePlus 6/T devices")
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Casey Connolly <casey.connolly@linaro.org>
Reviewed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
---
 drivers/gpu/drm/panel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 79264f7bbd0e2..2fd3222ba2e3b 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -904,20 +904,20 @@ config DRM_PANEL_SAMSUNG_S6E8AA5X01_AMS561RA01
 	help
 	  Say Y here if you want to enable support for Samsung AMS561RA01
 	  panel, which uses Samsung's S6E8AA5X01 controller. The panel has a
 	  ~5.6 inch AMOLED display, and the controller is driven by the MIPI
 	  DSI protocol with 4 lanes.
 
 config DRM_PANEL_SAMSUNG_SOFEF00
 	tristate "Samsung SOFEF00 DSI panel controller"
+	depends on GPIOLIB
 	depends on OF
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
-	select VIDEOMODE_HELPERS
 	help
 	  Say Y or M here if you want to enable support for the Samsung AMOLED
 	  panel SOFEF00 DDIC and connected panel.
 	  Currently supported panels:
 
 	    Samsung AMS628NW01 (found in OnePlus 6, 1080x2280@60Hz)
 
 config DRM_PANEL_SEIKO_43WVF1G

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 2/4] drm/panel: Clean up S6E3FC2X01 config dependencies
  2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
  2026-04-22  9:48 ` [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies David Heidelberg via B4 Relay
@ 2026-04-22  9:48 ` David Heidelberg via B4 Relay
  2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
  2026-04-22  9:48 ` [PATCH RESEND 3/4] drm/panel: Clean up S6E3HA2 config dependencies and fill help text David Heidelberg via B4 Relay
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-04-22  9:48 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov,
	Dzmitry Sankouski, Sam Ravnborg
  Cc: Casey Connolly, marijn.suijten, dri-devel, linux-kernel,
	phone-devel, David Heidelberg

From: David Heidelberg <david@ixit.cz>

As per the config name this Display IC features a DSI command-mode
interface (or the command to switch to video mode is not
known/documented) and does not use any of the video-mode helper
utilities, hence should not select VIDEOMODE_HELPERS.  In addition it
uses devm_gpiod_get() and related functions from GPIOLIB.

Fixes: 88148c30ef26 ("drm/panel: Add Samsung S6E3FC2X01 DDIC with AMS641RW panel")
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/gpu/drm/panel/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 2fd3222ba2e3b..a81ea7755c03f 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -825,20 +825,19 @@ config DRM_PANEL_SAMSUNG_S6D7AA0
 	tristate "Samsung S6D7AA0 MIPI-DSI video mode panel controller"
 	depends on OF
 	depends on BACKLIGHT_CLASS_DEVICE
 	select DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
 config DRM_PANEL_SAMSUNG_S6E3FC2X01
 	tristate "Samsung S6E3FC2X01 DSI panel controller"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
-	select VIDEOMODE_HELPERS
 	help
 	  Say Y or M here if you want to enable support for the
 	  Samsung S6E3FC2 DDIC and connected MIPI DSI panel.
 	  Currently supported panels:
 
 	    Samsung AMS641RW (found in the OnePlus 6T smartphone)
 
 config DRM_PANEL_SAMSUNG_S6E3HA2

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 3/4] drm/panel: Clean up S6E3HA2 config dependencies and fill help text
  2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
  2026-04-22  9:48 ` [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies David Heidelberg via B4 Relay
  2026-04-22  9:48 ` [PATCH RESEND 2/4] drm/panel: Clean up S6E3FC2X01 " David Heidelberg via B4 Relay
@ 2026-04-22  9:48 ` David Heidelberg via B4 Relay
  2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
  2026-04-22  9:48 ` [PATCH RESEND 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
  2026-04-22 21:47 ` Claude review: drm/panel: Small Kconfig cleanup Claude Code Review Bot
  4 siblings, 1 reply; 11+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-04-22  9:48 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov,
	Dzmitry Sankouski, Sam Ravnborg
  Cc: Casey Connolly, marijn.suijten, dri-devel, linux-kernel,
	phone-devel, David Heidelberg

From: David Heidelberg <david@ixit.cz>

As per the config name this Display IC features a DSI command-mode
interface (or the command to switch to video mode is not
known/documented) and does not use any of the video-mode helper
utilities, hence should not select VIDEOMODE_HELPERS. In addition it
uses devm_gpiod_get() and related functions from GPIOLIB.

Fixes: 779679d3c164 ("drm/panel: Add support for S6E3HA8 panel driver")
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/gpu/drm/panel/Kconfig | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index a81ea7755c03f..4e7b4808f6e7c 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -844,21 +844,27 @@ config DRM_PANEL_SAMSUNG_S6E3HA2
 	tristate "Samsung S6E3HA2 DSI video mode panel"
 	depends on OF
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	select VIDEOMODE_HELPERS
 
 config DRM_PANEL_SAMSUNG_S6E3HA8
 	tristate "Samsung S6E3HA8 DSI video mode panel"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	select DRM_DISPLAY_DSC_HELPER
-	select VIDEOMODE_HELPERS
+	help
+	  Say Y or M here if you want to enable support for the
+	  Samsung S6E3HA8 DDIC and connected MIPI DSI panel.
+	  Currently supported panels:
+
+	    Samsung AMB577PX01 (found in the Samsung S9 smartphone)
+
 
 config DRM_PANEL_SAMSUNG_S6E63J0X03
 	tristate "Samsung S6E63J0X03 DSI command mode panel"
 	depends on OF
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	select VIDEOMODE_HELPERS
 

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH RESEND 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it
  2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
                   ` (2 preceding siblings ...)
  2026-04-22  9:48 ` [PATCH RESEND 3/4] drm/panel: Clean up S6E3HA2 config dependencies and fill help text David Heidelberg via B4 Relay
@ 2026-04-22  9:48 ` David Heidelberg via B4 Relay
  2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
  2026-04-22 21:47 ` Claude review: drm/panel: Small Kconfig cleanup Claude Code Review Bot
  4 siblings, 1 reply; 11+ messages in thread
From: David Heidelberg via B4 Relay @ 2026-04-22  9:48 UTC (permalink / raw)
  To: Neil Armstrong, Jessica Zhang, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Dmitry Baryshkov,
	Dzmitry Sankouski, Sam Ravnborg
  Cc: Casey Connolly, marijn.suijten, dri-devel, linux-kernel,
	phone-devel, David Heidelberg

From: David Heidelberg <david@ixit.cz>

These panels used on sdm845 devices are using GPIOLIB functions,
ensure it's enabled.

Signed-off-by: David Heidelberg <david@ixit.cz>
---
 drivers/gpu/drm/panel/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 4e7b4808f6e7c..3e819b4b23962 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -102,17 +102,17 @@ config DRM_PANEL_BOE_TV101WUM_LL2
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to support for BOE TV101WUM-LL2
 	  WUXGA PANEL DSI Video Mode panel
 
 config DRM_PANEL_EBBG_FT8719
 	tristate "EBBG FT8719 panel driver"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to enable support for the EBBG FT8719
 	  video mode panel. Mainly found on Xiaomi Poco F1 mobile phone.
 	  The panel has a resolution of 1080x2246. It provides a MIPI DSI
 	  interface to the host.
 
@@ -427,17 +427,17 @@ config DRM_PANEL_LG_LG4573
 	depends on OF && SPI
 	select VIDEOMODE_HELPERS
 	help
 	  Say Y here if you want to enable support for LG4573 RGB panel.
 	  To compile this driver as a module, choose M here.
 
 config DRM_PANEL_LG_SW43408
 	tristate "LG SW43408 panel"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	select DRM_DISPLAY_DSC_HELPER
 	select DRM_DISPLAY_HELPER
 	help
 	  Say Y here if you want to enable support for LG sw43408 panel.
 	  The panel has a 1080x2160@60Hz resolution and uses 24 bit RGB per
 	  pixel. It provides a MIPI DSI interface to the host and has a
@@ -531,27 +531,27 @@ config DRM_PANEL_NOVATEK_NT36523
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to enable support for the panels built
 	  around the Novatek NT36523 display controller, such as some
 	  Boe panels used in Xiaomi Mi Pad 5 and 5 Pro tablets.
 
 config DRM_PANEL_NOVATEK_NT36672A
 	tristate "Novatek NT36672A DSI panel"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to enable support for the panels built
 	  around the Novatek NT36672A display controller, such as some
 	  Tianma panels used in a few Xiaomi Poco F1 mobile phones.
 
 config DRM_PANEL_NOVATEK_NT36672E
 	tristate "Novatek NT36672E DSI panel"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to enable support for Novatek NT36672E DSI Video Mode
 	  LCD panel module. The panel has a resolution of 1080x2408 and uses 24 bit
 	  RGB per pixel.
 
 config DRM_PANEL_NOVATEK_NT37801
@@ -1177,17 +1177,17 @@ config DRM_PANEL_VISIONOX_R66451
 	select DRM_DISPLAY_DSC_HELPER
 	select DRM_DISPLAY_HELPER
 	help
 	  Say Y here if you want to enable support for Visionox
 	  R66451 1080x2340 AMOLED DSI panel.
 
 config DRM_PANEL_VISIONOX_RM69299
 	tristate "Visionox RM69299"
-	depends on OF
+	depends on OF && GPIOLIB
 	depends on DRM_MIPI_DSI
 	depends on BACKLIGHT_CLASS_DEVICE
 	help
 	  Say Y here if you want to enable support for Visionox
 	  RM69299  DSI Video Mode panel.
 
 config DRM_PANEL_VISIONOX_RM692E5
 	tristate "Visionox RM692E5"

-- 
2.53.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Claude review: drm/panel: Small Kconfig cleanup
  2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
                   ` (3 preceding siblings ...)
  2026-04-22  9:48 ` [PATCH RESEND 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
@ 2026-04-22 21:47 ` Claude Code Review Bot
  4 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:47 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/panel: Small Kconfig cleanup
Author: David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org>
Patches: 5
Reviewed: 2026-04-23T07:47:41.259208

---

This is a well-motivated Kconfig cleanup series for DRM panel drivers. It addresses two categories of issues: (1) removing unnecessary `select VIDEOMODE_HELPERS` from command-mode DSI panels that don't use videomode helper functions, and (2) adding missing `depends on GPIOLIB` for panels that call `devm_gpiod_get()` and related GPIO descriptor functions.

I verified all 8 affected drivers in the kernel tree. Every driver listed does indeed use GPIOLIB functions (`devm_gpiod_get`, `gpiod_set_value`, `gpiod_set_value_cansleep`) and none of them include `<video/videomode.h>` or use videomode/display_timing structures from the VIDEOMODE_HELPERS infrastructure. The changes are correct.

There is one clear error: **Patch 3's subject line says "S6E3HA2" but the patch actually modifies S6E3HA8.** The Fixes tag and diff are correct for S6E3HA8. This must be fixed before merging.

Additionally, the existing `DRM_PANEL_SAMSUNG_S6E3HA2` config entry suffers from the exact same problems (unnecessary VIDEOMODE_HELPERS, missing GPIOLIB dependency) but is not addressed by this series.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: drm/panel: Clean up SOFEF00 config dependencies
  2026-04-22  9:48 ` [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies David Heidelberg via B4 Relay
@ 2026-04-22 21:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Author:** Marijn Suijten  
**Status:** Looks good.

This patch correctly:
- Adds `depends on GPIOLIB` (the driver uses `devm_gpiod_get` and `gpiod_set_value_cansleep`)
- Removes `select VIDEOMODE_HELPERS` (the driver does not include `<video/videomode.h>` or use any videomode helper functions)

The `depends on GPIOLIB` line is placed before `depends on OF`, which is a slightly unusual ordering (alphabetically/logically `OF` typically comes first as a more fundamental dependency). This is cosmetic and not wrong, but differs from the style used in patches 2-3 where GPIOLIB is appended to the OF line with `&&`.

Already has Reviewed-by from Neil Armstrong, Casey Connolly, and David Heidelberg. No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: drm/panel: Clean up S6E3FC2X01 config dependencies
  2026-04-22  9:48 ` [PATCH RESEND 2/4] drm/panel: Clean up S6E3FC2X01 " David Heidelberg via B4 Relay
@ 2026-04-22 21:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Author:** David Heidelberg  
**Status:** Looks good.

Same pattern as patch 1. The driver (`panel-samsung-s6e3fc2x01.c`) uses `devm_gpiod_get` and `gpiod_set_value_cansleep`, and does not use videomode helpers. Changes are correct:

```
-	depends on OF
+	depends on OF && GPIOLIB
...
-	select VIDEOMODE_HELPERS
```

The commit message body is identical to patch 1 ("As per the config name this Display IC features a DSI command-mode interface..."). The Kconfig tristate says `"Samsung S6E3FC2X01 DSI panel controller"` (no "video mode" in the name), so the claim is reasonable.

No issues.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: drm/panel: Clean up S6E3HA2 config dependencies and fill help text
  2026-04-22  9:48 ` [PATCH RESEND 3/4] drm/panel: Clean up S6E3HA2 config dependencies and fill help text David Heidelberg via B4 Relay
@ 2026-04-22 21:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Author:** David Heidelberg  
**Status:** Has issues that should be fixed.

**Issue 1 (subject line error):** The subject says "S6E3HA**2**" but the diff modifies `DRM_PANEL_SAMSUNG_S6E3HA**8**`:

```
 config DRM_PANEL_SAMSUNG_S6E3HA8
 	tristate "Samsung S6E3HA8 DSI video mode panel"
-	depends on OF
+	depends on OF && GPIOLIB
```

The Fixes tag also correctly references S6E3HA8:
```
Fixes: 779679d3c164 ("drm/panel: Add support for S6E3HA8 panel driver")
```

The subject must be corrected to say "S6E3HA8".

**Issue 2 (commit message vs. tristate mismatch):** The commit message says *"this Display IC features a DSI command-mode interface"*, which is copy-pasted from patch 1's SOFEF00 description. However, the tristate says `"Samsung S6E3HA8 DSI video mode panel"`. Checking the actual driver, I see the mode_flags do NOT include `MIPI_DSI_MODE_VIDEO`, meaning the driver actually operates in command mode despite the tristate saying "video mode". So the commit message is arguably more accurate than the tristate, but this creates confusion. Consider updating the tristate description as well, or at minimum adjusting the commit message to note the discrepancy.

**Issue 3 (missed opportunity):** The adjacent `DRM_PANEL_SAMSUNG_S6E3HA2` config entry has identical problems (selects `VIDEOMODE_HELPERS` unnecessarily, and its driver uses `devm_gpiod_get`/`gpiod_set_value` without depending on GPIOLIB). Since the patches are numbered and chained, the S6E3HA2 entry would naturally be fixed in this same series.

The actual Kconfig changes (adding GPIOLIB dependency, removing VIDEOMODE_HELPERS, adding help text) are correct and verified against the driver source.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Claude review: drm/panel: Enable GPIOLIB for panels which uses functions from it
  2026-04-22  9:48 ` [PATCH RESEND 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
@ 2026-04-22 21:47   ` Claude Code Review Bot
  0 siblings, 0 replies; 11+ messages in thread
From: Claude Code Review Bot @ 2026-04-22 21:47 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Author:** David Heidelberg  
**Status:** Looks good, minor nit.

This patch adds `depends on GPIOLIB` to five panel drivers. I verified all five in the source tree:

| Driver | Uses GPIOLIB functions | Confirmed |
|--------|----------------------|-----------|
| `panel-ebbg-ft8719.c` | `devm_gpiod_get`, `gpiod_set_value_cansleep` | Yes |
| `panel-lg-sw43408.c` | `devm_gpiod_get`, `gpiod_set_value` | Yes |
| `panel-novatek-nt36672a.c` | `devm_gpiod_get`, `gpiod_set_value` | Yes |
| `panel-novatek-nt36672e.c` | `devm_gpiod_get`, `gpiod_set_value` | Yes |
| `panel-visionox-rm69299.c` | `devm_gpiod_get`, `gpiod_set_value` | Yes |

All five correctly need this dependency. Unlike patches 1-3, none of these had `select VIDEOMODE_HELPERS` to remove, so only the GPIOLIB addition is needed.

**Minor grammar nit:** The subject says "which uses" but should be "which use" (panels is plural). Trivial.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-04-22 21:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-22  9:48 [PATCH RESEND 0/4] drm/panel: Small Kconfig cleanup David Heidelberg via B4 Relay
2026-04-22  9:48 ` [PATCH RESEND 1/4] drm/panel: Clean up SOFEF00 config dependencies David Heidelberg via B4 Relay
2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
2026-04-22  9:48 ` [PATCH RESEND 2/4] drm/panel: Clean up S6E3FC2X01 " David Heidelberg via B4 Relay
2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
2026-04-22  9:48 ` [PATCH RESEND 3/4] drm/panel: Clean up S6E3HA2 config dependencies and fill help text David Heidelberg via B4 Relay
2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
2026-04-22  9:48 ` [PATCH RESEND 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
2026-04-22 21:47   ` Claude review: " Claude Code Review Bot
2026-04-22 21:47 ` Claude review: drm/panel: Small Kconfig cleanup Claude Code Review Bot
  -- strict thread matches above, loose matches on Subject: below --
2026-02-26 11:57 [PATCH 0/4] " David Heidelberg via B4 Relay
2026-02-26 11:57 ` [PATCH 4/4] drm/panel: Enable GPIOLIB for panels which uses functions from it David Heidelberg via B4 Relay
2026-02-27  2:17   ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox