| Message ID | 20220411211243.11121-6-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm/solomon: Add SSD130x OLED displays SPI support | expand |
Hi Javier, On Mon, Apr 11, 2022 at 11:12 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 > +static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev) > +{ > + struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW); > + > + if (IS_ERR(dc)) > + return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n")); > + > + return dc; > +} > + > +static int ssd130x_spi_probe(struct spi_device *spi) > +{ > + struct ssd130x_spi_transport *t; > + struct ssd130x_device *ssd130x; > + struct regmap *regmap; > + struct device *dev = &spi->dev; > + > + t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL); > + if (!t) > + return dev_err_probe(dev, -ENOMEM, > + "Failed to allocate SPI transport data\n"); > + > + t->spi = spi; > + > + t->dc = ssd130x_spi_get_dc(&spi->dev); > + if (IS_ERR(t->dc)) > + return PTR_ERR(t->dc); This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance) by open-coding ssd130x_spi_get_dc() here. 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
On 4/12/22 09:31, Geert Uytterhoeven wrote: [snip] >> + >> + t->spi = spi; >> + >> + t->dc = ssd130x_spi_get_dc(&spi->dev); >> + if (IS_ERR(t->dc)) >> + return PTR_ERR(t->dc); > > This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance) > by open-coding ssd130x_spi_get_dc() here. > Right, that will be better indeed.
Hi Javier,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220412]
[cannot apply to drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next linus/master linux/master airlied/drm-next v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
base: git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220412/202204121654.38UTab7Q-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
git checkout 46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/solomon/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/solomon/ssd130x-spi.c:161:35: warning: unused variable 'ssd130x_spi_table' [-Wunused-const-variable]
static const struct spi_device_id ssd130x_spi_table[] = {
^
1 warning generated.
vim +/ssd130x_spi_table +161 drivers/gpu/drm/solomon/ssd130x-spi.c
152
153 /*
154 * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
155 * if the device was registered via OF. This means that the module will not be
156 * auto loaded, unless it contains an alias that matches the MODALIAS reported.
157 *
158 * To workaround this issue, add a SPI device ID table. Even when this should
159 * not be needed for this driver to match the registered SPI devices.
160 */
> 161 static const struct spi_device_id ssd130x_spi_table[] = {
162 { "sh1106", SH1106_ID },
163 { "ssd1305", SSD1305_ID },
164 { "ssd1306", SSD1306_ID },
165 { "ssd1307", SSD1307_ID },
166 { "ssd1309", SSD1309_ID },
167 { /* sentinel */ }
168 };
169 MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
170
diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig index 8c0a0c788385..e170716d976b 100644 --- a/drivers/gpu/drm/solomon/Kconfig +++ b/drivers/gpu/drm/solomon/Kconfig @@ -20,3 +20,12 @@ config DRM_SSD130X_I2C I2C bus. If M is selected the module will be called ssd130x-i2c. + +config DRM_SSD130X_SPI + tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)" + depends on DRM_SSD130X && SPI + select REGMAP + help + Say Y here if the SSD130x OLED display is connected via SPI bus. + + If M is selected the module will be called ssd130x-spi. diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile index 4bfc5acb0447..b5fc792257d7 100644 --- a/drivers/gpu/drm/solomon/Makefile +++ b/drivers/gpu/drm/solomon/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_DRM_SSD130X) += ssd130x.o obj-$(CONFIG_DRM_SSD130X_I2C) += ssd130x-i2c.o +obj-$(CONFIG_DRM_SSD130X_SPI) += ssd130x-spi.o diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c new file mode 100644 index 000000000000..b6fee66a0c01 --- /dev/null +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * DRM driver for Solomon SSD130X OLED displays (SPI bus) + * + * Copyright 2022 Red Hat Inc. + * Authors: Javier Martinez Canillas <javierm@redhat.com> + */ +#include <linux/spi/spi.h> +#include <linux/module.h> + +#include "ssd130x.h" + +#define DRIVER_NAME "ssd130x-spi" +#define DRIVER_DESC "DRM driver for Solomon SSD130X OLED displays (SPI)" + +struct ssd130x_spi_transport { + struct spi_device *spi; + struct gpio_desc *dc; +}; + +static const struct regmap_config ssd130x_spi_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +/* + * 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); +} + +/* The ssd130x driver does not read registers but regmap expects a .read */ +static int ssd130x_spi_read(void *context, const void *reg, size_t reg_size, + void *val, size_t val_size) +{ + return -EOPNOTSUPP; +} + +/* + * A custom bus is needed due the special write that toggles a D/C# pin, + * another option could be to just have a .reg_write() callback but that + * will prevent to do data writes in bulk. + * + * Once the regmap API is extended to support defining a bulk write handler + * in the struct regmap_config, this can be simplified and the bus dropped. + */ +static struct regmap_bus regmap_ssd130x_spi_bus = { + .write = ssd130x_spi_write, + .read = ssd130x_spi_read, +}; + +static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev) +{ + struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW); + + if (IS_ERR(dc)) + return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n")); + + return dc; +} + +static int ssd130x_spi_probe(struct spi_device *spi) +{ + struct ssd130x_spi_transport *t; + struct ssd130x_device *ssd130x; + struct regmap *regmap; + struct device *dev = &spi->dev; + + t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL); + if (!t) + return dev_err_probe(dev, -ENOMEM, + "Failed to allocate SPI transport data\n"); + + t->spi = spi; + + t->dc = ssd130x_spi_get_dc(&spi->dev); + if (IS_ERR(t->dc)) + return PTR_ERR(t->dc); + + regmap = devm_regmap_init(dev, ®map_ssd130x_spi_bus, t, + &ssd130x_spi_regmap_config); + if (IS_ERR(regmap)) + return PTR_ERR(regmap); + + ssd130x = ssd130x_probe(dev, regmap); + if (IS_ERR(ssd130x)) + return PTR_ERR(ssd130x); + + spi_set_drvdata(spi, ssd130x); + + return 0; +} + +static void ssd130x_spi_remove(struct spi_device *spi) +{ + struct ssd130x_device *ssd130x = spi_get_drvdata(spi); + + ssd130x_remove(ssd130x); +} + +static void ssd130x_spi_shutdown(struct spi_device *spi) +{ + struct ssd130x_device *ssd130x = spi_get_drvdata(spi); + + ssd130x_shutdown(ssd130x); +} + +static const struct of_device_id ssd130x_of_match[] = { + { + .compatible = "sinowealth,sh1106", + .data = (void *)SH1106_ID, + }, + { + .compatible = "solomon,ssd1305", + .data = (void *)SSD1305_ID, + }, + { + .compatible = "solomon,ssd1306", + .data = (void *)SSD1306_ID, + }, + { + .compatible = "solomon,ssd1307", + .data = (void *)SSD1307_ID, + }, + { + .compatible = "solomon,ssd1309", + .data = (void *)SSD1309_ID, + }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ssd130x_of_match); + +/* + * 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); + +static struct spi_driver ssd130x_spi_driver = { + .driver = { + .name = DRIVER_NAME, + .of_match_table = ssd130x_of_match, + }, + .probe = ssd130x_spi_probe, + .remove = ssd130x_spi_remove, + .shutdown = ssd130x_spi_shutdown, +}; +module_spi_driver(ssd130x_spi_driver); + +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>"); +MODULE_LICENSE("GPL");