On Fri, 17 Dec 2021 20:00:16 +0100, Ahmad Fatoum wrote:
> On 17.12.21 19:23, Michael Tretter wrote:
> > The Solomon display drivers also support SPI in addition to the I2C.
> > Add SPI support to the driver that already supports I2C by implementing
> > the bus write function for SPI and registering an SPI driver.
> >
> > While the driver needs I2C or SPI, either subsystem is optional as long
> > as one is enabled.
> >
> > WARNING: The device tree bindings for the ssd1306 with SPI are not
> > documented!
> >
> > Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> > ---
> > drivers/video/Kconfig | 2 +-
> > drivers/video/ssd1307fb.c | 72 +++++++++++++++++++++++++++++++++++----
> > 2 files changed, 67 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index a87e8c063899..cfbd541a956e 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -15,7 +15,7 @@ config FRAMEBUFFER_CONSOLE
> >
> > config DRIVER_VIDEO_FB_SSD1307
> > bool "Solomon SSD1307 framebuffer support"
> > - depends on I2C && GPIOLIB
> > + depends on (I2C || SPI) && GPIOLIB
> >
> > config VIDEO_VPL
> > depends on OFTREE
> > diff --git a/drivers/video/ssd1307fb.c b/drivers/video/ssd1307fb.c
> > index d0df073b8ef2..2939d4348405 100644
> > --- a/drivers/video/ssd1307fb.c
> > +++ b/drivers/video/ssd1307fb.c
> > @@ -23,6 +23,7 @@
> > #include <gpio.h>
> > #include <of_gpio.h>
> > #include <regulator.h>
> > +#include <spi/spi.h>
> >
> > #define SSD1307FB_DATA 0x40
> > #define SSD1307FB_COMMAND 0x80
> > @@ -73,12 +74,14 @@ struct ssd1307fb_par {
> > u32 dclk_frq;
> > const struct ssd1307fb_deviceinfo *device_info;
> > struct i2c_client *client;
> > + struct spi_device *spi;
> > u32 height;
> > struct fb_info *info;
> > u32 page_offset;
> > u32 prechargep1;
> > u32 prechargep2;
> > int reset;
> > + int dc;
> > struct regulator *vbat;
> > u32 seg_remap;
> > u32 vcomh;
> > @@ -100,6 +103,30 @@ static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
> > return array;
> > }
> >
> > +#if IS_ENABLED(CONFIG_SPI)
> > +static int ssd1307fb_spi_write_array(struct ssd1307fb_par *par,
> > + struct ssd1307fb_array *array, u32 len)
>
> __maybe_unused and drop the #if?
Why doesn't this fail to link? The function uses spi_write, which is static
inline, but spi_write uses spi_sync, which is only defined in spi.c and is not
stubbed if !CONFIG_SPI. I would have expected to not be able to use spi_write
if CONFIG_SPI is disabled. Is there some dead code elimination happening
before linking?
Michael
>
> > +{
> > + struct spi_device *spi = par->spi;
> > + int ret;
> > +
> > + if (array->type == SSD1307FB_COMMAND)
> > + gpio_direction_output(par->dc, 0);
> > + else
> > + gpio_direction_output(par->dc, 1);
> > +
> > + ret = spi_write(spi, array->data, len);
> > + if (ret)
> > + dev_err(&spi->dev, "Couldn't send SPI command.\n");
> > +
> > + /* Ensure that we remain in data mode. */
> > + gpio_direction_output(par->dc, 1);
> > +
> > + return ret;
> > +}
> > +#endif
> > +
> > +#if IS_ENABLED(CONFIG_I2C)
>
> Ditto
>
> > static int ssd1307fb_i2c_write_array(struct ssd1307fb_par *par,
> > struct ssd1307fb_array *array, u32 len)
> > {
> > @@ -116,6 +143,7 @@ static int ssd1307fb_i2c_write_array(struct ssd1307fb_par *par,
> >
> > return 0;
> > }
> > +#endif
> >
> > static inline int ssd1307fb_write_cmd(struct ssd1307fb_par *par, u8 cmd)
> > {
> > @@ -385,6 +413,10 @@ static const struct of_device_id ssd1307fb_of_match[] = {
> > .compatible = "solomon,ssd1306fb-i2c",
> > .data = (void *)&ssd1307fb_ssd1306_deviceinfo,
> > },
> > + {
> > + .compatible = "solomon,ssd1306",
> > + .data = (void *)&ssd1307fb_ssd1306_deviceinfo,
> > + },
> > {
> > .compatible = "solomon,ssd1309fb-i2c",
> > .data = (void *)&ssd1307fb_ssd1309_deviceinfo,
> > @@ -419,9 +451,24 @@ static int ssd1307fb_probe(struct device_d *dev)
> >
> > par->device_info = (struct ssd1307fb_deviceinfo *)match->data;
> >
> > - par->client = to_i2c_client(dev);
> > - i2c_set_clientdata(par->client, par);
> > - par->write_array = ssd1307fb_i2c_write_array;
> > +#if IS_ENABLED(CONFIG_I2C)
> > + if (dev->bus == &i2c_bus) {
>
> if (IS_ENABLED(CONFIG_I2C) && dev->bus == &i2c_bus) {
>
> > + par->client = to_i2c_client(dev);
> > + i2c_set_clientdata(par->client, par);
> > + par->write_array = ssd1307fb_i2c_write_array;
> > + }
> > +#endif
> > +#if IS_ENABLED(CONFIG_SPI)
> > + if (dev->bus == &spi_bus) {
>
> Ditto
>
> > + par->spi = (struct spi_device *)dev->type_data;
> > + par->dc = of_get_named_gpio(node, "dc-gpios", 0);
> > + if (!gpio_is_valid(par->dc)) {
> > + ret = par->dc;
> > + goto fb_alloc_error;
> > + }
> > + par->write_array = ssd1307fb_spi_write_array;
> > + }
> > +#endif
> >
> > par->reset = of_get_named_gpio_flags(node,
> > "reset-gpios", 0, &of_flags);
> > @@ -591,9 +638,22 @@ fb_alloc_error:
> > return ret;
> > }
> >
> > -static struct driver_d ssd1307fb_driver = {
> > - .name = "ssd1307fb",
> > +static __maybe_unused struct driver_d ssd1307fb_i2c_driver = {
> > + .name = "ssd1307fb-i2c",
> > .probe = ssd1307fb_probe,
> > .of_compatible = DRV_OF_COMPAT(ssd1307fb_of_match),
> > };
> > -device_i2c_driver(ssd1307fb_driver);
> > +
> > +#if IS_ENABLED(CONFIG_I2C)
> > +device_i2c_driver(ssd1307fb_i2c_driver);
> > +#endif
>
> I think you should add !CONFIG_I2C stubs to i2c/i2c.h
>
> > +
> > +static __maybe_unused struct driver_d ssd1307fb_spi_driver = {
> > + .name = "ssd1307fb-spi",
> > + .probe = ssd1307fb_probe,
> > + .of_compatible = DRV_OF_COMPAT(ssd1307fb_of_match),
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_SPI)
> > +device_spi_driver(ssd1307fb_spi_driver);
> > +#endif
>
> Ditto.
>
> >
>
>
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
