Marcus Folkesson <marcus.folkes...@gmail.com> writes: > 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
Great! > subdirectory. > When doing that, should I replace the TINYDRM part with DRM for those drivers? > E.g. CONFIG_TINYDRM_ST7735R -> CONFIG_DRM_ST7735R. > I would drop the TINY prefix. That's also why I thought that your new driver not having TINY in its Kconfig symbol name was the correct call. > Or do we want to keep the config name intact? > > Best regards, > Marcus Folkesson -- Best regards, Javier Martinez Canillas Core Platforms Red Hat