From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/tiny: add support for PIXPAPER 4.26 monochrome e-ink panel Date: Wed, 27 May 2026 15:04:40 +1000 Message-ID: In-Reply-To: <20260526-bar-v2-0-c66df9a840c4@gmail.com> References: <20260526-bar-v2-0-c66df9a840c4@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: drm/tiny: add support for PIXPAPER 4.26 monochrome e-ink panel Author: LiangCheng Wang Patches: 4 Reviewed: 2026-05-27T15:04:40.512648 --- This is a v2 patch series adding support for a Mayqueen Pixpaper 4.26-inch = monochrome e-ink panel (800x480) over SPI. The series has 2 patches: a DT b= inding update and the DRM tiny driver + MAINTAINERS update. The driver is largely modeled on the existing `pixpaper.c` driver for the s= maller panel, adapted for a monochrome (1-bit) panel. The code is generally= clean and follows the existing driver's patterns. However, there are sever= al issues ranging from a type mismatch bug to substantial code duplication = opportunities with the existing driver, indentation problems, and questiona= ble reset polarity logic. **Key concerns:** 1. **Wrong type for `mode_config_funcs.mode_valid`** =E2=80=94 returns `int= ` instead of `enum drm_mode_status` 2. **Significant code duplication** with existing `pixpaper.c` =E2=80=94 th= e two drivers share the same SPI/GPIO infrastructure, error context pattern= , DRM pipeline setup, and probe/remove flow. A single driver with per-panel= data would be much more maintainable. 3. **Indentation errors** in `pixpaper_connector_get_modes()` 4. **Reset polarity is inverted** compared to typical active-low reset conv= entions with `GPIOD_OUT_HIGH` 5. **Empty CRTC enable/disable** callbacks that do nothing useful 6. **Unnecessary inner brace scope** in `pixpaper_plane_atomic_update()` 7. **Uses `drm_atomic_state`** instead of the current drm-next `drm_atomic_= commit` (though this may be intentional if based on a different kernel vers= ion) --- --- Generated by Claude Code Patch Reviewer