linux icon indicating copy to clipboard operation
linux copied to clipboard

Add driver code for DFRobot's 8.8-inch DSI interface touch screen.

Open fary99 opened this issue 11 months ago • 1 comments

fary99 avatar Mar 22 '24 05:03 fary99

Thanks for the patches, but a few things to sort.

  • Kernel coding style mandates tabs (set to 8-spaces) rather than spaces for indentation. The checkpatch.pl script (in the scripts directory) allows quick checking for obvious coding style errors. Currently panel-dfrobot-dsi.c produces "43 errors, 194 warnings, 3 checks, 278 lines checked".
  • Adding a driver should be one patch that includes the driver C file, Kconfig, and Makefile changes. Ideally MAINTAINERS should also be updated if the driver too.
  • Adding an overlay should be one patch that includes the dtbo file, Makefile change, and documentation in README.
  • What is the need for a new dfrobot_gt911 driver when drivers/input/touchscreen/goodix.c already supports GT911? It's already used by the HyperPixel4 and Waveshare panel overlays.
  • Why is your overlay enabling i2c_arm, when your panel and touch controller are both connected to i2c_csi_dsi?
  • Have you considered adding a dsi0 override for use on Pi5?
  • Why do df_panel_enable and df_panel_disable set register 0x01, which is also set by df_panel_bl_update_status as the backlight intensity? You have assigned the backlight to the panel, so that should be handled automatically, and then be enabled at the currently configured brightness.

6by9 avatar Mar 25 '24 11:03 6by9