Hello Javier, On Tue, Apr 08, 2025 at 12:44:46PM +0200, Javier Martinez Canillas wrote: > Marcus Folkesson <marcus.folkes...@gmail.com> writes: > > Hello Marcus, > > > Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. > > The controller has a SPI, I2C and 8bit parallel interface, this > > driver is for the I2C interface only. > > > > I would structure the driver differently. For example, what was done > for the Solomon SSD130X display controllers, that also support these > three interfaces: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/solomon > > Basically, it was split in a ssd130x.c module that's agnostic of the > transport interface and implements all the core logic for the driver. > > And a set of different modules that have the interface specific bits: > ssd130x-i2c.c and ssd130x-spi.c. > > That way, adding for example SPI support to your driver would be quite > trivial and won't require any refactoring. Specially since you already > are using regmap, which abstracts away the I2C interface bits. > > > Reviewed-by: Thomas Zimmermann <tzimmrm...@suse.de> > > Signed-off-by: Marcus Folkesson <marcus.folkes...@gmail.com> > > --- > > drivers/gpu/drm/tiny/Kconfig | 11 + > > drivers/gpu/drm/tiny/Makefile | 1 + > > drivers/gpu/drm/tiny/st7571-i2c.c | 721 > > ++++++++++++++++++++++++++++++++++++++ > > I personally think that the tiny sub-directory is slowly becoming a > dumping ground for small drivers. Instead, maybe we should create a > drivers/gpu/drm/sitronix/ sub-dir and put all Sitronix drivers there? > > So far we have drivers in tiny for: ST7735R, ST7586 and ST7571 with > your driver. And also have a few more Sitronix drivers in the panel > sub-directory (although those likely should remain there). > > I have a ST7565S and plan to write a driver for it. And I know someone > who is working on a ST7920 driver. That would be 5 Sitronix drivers and > the reason why I think that a dedicated sub-dir would be more organized.
I'm looking into moving all the (tiny) Sitronix drivers into their own subdirectory. When doing that, should I replace the TINYDRM part with DRM for those drivers? E.g. CONFIG_TINYDRM_ST7735R -> CONFIG_DRM_ST7735R. Or do we want to keep the config name intact? Best regards, Marcus Folkesson
signature.asc
Description: PGP signature
