Re: [PATCH 5/5] video: ssd1307fb: add spi support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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@pengutronix.de>
> ---
>  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?

> +{
> +	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.

> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox