Hi Javier,
On Wed, Apr 13, 2022 at 6:24 PM Javier Martinez Canillas javierm@redhat.com wrote:
The ssd130x driver only provides the core support for these devices but it does not have any bus transport logic. Add a driver to interface over SPI.
There is a difference in the communication protocol when using 4-wire SPI instead of I2C. For the latter, a control byte that contains a D/C# field has to be sent. This field tells the controller whether the data has to be written to the command register or to the graphics display data memory.
But for 4-wire SPI that control byte is not used, instead a real D/C# line must be pulled HIGH for commands data and LOW for graphics display data.
For this reason the standard SPI regmap can't be used and a custom .write bus handler is needed.
Signed-off-by: Javier Martinez Canillas javierm@redhat.com Acked-by: Mark Brown broonie@kernel.org
Thanks for your patch!
--- /dev/null +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
+/*
- The regmap bus .write handler, it is just a wrapper around spi_write()
- but toggling the Data/Command control pin (D/C#). Since for 4-wire SPI
- a D/C# pin is used, in contrast with I2C where a control byte is sent,
- prior to every data byte, that contains a bit with the D/C# value.
- These control bytes are considered registers by the ssd130x core driver
- and can be used by the ssd130x SPI driver to determine if the data sent
- is for a command register or for the Graphic Display Data RAM (GDDRAM).
- */
+static int ssd130x_spi_write(void *context, const void *data, size_t count) +{
struct ssd130x_spi_transport *t = context;struct spi_device *spi = t->spi;const u8 *reg = data;if (*reg == SSD130X_COMMAND)gpiod_set_value_cansleep(t->dc, 0);if (*reg == SSD130X_DATA)gpiod_set_value_cansleep(t->dc, 1);/* Remove the control byte since is not used by the 4-wire SPI */return spi_write(spi, ((u8 *)data) + 1, count - 1);
As I don't like casts, perhaps
spi_write(spi, reg + 1, count - 1);
? But this is up to you.
+/*
- The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
- if the device was registered via OF. This means that the module will not be
- auto loaded, unless it contains an alias that matches the MODALIAS reported.
- To workaround this issue, add a SPI device ID table. Even when this should
- not be needed for this driver to match the registered SPI devices.
- */
+static const struct spi_device_id ssd130x_spi_table[] = {
{ "sh1106", SH1106_ID },{ "ssd1305", SSD1305_ID },{ "ssd1306", SSD1306_ID },{ "ssd1307", SSD1307_ID },{ "ssd1309", SSD1309_ID },{ /* sentinel */ }+}; +MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
I'm not sure about the need for this part, but as Mark provided his Ac-ed--by, I assume it's correct.
The rest LGTM, so Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds