fbtft: 5 years in staging

[Noralf Trønnes]

Hi,

Since I'm the original author of fbtft I thought I'd highlight a couple
of issues that's probably not well known.

Right after fbtft was added, fbdev was closed for new drivers[1] and
the fbdev maintainer wanted to remove fbtft as a consequence of that
decision, but Greg KH said he'd keep fbtft drivers in staging
"until a matching DRM driver is present in the tree"[2].

There are 2 issues wrt the goal of making a matching DRM driver
(strictly speaking). One is impossible to do (policy), the other is
unlikely to happen:

1. Device Tree 'init' property

All fbtft drivers have controller setup code that matches one
particular display panel. From the start of fbtft it was possible to
override this using platform data. Later when Device Tree support was
added, an 'init=' property to do the same was added.

Example:
init = <0x1e5 0x78F0
0x101 0x0100
0x232
0x107 0x0133>;

This translates to:
register_write(0x00e5, 0x78F0);
register_write(0x0001, 0x0100);
mdelay(32);
register_write(0x0007, 0x0133);

AFAIU setting register values from DT is a no-go, so this functionality
can't be replicated in a DRM driver. Many displays are made to work
using this mechanism, in particular ili9341 based ones.

2. Parallel bus interface

All fbtft drivers support both a SPI and a parallel bus interface
through the fbtft helper module. Many (not all) controllers support more
than one interface. The parallel bus support was added to fbtft in its
early days when not many SPI displays were available (nowadays there's
lots to choose from). fbtft uses bitbanging to drive the parallel
interface so it's really slow, even more slow than SPI and SPI with DMA
beats it thoroughly. I know there are people that use the paralell bus
support, but I don't see much point in it unless we get a parallel bus
subsystem that can use the dedicated hw on certain SoC's (Beaglebone,
Pi). And those SOC's most likely have a parallel video/RGB bus as well,
which IMO is a much better option for a panel.


The following drivers have DRM counterparts that have the same panel
setup code:

- fb_hx8357d.c: drivers/gpu/drm/tiny/hx8357d.c
- fb_ili9341.c: drivers/gpu/drm/tiny/mi0283qt.c
- fb_st7735r.c: drivers/gpu/drm/tiny/st7735r.c
- fb_ili9486.c: Patches are posted on dri-devel[3]

But they don't support all panels based on that controller and they
don't have parallel bus support.

There is actually also another obstacle for conversion and that is, some
of the displays (for which there is builtin driver support) might be
impossible to source except as second hand. And it's not always obvious
which panel is supported by a certain driver.
At least the displays supported by these drivers are listed as
discontinued on the fbtft wiki[4]:
- fb_hx8340bn.c
- fb_hx8347d.c
- fb_ili9320

This one never made it from a prototype to an actual product, because
it was too slow:
- fb_watterott.c

I have no plans to convert fbtft drivers myself, but I figured a 5 year
anniversary was a good excuse for a status update.

Noralf.

[1] https://lkml.org/lkml/2015/9/24/253
[2] https://lkml.org/lkml/2016/11/23/146
[3] https://patchwork.freedesktop.org/series/72645/
[4] https://github.com/notro/fbtft/wiki/LCD-Modules#discontinued-products
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: fbtft: 5 years in staging

[Noralf Trønnes]

Den 02.02.2020 16.41, skrev Noralf Trønnes:
> Hi,
> 
> Since I'm the original author of fbtft I thought I'd highlight a couple
> of issues that's probably not well known.
> 
> Right after fbtft was added, fbdev was closed for new drivers[1] and
> the fbdev maintainer wanted to remove fbtft as a consequence of that
> decision, but Greg KH said he'd keep fbtft drivers in staging
> "until a matching DRM driver is present in the tree"[2].
> 
> There are 2 issues wrt the goal of making a matching DRM driver
> (strictly speaking). One is impossible to do (policy), the other is
> unlikely to happen:
> 
> 1. Device Tree 'init' property
> 
> All fbtft drivers have controller setup code that matches one
> particular display panel. From the start of fbtft it was possible to
> override this using platform data. Later when Device Tree support was
> added, an 'init=' property to do the same was added.
> 
> Example:
>   init = <0x1e5 0x78F0
>   0x101 0x0100
>   0x232
>   0x107 0x0133>;
> 
> This translates to:
>   register_write(0x00e5, 0x78F0);
>   register_write(0x0001, 0x0100);
>   mdelay(32);
>   register_write(0x0007, 0x0133);
> 
> AFAIU setting register values from DT is a no-go, so this functionality
> can't be replicated in a DRM driver. Many displays are made to work
> using this mechanism, in particular ili9341 based ones.
> 
> 2. Parallel bus interface
> 
> All fbtft drivers support both a SPI and a parallel bus interface
> through the fbtft helper module. Many (not all) controllers support more
> than one interface. The parallel bus support was added to fbtft in its
> early days when not many SPI displays were available (nowadays there's
> lots to choose from). fbtft uses bitbanging to drive the parallel
> interface so it's really slow, even more slow than SPI and SPI with DMA
> beats it thoroughly. I know there are people that use the paralell bus
> support, but I don't see much point in it unless we get a parallel bus
> subsystem that can use the dedicated hw on certain SoC's (Beaglebone,
> Pi). And those SOC's most likely have a parallel video/RGB bus as well,
> which IMO is a much better option for a panel.
> 
> 
> The following drivers have DRM counterparts that have the same panel
> setup code:
> 
> - fb_hx8357d.c: drivers/gpu/drm/tiny/hx8357d.c
> - fb_ili9341.c: drivers/gpu/drm/tiny/mi0283qt.c
> - fb_st7735r.c: drivers/gpu/drm/tiny/st7735r.c
> - fb_ili9486.c: Patches are posted on dri-devel[3]
> 
> But they don't support all panels based on that controller and they
> don't have parallel bus support.
> 
> There is actually also another obstacle for conversion and that is, some
> of the displays (for which there is builtin driver support) might be
> impossible to source except as second hand. And it's not always obvious
> which panel is supported by a certain driver.
> At least the displays supported by these drivers are listed as
> discontinued on the fbtft wiki[4]:
> - fb_hx8340bn.c
> - fb_hx8347d.c
> - fb_ili9320
> 
> This one never made it from a prototype to an actual product, because
> it was too slow:
> - fb_watterott.c
> 
> I have no plans to convert fbtft drivers myself, but I figured a 5 year
> anniversary was a good excuse for a status update.

Some info for anyone wanting to convert fbtft drivers:

The following drivers are MIPI DBI compatible (like the ones already
converted) which means they have the same protocol and share certain
commands. They are very easy to port over for anyone with access to
such a display.

- fb_hx8340bn.c
- fb_hx8353d.c
- fb_ili9340.c
- fb_ili9481.c
- fb_s6d02a1.c
- fb_st7789v.c
- fb_tinylcd.c (probably has an ili9486 chip)
- fb_ili9163.c

(library: drivers/gpu/drm/drm_mipi_dbi.c)

There is also an out-of-tree ili9325 driver that supports the protocol
used by fb_ili9320 and fb_ili9325, but it doesn't support their panels
(at least the register values differ):
https://github.com/notro/tinydrm/blob/master/ili9325.c

> 
> Noralf.
> 
> [1] https://lkml.org/lkml/2015/9/24/253
> [2] https://lkml.org/lkml/2016/11/23/146
> [3] https://patchwork.freedesktop.org/series/72645/
> [4] https://github.com/notro/fbtft/wiki/LCD-Modules#discontinued-products
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code

[Noralf Trønnes]

Den 20.11.2019 15.43, skrev Noralf Trønnes:
> 
> 
> Den 20.11.2019 10.57, skrev Andy Shevchenko:
>> First of all there is no need to guard GPIO request by CONFIG_OF.
>> It works for everybody independently on resource provider. While here,
>> rename the function to reflect the above.
>>
>> Moreover, since we have a global dependency to OF, the rest of
>> conditional compilation is no-op, i.e. it's always be true.
>>
>> Due to above drop useless #ifdef CONFIG_OF and therefore dead code.
>>
>> Signed-off-by: Andy Shevchenko 
>> ---
>>  drivers/staging/fbtft/fbtft-core.c | 19 ++-
>>  1 file changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-core.c 
>> b/drivers/staging/fbtft/fbtft-core.c
> 
> 
> 
>> @@ -1184,17 +1176,10 @@ static struct fbtft_platform_data 
>> *fbtft_probe_dt(struct device *dev)
>>  pdata->display.backlight = 1;
>>  if (of_find_property(node, "init", NULL))
>>  pdata->display.fbtftops.init_display = fbtft_init_display_dt;
>> -pdata->display.fbtftops.request_gpios = fbtft_request_gpios_dt;
>> +pdata->display.fbtftops.request_gpios = fbtft_request_gpios;
> 
> You can ditch the .request_gpios callback and call fbtft_request_gpios()
> directly in fbtft_register_framebuffer(). That will make it safe to drop
> the OF dependency, otherwise .request_gpios will be NULL in the non-DT
> case. This is one of the bugs that follwed the gpio refactoring.

Really difficult to read this fbtft code (that I wrote...).
The NULL deref can only happen when dev->platform_data is set. That
can't happen, in mainline at least, now that fbtft_device is gone.

> 
> You can also ditch the .request_gpios_match callback if you want, it
> isn't called anymore (it is set in fb_agm1264k-fl).
> 
> Noralf.
> 
>>  
>>  return pdata;
>>  }
>> -#else
>> -static struct fbtft_platform_data *fbtft_probe_dt(struct device *dev)
>> -{
>> -dev_err(dev, "Missing platform data\n");
>> -return ERR_PTR(-EINVAL);
>> -}
>> -#endif
>>  
>>  /**
>>   * fbtft_probe_common() - Generic device probe() helper function
>>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1 3/5] fbtft: Drop useless #ifdef CONFIG_OF and dead code

[Noralf Trønnes]

Den 20.11.2019 10.57, skrev Andy Shevchenko:
> First of all there is no need to guard GPIO request by CONFIG_OF.
> It works for everybody independently on resource provider. While here,
> rename the function to reflect the above.
> 
> Moreover, since we have a global dependency to OF, the rest of
> conditional compilation is no-op, i.e. it's always be true.
> 
> Due to above drop useless #ifdef CONFIG_OF and therefore dead code.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/staging/fbtft/fbtft-core.c | 19 ++-
>  1 file changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-core.c 
> b/drivers/staging/fbtft/fbtft-core.c



> @@ -1184,17 +1176,10 @@ static struct fbtft_platform_data 
> *fbtft_probe_dt(struct device *dev)
>   pdata->display.backlight = 1;
>   if (of_find_property(node, "init", NULL))
>   pdata->display.fbtftops.init_display = fbtft_init_display_dt;
> - pdata->display.fbtftops.request_gpios = fbtft_request_gpios_dt;
> + pdata->display.fbtftops.request_gpios = fbtft_request_gpios;

You can ditch the .request_gpios callback and call fbtft_request_gpios()
directly in fbtft_register_framebuffer(). That will make it safe to drop
the OF dependency, otherwise .request_gpios will be NULL in the non-DT
case. This is one of the bugs that follwed the gpio refactoring.

You can also ditch the .request_gpios_match callback if you want, it
isn't called anymore (it is set in fb_agm1264k-fl).

Noralf.

>  
>   return pdata;
>  }
> -#else
> -static struct fbtft_platform_data *fbtft_probe_dt(struct device *dev)
> -{
> - dev_err(dev, "Missing platform data\n");
> - return ERR_PTR(-EINVAL);
> -}
> -#endif
>  
>  /**
>   * fbtft_probe_common() - Generic device probe() helper function
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging/fbtft: Remove flexfb

[Noralf Trønnes]

flexfb was an attempt to write a generic fbtft driver that was abandoned.
All the displays it supports are supported by other fbtft drivers.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig  |   6 -
 drivers/staging/fbtft/Makefile |   1 -
 drivers/staging/fbtft/flexfb.c | 851 -
 3 files changed, 858 deletions(-)
 delete mode 100644 drivers/staging/fbtft/flexfb.c

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 4c37d087e25c..cb61c2a772bd 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -199,9 +199,3 @@ config FB_TFT_WATTEROTT
depends on FB_TFT
help
  Generic Framebuffer support for WATTEROTT
-
-config FB_FLEX
-   tristate "Generic FB driver for TFT LCD displays"
-   depends on FB_TFT
-   help
- Generic Framebuffer support for TFT LCD displays.
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 925ec17e318d..27af43f32f81 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -36,4 +36,3 @@ obj-$(CONFIG_FB_TFT_UC1611)  += fb_uc1611.o
 obj-$(CONFIG_FB_TFT_UC1701)  += fb_uc1701.o
 obj-$(CONFIG_FB_TFT_UPD161704)   += fb_upd161704.o
 obj-$(CONFIG_FB_TFT_WATTEROTT)   += fb_watterott.o
-obj-$(CONFIG_FB_FLEX)+= flexfb.o
diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
deleted file mode 100644
index 3747321011fa..
--- a/drivers/staging/fbtft/flexfb.c
+++ /dev/null
@@ -1,851 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Generic FB driver for TFT LCD displays
- *
- * Copyright (C) 2013 Noralf Tronnes
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "fbtft.h"
-
-#define DRVNAME"flexfb"
-
-static char *chip;
-module_param(chip, charp, );
-MODULE_PARM_DESC(chip, "LCD controller");
-
-static unsigned int width;
-module_param(width, uint, );
-MODULE_PARM_DESC(width, "Display width");
-
-static unsigned int height;
-module_param(height, uint, );
-MODULE_PARM_DESC(height, "Display height");
-
-static s16 init[512];
-static int init_num;
-module_param_array(init, short, &init_num, );
-MODULE_PARM_DESC(init, "Init sequence");
-
-static unsigned int setaddrwin;
-module_param(setaddrwin, uint, );
-MODULE_PARM_DESC(setaddrwin, "Which set_addr_win() implementation to use");
-
-static unsigned int buswidth = 8;
-module_param(buswidth, uint, );
-MODULE_PARM_DESC(buswidth, "Width of databus (default: 8)");
-
-static unsigned int regwidth = 8;
-module_param(regwidth, uint, );
-MODULE_PARM_DESC(regwidth, "Width of controller register (default: 8)");
-
-static bool nobacklight;
-module_param(nobacklight, bool, );
-MODULE_PARM_DESC(nobacklight, "Turn off backlight functionality.");
-
-static bool latched;
-module_param(latched, bool, );
-MODULE_PARM_DESC(latched, "Use with latched 16-bit databus");
-
-static const s16 *initp;
-static int initp_num;
-
-/* default init sequences */
-static const s16 st7735r_init[] = {
-   -1, 0x01,
-   -2, 150,
-   -1, 0x11,
-   -2, 500,
-   -1, 0xB1, 0x01, 0x2C, 0x2D,
-   -1, 0xB2, 0x01, 0x2C, 0x2D,
-   -1, 0xB3, 0x01, 0x2C, 0x2D, 0x01, 0x2C, 0x2D,
-   -1, 0xB4, 0x07,
-   -1, 0xC0, 0xA2, 0x02, 0x84,
-   -1, 0xC1, 0xC5,
-   -1, 0xC2, 0x0A, 0x00,
-   -1, 0xC3, 0x8A, 0x2A,
-   -1, 0xC4, 0x8A, 0xEE,
-   -1, 0xC5, 0x0E,
-   -1, 0x20,
-   -1, 0x36, 0xC0,
-   -1, 0x3A, 0x05,
-   -1, 0xE0, 0x0f, 0x1a, 0x0f, 0x18, 0x2f, 0x28, 0x20, 0x22,
-   0x1f, 0x1b, 0x23, 0x37, 0x00, 0x07, 0x02, 0x10,
-   -1, 0xE1, 0x0f, 0x1b, 0x0f, 0x17, 0x33, 0x2c, 0x29, 0x2e,
-   0x30, 0x30, 0x39, 0x3f, 0x00, 0x07, 0x03, 0x10,
-   -1, 0x29,
-   -2, 100,
-   -1, 0x13,
-   -2, 10,
-   -3
-};
-
-static const s16 ssd1289_init[] = {
-   -1, 0x00, 0x0001,
-   -1, 0x03, 0xA8A4,
-   -1, 0x0C, 0x,
-   -1, 0x0D, 0x080C,
-   -1, 0x0E, 0x2B00,
-   -1, 0x1E, 0x00B7,
-   -1, 0x01, 0x2B3F,
-   -1, 0x02, 0x0600,
-   -1, 0x10, 0x,
-   -1, 0x11, 0x6070,
-   -1, 0x05, 0x,
-   -1, 0x06, 0x,
-   -1, 0x16, 0xEF1C,
-   -1, 0x17, 0x0003,
-   -1, 0x07, 0x0233,
-   -1, 0x0B, 0x,
-   -1, 0x0F, 0x,
-   -1, 0x41, 0x,
-   -1, 0x42, 0x,
-   -1, 0x48, 0x,
-   -1, 0x49, 0x013F,
-   -1, 0x4A, 0x,
-   -1, 0x4B, 0x,
-   -1, 0x44, 0xEF00,
-   -1, 0x45, 0x,
-   -1, 0x46, 0x013F,
-   -1, 0x30, 0x0707,
-   -1, 0x31, 0x0204,
-   -1, 0x32, 0x0204,
-   -1, 0x33, 0x0502,
-   -1, 0x34, 0x0507,
-   -1, 0x35, 0x0204,
-   -1, 0x36, 0x0204,
-   -1, 0x37, 0x0502,
-   -1, 0x3A, 0x0302,
-   

[PATCH 2/3] staging/fbtft: Remove fbtft_device

[Noralf Trønnes]

Commit c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
interface") removed the gpio code from fbtft_device rendering it useless.

fbtft_device is a module that was used on the Raspberry Pi to dynamically
add fbtft devices when the Pi didn't have Device Tree support.
Just remove the module since it's the responsibility of Device Tree, ACPI
or platform code to add devices.

Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig|4 -
 drivers/staging/fbtft/Makefile   |3 -
 drivers/staging/fbtft/fbtft_device.c | 1261 --
 3 files changed, 1268 deletions(-)
 delete mode 100644 drivers/staging/fbtft/fbtft_device.c

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 4e5d860fd788..4c37d087e25c 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -205,7 +205,3 @@ config FB_FLEX
depends on FB_TFT
help
  Generic Framebuffer support for TFT LCD displays.
-
-config FB_TFT_FBTFT_DEVICE
-   tristate "Module to for adding FBTFT devices"
-   depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 6bc03311c9c7..925ec17e318d 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -37,6 +37,3 @@ obj-$(CONFIG_FB_TFT_UC1701)  += fb_uc1701.o
 obj-$(CONFIG_FB_TFT_UPD161704)   += fb_upd161704.o
 obj-$(CONFIG_FB_TFT_WATTEROTT)   += fb_watterott.o
 obj-$(CONFIG_FB_FLEX)+= flexfb.o
-
-# Device modules
-obj-$(CONFIG_FB_TFT_FBTFT_DEVICE) += fbtft_device.o
diff --git a/drivers/staging/fbtft/fbtft_device.c 
b/drivers/staging/fbtft/fbtft_device.c
deleted file mode 100644
index 44e1410eb3fe..
--- a/drivers/staging/fbtft/fbtft_device.c
+++ /dev/null
@@ -1,1261 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- *
- * Copyright (C) 2013, Noralf Tronnes
- */
-
-#define pr_fmt(fmt) "fbtft_device: " fmt
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "fbtft.h"
-
-#define MAX_GPIOS 32
-
-static struct spi_device *spi_device;
-static struct platform_device *p_device;
-
-static char *name;
-module_param(name, charp, );
-MODULE_PARM_DESC(name,
-"Devicename (required). name=list => list all supported 
devices.");
-
-static unsigned int rotate;
-module_param(rotate, uint, );
-MODULE_PARM_DESC(rotate,
-"Angle to rotate display counter clockwise: 0, 90, 180, 270");
-
-static unsigned int busnum;
-module_param(busnum, uint, );
-MODULE_PARM_DESC(busnum, "SPI bus number (default=0)");
-
-static unsigned int cs;
-module_param(cs, uint, );
-MODULE_PARM_DESC(cs, "SPI chip select (default=0)");
-
-static unsigned int speed;
-module_param(speed, uint, );
-MODULE_PARM_DESC(speed, "SPI speed (override device default)");
-
-static int mode = -1;
-module_param(mode, int, );
-MODULE_PARM_DESC(mode, "SPI mode (override device default)");
-
-static unsigned int fps;
-module_param(fps, uint, );
-MODULE_PARM_DESC(fps, "Frames per second (override driver default)");
-
-static char *gamma;
-module_param(gamma, charp, );
-MODULE_PARM_DESC(gamma,
-"String representation of Gamma Curve(s). Driver specific.");
-
-static int txbuflen;
-module_param(txbuflen, int, );
-MODULE_PARM_DESC(txbuflen, "txbuflen (override driver default)");
-
-static int bgr = -1;
-module_param(bgr, int, );
-MODULE_PARM_DESC(bgr,
-"BGR bit (supported by some drivers).");
-
-static unsigned int startbyte;
-module_param(startbyte, uint, );
-MODULE_PARM_DESC(startbyte, "Sets the Start byte used by some SPI displays.");
-
-static bool custom;
-module_param(custom, bool, );
-MODULE_PARM_DESC(custom, "Add a custom display device. Use speed= argument to 
make it a SPI device, else platform_device");
-
-static unsigned int width;
-module_param(width, uint, );
-MODULE_PARM_DESC(width, "Display width, used with the custom argument");
-
-static unsigned int height;
-module_param(height, uint, );
-MODULE_PARM_DESC(height, "Display height, used with the custom argument");
-
-static unsigned int buswidth = 8;
-module_param(buswidth, uint, );
-MODULE_PARM_DESC(buswidth, "Display bus width, used with the custom argument");
-
-static s16 init[FBTFT_MAX_INIT_SEQUENCE];
-static int init_num;
-module_param_array(init, short, &init_num, );
-MODULE_PARM_DESC(init, "Init sequence, used with the custom argument");
-
-static unsigned long debug;
-module_param(debug, ulong, );
-MODULE_PARM_DESC(debug,
-"level: 0-7 (the remaining 29 bits is for advanced usage)");
-
-static uns

[PATCH 1/3] staging/fbtft: Depend on OF

[Noralf Trønnes]

Commit c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
interface") removed setting gpios via platform data. This means that
fbtft will now only work with Device Tree so set the dependency.

This also prevents a NULL pointer deref on non-DT platform because
fbtftops.request_gpios is not set in that case anymore.

Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Cc: stable 
Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 8ec524a95ec8..4e5d860fd788 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 menuconfig FB_TFT
tristate "Support for small TFT LCD display modules"
-   depends on FB && SPI
+   depends on FB && SPI && OF
depends on GPIOLIB || COMPILE_TEST
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
-- 
2.20.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 4/6] staging: fbtft: Fix sparse warnings of incorrect type in assignment

[Noralf Trønnes]

Den 02.03.2017 14.04, skrev simran singhal:

This patch fixes the following sparse warnings:

drivers/staging/fbtft/fbtft-bus.c:166:36: warning: incorrect type in assignment 
(different base types)
drivers/staging/fbtft/fbtft-bus.c:166:36:expected unsigned short [unsigned] 
[short] [usertype] 
drivers/staging/fbtft/fbtft-bus.c:166:36:got restricted __be16 [usertype] 


drivers/staging/fbtft/fbtft-io.c:74:29: warning: incorrect type in assignment 
(different base types)
drivers/staging/fbtft/fbtft-io.c:74:29:expected unsigned long long [unsigned] 
[long] [long long] [usertype] 
drivers/staging/fbtft/fbtft-io.c:74:29:got restricted __be64 [usertype] 


Signed-off-by: simran singhal 
---
  
  v2:

-changed commit message

  drivers/staging/fbtft/fbtft-bus.c | 2 +-
  drivers/staging/fbtft/fbtft-io.c  | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c 
b/drivers/staging/fbtft/fbtft-bus.c
index ec45043..df2223e 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -163,7 +163,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t 
offset, size_t len)
to_copy, remain - to_copy);
  
  		for (i = 0; i < to_copy; i++)

-   txbuf16[i] = cpu_to_be16(vmem16[i]);
+   txbuf16[i] = vmem16[i];
  


This change breaks functionality on little endian machines like
the Raspberry Pi.


Noralf.


vmem16 = vmem16 + to_copy;
ret = par->fbtftops.write(par, par->txbuf.buf,
diff --git a/drivers/staging/fbtft/fbtft-io.c b/drivers/staging/fbtft/fbtft-io.c
index d868405..ffb9a3b 100644
--- a/drivers/staging/fbtft/fbtft-io.c
+++ b/drivers/staging/fbtft/fbtft-io.c
@@ -71,7 +71,7 @@ int fbtft_write_spi_emulate_9(struct fbtft_par *par, void 
*buf, size_t len)
src++;
}
tmp |= ((*src & 0x0100) ? 1 : 0);
-   *(u64 *)dst = cpu_to_be64(tmp);
+   *(__be64 *)dst = cpu_to_be64(tmp);
dst += 8;
*dst++ = (u8)(*src++ & 0x00FF);
added++;


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V2] staging: vchiq_2835_arm: Make cache-line-size a required DT property

[Noralf Trønnes]

Den 26.02.2017 19.11, skrev Stefan Wahren:

Eric Anholt  hat am 26. Februar 2017 um 18:16 geschrieben:

...

For staging, Greg has been taking patches without platform maintainer
ack.  I think this is great -- the staging code needs *lots* of work,
and it generally doesn't need any platform knowledge.

As far as this patch, I don't know why we wouldn't just use
cache_line_size() instead of asking DT.

AFAIK this isn't implemented for arm32 yet only for arm64


cache_line_size() is available on arm32 as well.

Here's the logic:

arch/arm/mm/Kconfig:

config ARM_L1_CACHE_SHIFT_6
bool
default y if CPU_V7
help
  Setting ARM L1 cache line size to 64 Bytes.

config ARM_L1_CACHE_SHIFT
int
default 7 if ARM_L1_CACHE_SHIFT_7
default 6 if ARM_L1_CACHE_SHIFT_6
default 5


arch/arm/include/asm/cache.h:

#define L1_CACHE_SHIFT  CONFIG_ARM_L1_CACHE_SHIFT
#define L1_CACHE_BYTES  (1 << L1_CACHE_SHIFT)


include/linux/cache.h:

#ifndef CONFIG_ARCH_HAS_CACHE_LINE_SIZE
#define cache_line_size()   L1_CACHE_BYTES
#endif


The only reason for using a DT property is if one should build a kernel
that can boot on both Pi1 and Pi2/3. In theory it should be possible,
but when I did the downstream "move to mainline arch code" work, I
couldn't get such a unified kernel image to work. All kinds of memory
corruption problems that seemed to be dma related.


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] MAINTAINERS: Remove Noralf Trønnes as fbtft maintainer

[Noralf Trønnes]

Due to personal reasons I'm unable to continue as fbtft maintainer.

Signed-off-by: Noralf Trønnes 
---

For a long time I have wanted to convert the fbtft drivers to drm.
There's now a foundation in place to do so in drivers/gpu/drm/tinydrm.

Because of a worsening fatigue illness, I'm unable to take this work
any further.

I have done some conversion work on the fb_ili9325 driver[1], but I'm
unable to take it all the way to completion.

The big question for many interested in fbtft, is whether or not
backwards Device Tree compatibility can be kept. The problem I see, is
that the 'init' property takes a coded register initialization sequence,
including delays. This seems to be a empathic NO at least for new drivers.
But the hope is that since it's been in staging, maybe it can be kept.

Noralf.

[1] https://github.com/notro/tinydrm
Wiki: https://github.com/notro/tinydrm/wiki


 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6c588fd..7ab75d6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5038,7 +5038,6 @@ F:lib/fault-inject.c

 FBTFT Framebuffer drivers
 M:     Thomas Petazzoni 
-M: Noralf Trønnes 
 S: Maintained
 F: drivers/staging/fbtft/

--
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 0/5] fbtft: make it work with DMA enabled SPI

[Noralf Trønnes]

Den 11.01.2017 15:43, skrev Andy Shevchenko:

On Tue, 2017-01-03 at 20:29 +0200, Andy Shevchenko wrote:

This series enables 64x48 OLED display and fixes the driver to work
with DMA
enabled SPI properly.

Has been tested on Intel Edison board with Adafruit 2'8" and SSD1306
64x48
(Sparkfun for Intel Edison) OLED displays at their maximum speed
(25MHz and
10MHz).

Noralf, are you okay with this version of series?


Yes, Greg applies them without my ack, unless it touches core stuff.

Series:
Acked-by: Noralf Trønnes 



Since v2:
- fix kbuild bot warning
- remove duplication of might_sleep() (Noralf)
- re-do DMA appoach based on Noralf's suggestion
- append Noralf's tags

Andy Shevchenko (5):
   staging: fbtft: convert fbtft_reset() to be non-atomic
   staging: fbtft: remove custom DMA mapped buffer
   staging: fbtft: propagate error code from kstrto*()
   staging: fbtft: fb_ssd1306: Support smaller screen sizes
   staging: fbtft: fb_ssd1306: Refactor write_vmem()

  drivers/staging/fbtft/fb_ra8875.c   |  4 
  drivers/staging/fbtft/fb_ssd1306.c  | 37
-
  drivers/staging/fbtft/fbtft-core.c  | 30 ++

  drivers/staging/fbtft/fbtft-io.c|  4 
  drivers/staging/fbtft/fbtft-sysfs.c |  7 +--
  drivers/staging/fbtft/fbtft.h   |  1 -
  6 files changed, 35 insertions(+), 48 deletions(-)



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

staging/fbtft: Backward Device Tree compatibility in a drm version

[Noralf Trønnes]

Hi,

I'm working on a drivers/gpu/drm version of drivers/staging/fbtft which
are drivers for tiny, usually SPI connected, displays. Now I'm wondering
if I can be backwards compatible and support Device Trees written for the
fbtft drivers. The main obstacle as I understand it, is the init property
which are values to be written to the controller registers to support a
different panel with the same controller. It even has encoded values for
delays... My understanding is that this is not accepted.
It's in fact a display panel description in the form of register values
and delays.

Here is what a binding doc would look like for one of the fbtft drivers:

* Samsung S6D02A1 Framebuffer Driver

Required properties:
  - compatible: Should be "samsung,s6d02a1".

The node for this driver must be a child node of a SPI controller, hence
all mandatory properties described in ../spi/spi-bus.txt must be specified.

Optional properties:
- dc-gpios:D/C pin used with the 4-wire 8-bit data serial interface mode
- reset-gpios:Reset pin
- led-gpios:Backlight control
- width:Panel width in pixels
- height:Panel height in pixels
- rotate:Panel rotation in degrees counter clockwise (0,90,180,270)
- bgr:Panel is wired as BGR565 instead of RGB565
- buswidth:Bit width of the bus, in the case of SPI: 8 (4-wire) or 9 
(3-wire) bits.
- txbuflen:Size of transfer buffer. Used for little-big endian 
conversion.

- debug:Control debug output to the kernel log
- init:Panel initialization sequence overriding the driver default.
Values OR'ed with:
0x100 - Write the following values to this register.
0x200 - Delay in milliseconds

Example:
mz61581: mz61581@0{
compatible = "samsung,s6d02a1";
reg = <0>;
spi-max-frequency = <12800>;
spi-cpol;
spi-cpha;

width = <320>;
height = <480>;
rotate = <270>;
bgr;
buswidth = <8>;
txbuflen = <32768>;

reset-gpios = <&gpio 15 0>;
dc-gpios = <&gpio 25 0>;
led-gpios = <&gpio 18 0>;

init = <0x1b0 00
0x111
0x2ff
0x1b3 0x02 0x00 0x00 0x00
0x1c0 0x13 0x3b 0x00 0x02 0x00 0x01 0x00 0x43
0x1c1 0x08 0x16 0x08 0x08
0x1c4 0x11 0x07 0x03 0x03
0x1c6 0x00
0x1c8 0x03 0x03 0x13 0x5c 0x03 0x07 0x14 0x08 0x00 0x21 
0x08 0x14 0x07 0x53 0x0c 0x13 0x03 0x03 0x21 0x00

0x135 0x00
0x136 0xa0
0x13a 0x55
0x144 0x00 0x01
0x1d0 0x07 0x07 0x1d 0x03
0x1d1 0x03 0x30 0x10
0x1d2 0x03 0x14 0x04
0x129
0x12c>;

/* This is a workaround to make sure the init sequence slows down 
and doesn't fail */

debug = <3>;
};


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 5/6] staging: fbtft: fb_ssd1306: Support smaller screen sizes

[Noralf Trønnes]

Den 02.01.2017 12:35, skrev Andy Shevchenko:

There is 64x48 display exists. In order to support that set multiplexer
to 48 pixels and window address to proper position in the graphic display
data RAM.

Signed-off-by: Andy Shevchenko 
---


Patches 5 and 6:
Acked-by: Noralf Trønnes 



  drivers/staging/fbtft/fb_ssd1306.c | 21 +
  1 file changed, 21 insertions(+)

diff --git a/drivers/staging/fbtft/fb_ssd1306.c 
b/drivers/staging/fbtft/fb_ssd1306.c
index 80fc57029fee..bede2d5a5571 100644
--- a/drivers/staging/fbtft/fb_ssd1306.c
+++ b/drivers/staging/fbtft/fb_ssd1306.c
@@ -62,6 +62,8 @@ static int init_display(struct fbtft_par *par)
write_reg(par, 0xA8);
if (par->info->var.yres == 64)
write_reg(par, 0x3F);
+   else if (par->info->var.yres == 48)
+   write_reg(par, 0x2F);
else
write_reg(par, 0x1F);
  
@@ -95,6 +97,9 @@ static int init_display(struct fbtft_par *par)

if (par->info->var.yres == 64)
/* A[4]=1b, Alternative COM pin configuration */
write_reg(par, 0x12);
+   else if (par->info->var.yres == 48)
+   /* A[4]=1b, Alternative COM pin configuration */
+   write_reg(par, 0x12);
else
/* A[4]=0b, Sequential COM pin configuration */
write_reg(par, 0x02);
@@ -124,6 +129,19 @@ static int init_display(struct fbtft_par *par)
return 0;
  }
  
+static void set_addr_win_64x48(struct fbtft_par *par)

+{
+   /* Set Column Address */
+   write_reg(par, 0x21);
+   write_reg(par, 0x20);
+   write_reg(par, 0x5F);
+
+   /* Set Page Address */
+   write_reg(par, 0x22);
+   write_reg(par, 0x0);
+   write_reg(par, 0x5);
+}
+
  static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int 
ye)
  {
/* Set Lower Column Start Address for Page Addressing Mode */
@@ -132,6 +150,9 @@ static void set_addr_win(struct fbtft_par *par, int xs, int 
ys, int xe, int ye)
write_reg(par, 0x10 | 0x0);
/* Set Display Start Line */
write_reg(par, 0x40 | 0x0);
+
+   if (par->info->var.xres == 64 && par->info->var.yres == 48)
+   set_addr_win_64x48(par);
  }
  
  static int blank(struct fbtft_par *par, bool on)


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/6] staging: fbtft: fallback to usual allocation when DMA fails

[Noralf Trønnes]

Den 03.01.2017 17:12, skrev Andy Shevchenko:

On Mon, 2017-01-02 at 13:35 +0200, Andy Shevchenko wrote:

Fall back to usual allocation method if DMA coherent allocation fails.

SPI framework will map and use DMA mapped memory when possible.

Locally I have re-done DMA approach and thus this patch became optional.

Should I leave or remove it? Opinions?


If we use kmalloc always, then this goes away.


Signed-off-by: Andy Shevchenko 
---
  drivers/staging/fbtft/fbtft-core.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 226be8c09768..9f024986aff4 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -843,7 +843,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct
fbtft_display *display,
if (dma) {
txbuf = dmam_alloc_coherent(dev, txbuflen,
&par->txbuf.dma,
GFP_DMA);
-   } else
+   }
+   if (!txbuf)
  #endif
{
txbuf = devm_kzalloc(par->info->device,


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 1/6] staging: fbtft: convert fbtft_reset() to be non-atomic

[Noralf Trønnes]

Den 02.01.2017 12:35, skrev Andy Shevchenko:

First of all, fbtft in current state doesn't allow to override GPIOs to be
optional, like "reset" one. It might be a bug somewhere, but rather out of
scope of this fix.

Second, not all GPIOs available on the board would be SoC based, some of them
might sit on I2C GPIO expanders, for example, on Intel Edison/Arduino, and thus
any communication with them might sleep.

Besides that using udelay() and mdelay() is kinda resource wasteful.

Summarize all of the above, convert fbtft_reset() function to non-atomic
variant by using gpio_set_value_cansleep(), usleep_range(), and msleep().

To avoid potential use in atomic context annotate it via might_sleep() macro.

Signed-off-by: Andy Shevchenko 
---
  drivers/staging/fbtft/fbtft-core.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index bbe89c9c4fb9..e8bf0d1ec11f 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -336,11 +336,14 @@ static void fbtft_reset(struct fbtft_par *par)
  {
if (par->gpio.reset == -1)
return;
+
+   might_sleep();


gpio_set_value_cansleep() already does might_sleep().
So with that removed:
Reviewed-by: Noralf Trønnes 


+
fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__);
-   gpio_set_value(par->gpio.reset, 0);
-   udelay(20);
-   gpio_set_value(par->gpio.reset, 1);
-   mdelay(120);
+   gpio_set_value_cansleep(par->gpio.reset, 0);
+   usleep_range(20, 40);
+   gpio_set_value_cansleep(par->gpio.reset, 1);
+   msleep(120);
  }
  
  static void fbtft_update_display(struct fbtft_par *par, unsigned int start_line,


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask

[Noralf Trønnes]

Den 03.01.2017 14:58, skrev Andy Shevchenko:

On Tue, 2017-01-03 at 12:51 +0200, Andy Shevchenko wrote:

On Mon, 2017-01-02 at 19:14 +0100, Noralf Trønnes wrote:

Den 02.01.2017 12:35, skrev Andy Shevchenko:

Usually it's not consumer's business to override resources passed
from
provider, in particularly DMA coherent mask.
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -841,7 +841,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct
fbtft_display *display,
if (txbuflen > 0) {
   #ifdef CONFIG_HAS_DMA
if (dma) {
-   dev->coherent_dma_mask = ~0;

Can we make this conditional like in of_dma_configure():

   if (!dev->coherent_dma_mask)
  dev->coherent_dma_mask = DMA_BIT_MASK(32);

If not, I guess the mask has to be set before adding the spi device
in
fbtft_device.c to keep it from breaking.

Good point. I will check this.

So, I was too fast.

Clearly an SPI slave device does not and *should not* know about DMA
capabilities of SPI *host* controller. It's completely out of slave's
business.

The masks and other DMA properties only makes sense for the device which
*actually does DMA*, and apparently SPI slaves do not suit that category
(there are might be real SPI slaves with private DMA engines, though
it's another story).

Thus, the patch from my point of view should be kept in the same form.

Regarding to the kbuild bot warning, would you like me to resend it
fixed? Whatever the decision, I will wait for more comments and
hopefully your tags.

P.S. I dunno how it did work before, since DMA mask of slave device
basically has no effect. Perhaps first argument to allocator should be
NULL. That's only amendment I can see to this particular patch.



I have looked at this more closely and it seems that
spi_message.is_dma_mapped is deprecated. And if the spi master driver
can do dma, then the spi core will dma map the buffer (spi_map_buf()
even does vmalloc'ed buffers), and this is what happens for most dma
capable master drivers it seems. Since kmalloc allocates buffers that
is always dma mappable, we can always use kmalloc() instead of
dmam_alloc_coherent() for the transfer buffer.

In addition cleaning up these would prevent any later confusion:

drivers/staging/fbtft/fbtft.h:

 struct fbtft_par {
 struct {
 void *buf;
-dma_addr_t dma;
 size_t len;
 } txbuf;


drivers/staging/fbtft/fbtft-core.c:

-#include 

-#ifdef CONFIG_HAS_DMA
-static bool dma = true;
-module_param(dma, bool, 0);
-MODULE_PARM_DESC(dma, "Use DMA buffer");
-#endif

if (par->txbuf.buf)
-   sprintf(text1, ", %zu KiB %sbuffer memory",
-   par->txbuf.len >> 10, par->txbuf.dma ? "DMA " : "");
+   sprintf(text1, ", %zu KiB buffer memory", par->txbuf.len 
>> 10);



drivers/staging/fbtft/fbtft-io.c:

 int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len)
-   if (par->txbuf.dma && buf == par->txbuf.buf) {
-   t.tx_dma = par->txbuf.dma;
-   m.is_dma_mapped = 1;
-   }


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 2/6] staging: fbtft: do not override DMA coherent mask

[Noralf Trønnes]

Den 02.01.2017 12:35, skrev Andy Shevchenko:

Usually it's not consumer's business to override resources passed from
provider, in particularly DMA coherent mask.

Signed-off-by: Andy Shevchenko 
---
  drivers/staging/fbtft/fbtft-core.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index e8bf0d1ec11f..226be8c09768 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -841,7 +841,6 @@ struct fb_info *fbtft_framebuffer_alloc(struct 
fbtft_display *display,
if (txbuflen > 0) {
  #ifdef CONFIG_HAS_DMA
if (dma) {
-   dev->coherent_dma_mask = ~0;


Can we make this conditional like in of_dma_configure():

 if (!dev->coherent_dma_mask)
dev->coherent_dma_mask = DMA_BIT_MASK(32);

If not, I guess the mask has to be set before adding the spi device in
fbtft_device.c to keep it from breaking.

Noralf.


txbuf = dmam_alloc_coherent(dev, txbuflen,
&par->txbuf.dma, GFP_DMA);
} else


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: fbtft: do not allocate huge txbuf

[Noralf Trønnes]

[resending got rejected on the list for html format]

Den 09.06.2016 19:18, skrev Michal Suchanek:

Hello,

On 9 June 2016 at 18:20, Noralf Trønnes  wrote:

Den 09.06.2016 17:08, skrev Michal Suchanek:

txbuflen can be set to arbitrary value by user and it is also set
automagically to the maximum transfer size of the SPI master controller.


AFAICT this is a result of your previous patch. Please make a new version of
your previous patch with this fix in it. Also make a note in that thread
that

That patch is pretty much independent and just exposes this problem
under different circumstances. The txbuflen can be also set by the
user.


a new version is made so Greg doesn't pull it when he sees my ack. He
handles a large volume of patches so let's make it as easy as we can for
him.

Which would be letting in the patches separately imho.



Ok,

Acked-by: Noralf Trønnes 


Thanks

Michal



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: fbtft: do not allocate huge txbuf

[Noralf Trønnes]

Den 09.06.2016 17:08, skrev Michal Suchanek:

txbuflen can be set to arbitrary value by user and it is also set
automagically to the maximum transfer size of the SPI master controller.


AFAICT this is a result of your previous patch. Please make a new version of
your previous patch with this fix in it. Also make a note in that thread 
that

a new version is made so Greg doesn't pull it when he sees my ack. He
handles a large volume of patches so let's make it as easy as we can for 
him.


Thanks,
Noralf.


Do not allocate the buffer when larger than vmem. When my SPI master
controller reports maximum transfer size 16M the probe of fbtft fails.

Signed-off-by: Michal Suchanek 
---
  drivers/staging/fbtft/fbtft-core.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index f3bdc8f..b95cf69 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -820,6 +820,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct 
fbtft_display *display,
/* Transmit buffer */
if (txbuflen == -1)
txbuflen = vmem_size + 2; /* add in case startbyte is used */
+   if (txbuflen >= vmem_size + 2)
+   txbuflen = 0;
  
  #ifdef __LITTLE_ENDIAN

if ((!txbuflen) && (bpp > 8))


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] fbtft: limit transfer length by spi device limit

[Noralf Trønnes]

Den 26.05.2016 21:25, skrev Michal Suchanek:

Some SPI controllers can transfer only small piece of data at a time.
Since SPI core gained a function to get the maximum transfer length use
it.

Signed-off-by: Michal Suchanek 
---

  Tested on sunxi spi with DMA enabled and disabled. Makes a visible speed
difference and display works in either case.

  drivers/staging/fbtft/fbtft-core.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index 0c1a77c..f3bdc8f 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1346,6 +1346,15 @@ int fbtft_probe_common(struct fbtft_display *display,
return PTR_ERR(pdata);
}
  
+	if (sdev && (spi_max_transfer_size(sdev) < SIZE_MAX))

+   if ((pdata->txbuflen <= 0) || (pdata->txbuflen > 
spi_max_transfer_size(sdev))) {
+   dev_warn(dev,
+"Limiting used buffer size %i -> %i due to device 
%s transfer size limitation",
+pdata->txbuflen, spi_max_transfer_size(sdev),
+dev_name(&sdev->dev));
+   pdata->txbuflen = spi_max_transfer_size(sdev);
+   }
+
info = fbtft_framebuffer_alloc(display, dev, pdata);
if (!info)
return -ENOMEM;


(Ugh, this code of mine looks worse each time I'm confronted with it.)

You have even taken care of the special txbuflen == -1 value I see,
so I guess this is as good as it gets without any major refactoring, so:

Acked-by: Noralf Trønnes 

And there's no point in doing any refactoring since I'm working on a DRM
successor for fbtft. I have been working on it since the fbdev maintainer
issued the "No more new fbdev drivers, please" call in September and now
it has reached a tipping point where I can say that I will keep working
on it until it's done.
Some info: https://github.com/notro/tinydrm/wiki

Thanks,
Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Noralf Trønnes]

Den 11.10.2015 09:31, skrev Dennis Menschel:

Am 10.10.2015 um 17:36 schrieb Noralf Trønnes:

Den 07.10.2015 22:15, skrev Dennis Menschel:

This patch adds support for the Sitronix ST7789V display controller.
The controller is intended for small color displays with a resolution
of up to 320x240 pixels.

Signed-off-by: Dennis Menschel 
---


...


(blank() is used on OLED controllers and set_gamma() will be obsolete with
display drivers since gamma can be set in init())


...


Thank you for your detailed explanation about the current state of fbtft
and the future plans for a possible successor framework. As suggested by
you, I'll make the following changes in the next patch:

- Change the st7789v controller driver to a cberry28 display driver.
- If applicable, use the default fbtft implementation of set_addr_win().
- Remove the blank() function as it is only intended for OLED displays.


I want to expand a bit on blank(), it can be used by all drivers, but all
the non-OLED displays I have tried turns off the pixels when it's blanked,
so the backlight shines through, making it of no use (no power savings to
speak of either). So these displays need the backlight to be turned off
during blanking. The main reason blank() isn't implemented in the
controller drivers, is that if it's used on a display without backlight
control, the display would turn white during blanking.

There's a bug in the fbtft backlight implementation that prevents it from
turning off backlight on the first fb_blank. Subsequent blanks are ok.
I haven't looked into it, because the fbtft backlight implementation
should really be handled by the gpio-backlight driver. That's what I've
done in my new work.


Noralf.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Noralf Trønnes]

Den 07.10.2015 22:15, skrev Dennis Menschel:

This patch adds support for the Sitronix ST7789V display controller.
The controller is intended for small color displays with a resolution
of up to 320x240 pixels.

Signed-off-by: Dennis Menschel 
---


The future of fbtft is uncertain after fbdev maintainer Tomi Valkeinen
suggested that it be removed [1], since fbdev is a deprecated framework.
I have started to look at a DRM version, but this will take some time pull
through (fbtft is just one of my hobby projects). I suggest that we still
accept new drivers and if necessary convert them to DRM drivers later.

If we do that, there's one change I would like to see and that is switching
to display/panel drivers instead of controller drivers. The only common
functionality between controllers, that we make use of, is the
set_addr_win() function (I didn't know that when I started fbtft based on
the 2 displays I had). This is a small function so I don't see a problem
duplicating this in new drivers. It's also possible to export the function
from a controller driver. fbtft has a default set_addr_win() implementation
that matches the MIPI compatible controllers (0x2A, 0x2B, 0x2C).
(blank() is used on OLED controllers and set_gamma() will be obsolete with
display drivers since gamma can be set in init())

Whatever fbtft turns into, it will use display drivers instead of
controller drivers. I have done some work on fbtft that gives a hint on
how it might look [2] (I stopped that work when I understood that fbtft
might not move out of staging after all).
The main reason for switching to displays drivers is that most controllers
have a quite large register set with lots of settings. This is quite
impossible to turn into Device Tree properties. And many panels also use
undocumented registers.

fbtft_device
The fbtft_device module provides a way to add devices for the fbtft drivers.
It was made to make life simple for Raspberry Pi users when it's kernel
didn't support Device Tree. There are no other modules like it in the 
kernel,

so it will not move out of staging. The reason I haven't removed it is that
it's tightly coupled with the flexfb driver making it usable with panels
that's not supported by the other drivers. For instance the set_par()
function, in the drivers that implement it, hardcodes some rotation/mirror
register values that are panel specific. flexfb doesn't have a set_par()
function, so it won't overwrite values set in init(), making it useful
with some panels. I suggest that we don't make any more additions to
fbtft_device, but just keep it as it is for now. fbtft_device has many
module parameters that should cover most if not all configs [3].


So for this particular patch I suggest you turn it into a display driver
and that we merge that (drop the fbtft_device patch).
Please write the init sequence in an init() function using write_reg()
since init_sequence will go away. There's an enum for the MIPI DCS
commands in include/video/mipi_display.h


Noralf.

[1] https://lkml.org/lkml/2015/9/24/253
[2] https://github.com/notro/linux-staging/commits/next
[3] https://github.com/notro/fbtft/wiki/fbtft_device

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: fbtft: use pr_fmt

[Noralf Trønnes]

10.09.2015 06:54, skrev Sudip Mukherjee:

On Wed, Sep 09, 2015 at 11:20:08PM +0200, Noralf Trønnes wrote:

Den 09.09.2015 20:35, skrev Greg Kroah-Hartman:

On Sat, Sep 05, 2015 at 07:13:45PM +0530, Sudip Mukherjee wrote:

Instead of defining DRVNAME and using it in all calls to pr_* family of
macros lets start using pr_fmt.

Signed-off-by: Sudip Mukherjee 
---
  drivers/staging/fbtft/fbtft_device.c | 79 
  1 file changed, 35 insertions(+), 44 deletions(-)

[...]


If a driver is working properly, nothing should show up in the kernel
log at all, otherwise it's just noise that everyone ignores.

This isn't a device driver, it's a module for adding "fbtft" devices
(spi/platform with pdata). When I created fbtft, the Raspberry Pi didn't
have Device Tree support, so I made this module as a way for the end user
to add devices without having to build a kernel.
I haven't seen any module like this in the kernel, so maybe it
really doesn't
belong here at all?

Is this driver only for Raspberry Pi?
I have seen someone submitted a drm driver for Raspberry Pi. And I guess
that is already merged.
Tomi (fbdev maintainer) said "Fremebuffer  driver will be obsolete
immediately when there's a DRM driver for that device". And if a drm
driver is already there for Raspberry Pi then ?


Not sure what you refer to as driver here.

The fbtft module presents a simplistic view of fbdev through
'struct fbtft_par' which the various fbtft drivers use.

fbtft_device is just a kernel module that can add spi and platform
devices to the kernel and it can be used on all platforms. It's not a
driver of any kind. It really does what Device Tree does, it adds devices.

vc4 is an upcoming DRM driver for the BCM2835 display hardware (hdmi).
This has nothing to do with the fbtft drivers which target SPI/parallel
bus connected displays with onboard video memory.


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 3/3] staging: fbtft: use pr_fmt

[Noralf Trønnes]

Den 09.09.2015 20:35, skrev Greg Kroah-Hartman:

On Sat, Sep 05, 2015 at 07:13:45PM +0530, Sudip Mukherjee wrote:

Instead of defining DRVNAME and using it in all calls to pr_* family of
macros lets start using pr_fmt.

Signed-off-by: Sudip Mukherjee 
---
  drivers/staging/fbtft/fbtft_device.c | 79 
  1 file changed, 35 insertions(+), 44 deletions(-)


[...]


If a driver is working properly, nothing should show up in the kernel
log at all, otherwise it's just noise that everyone ignores.


This isn't a device driver, it's a module for adding "fbtft" devices
(spi/platform with pdata). When I created fbtft, the Raspberry Pi didn't
have Device Tree support, so I made this module as a way for the end user
to add devices without having to build a kernel.
I haven't seen any module like this in the kernel, so maybe it really 
doesn't

belong here at all?


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging/fbtft: use spi_setup instead of direct call to master->setup

[Noralf Trønnes]

Den 01.09.2015 19:52, skrev Brüns, Stefan:

On Tuesday, September 01, 2015 16:57:14 Noralf Trønnes wrote:

IMHO, this is a bad idea. A controller supporting more than 32 bpw can
not set the bpw mask, as the mask is limited to 32 bits. Thus it has to
check the bpw in its own setup function, and may or may not reject 9 bpw.

Why should one penalize any controllers having an unset bpw mask, which
is a completely valid configuration?

BTW, the SPI core patch needed for this to work is in the 4.3 pull
request.

I have acked a patch from Stefan Wahren adressing this issue:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-Augus
t/075423.html

Two issues:

1st: Stefan Wahren posted a patch large part identical to mine, 18 hours
later. Coincidence?


For my part I admit to being sloppy here, I should have pulled you into 
the thread.


According to the commit message, Stefan Wahren seem to have detected 
this problem
on the ARM MXS platform and you refere to xilinx, so I guess it's a 
coincidence.



2nd: Modifying bpw and not calling spi_setup() is IMHO wrong. The controller
setup function could e.g. modify max_speed_hz.


We are only changing bits_per_word, and spi_setup() has already been called.

Callchain for DT registered spi devices:
spi_register_master->of_register_spi_devices->of_register_spi_device->
spi_add_device->spi_setup

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging/fbtft: use spi_setup instead of direct call to master->setup

[Noralf Trønnes]

Den 01.09.2015 01:28, skrev Stefan Bruens:

On Monday 24 August 2015 01:24:59 Noralf Trønnes wrote:

Den 24.08.2015 00:24, skrev Stefan Brüns:

Avoid a crash, as master->setup may be NULL (e.g. xilinx controller).
Even if master->setup is set, spi_setup does several compatibility/
sanity checks which should not be skipped (fixes problems with
displays/controllers needing emulation for bits_per_word = 9).

Signed-off-by: Stefan Brüns 
---

   drivers/staging/fbtft/fb_watterott.c | 4 ++--
   drivers/staging/fbtft/fbtft-core.c   | 4 ++--
   drivers/staging/fbtft/flexfb.c   | 4 ++--
   3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c index 9cc8141..ba08da3 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1434,12 +1434,12 @@ int fbtft_probe_common(struct fbtft_display
*display,>
/* 9-bit SPI setup */
if (par->spi && display->buswidth == 9) {

par->spi->bits_per_word = 9;

-   ret = par->spi->master->setup(par->spi);
+   ret = spi_setup(par->spi);

You could also check against master->bits_per_word_mask to verify that
9-bit is supported before setting bits_per_word=9 and then drop
spi_setup() altogether. And if bits_per_word_mask is not set, assume
9-bit is not supported.

IMHO, this is a bad idea. A controller supporting more than 32 bpw can
not set the bpw mask, as the mask is limited to 32 bits. Thus it has to
check the bpw in its own setup function, and may or may not reject 9 bpw.

Why should one penalize any controllers having an unset bpw mask, which
is a completely valid configuration?

BTW, the SPI core patch needed for this to work is in the 4.3 pull request.


I have acked a patch from Stefan Wahren adressing this issue:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-August/075423.html

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: fbtft: fix 9-bit SPI support detection

[Noralf Trønnes]

Den 25.08.2015 23:04, skrev Stefan Wahren:

Since the result of the setup function isn't adequate to check
9-bit SPI support, we better check bits_per_word_mask. Btw this
change avoids a NULL pointer dereference with master drivers
without a separate setup function.

Signed-off-by: Stefan Wahren 
---
  drivers/staging/fbtft/fbtft-core.c |   10 +++---
  drivers/staging/fbtft/flexfb.c |   11 ---
  2 files changed, 7 insertions(+), 14 deletions(-)


Thanks Stefan.

Both patches:
Acked-by: Noralf Trønnes 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection

[Noralf Trønnes]

Den 25.08.2015 19:34, skrev Stefan Wahren:

Noralf Trønnes  hat am 25. August 2015 um 00:00
geschrieben:



Den 24.08.2015 20:33, skrev Stefan Wahren:

Since bits_per_word isn't usually checked during SPI setup the 9-bit
support must be checked manually.

Signed-off-by: Stefan Wahren 
---
drivers/staging/fbtft/fbtft-core.c | 7 +++
drivers/staging/fbtft/flexfb.c | 7 +++
2 files changed, 14 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 3638554..bd71487 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display,
if (par->spi && display->buswidth == 9) {
par->spi->bits_per_word = 9;
ret = spi_setup(par->spi);
+ if (!ret) {
+ struct spi_master *ma = par->spi->master;
+
+ if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9)))
+ ret = -EINVAL;
+ }
+
if (ret) {

There's no point in calling spi_setup() when it doesn't check bits_per_word.

If checking of bits_per_word is the only intention of the setup call, then i
agree.
But i'm not sure it is safe to remove the setup call complete.

Couldn't this cause regressions since there is no common spi setup call for all
drivers?


spi_add_device() calls spi_setup() for all spi devices so it's already done.
If spi_setup() did something with bits_per_word, then I agree that we should
call it in the 9-bit case. But this is not the case, and it's possible to
change bits_per_word per transfer later, so I can't see that this is needed.


Apparently this changed with the commit:
spi: convert drivers to use bits_per_word_mask.
This has not been detected earlier, because FBTFT was previously mostly
used on the Raspberry Pi which had a downstream SPI driver that did this
check.

How about this:

- par->spi->bits_per_word = 9;
- ret = par->spi->master->setup(par->spi);
+ if (par->spi->master->bits_per_word_mask &
SPI_BPW_MASK(9)) {
+ par->spi->bits_per_word = 9;
- if (ret) {
+ } else {
dev_warn(&par->spi->dev,
"9-bit SPI not available, emulating
using 8-bit.\n");
- par->spi->bits_per_word = 8;

I think this assignment should stay.


Not needed since we haven't changed the default set by spi_setup():
if (!spi->bits_per_word)
spi->bits_per_word = 8;

Callchain for DT registered spi devices:
spi_register_master->of_register_spi_devices->of_register_spi_device->
spi_add_device->spi_setup

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH RFC 2/2] staging: fbtft: fix 9-bit SPI support detection

[Noralf Trønnes]

Den 24.08.2015 20:33, skrev Stefan Wahren:

Since bits_per_word isn't usually checked during SPI setup the 9-bit
support must be checked manually.

Signed-off-by: Stefan Wahren 
---
  drivers/staging/fbtft/fbtft-core.c |7 +++
  drivers/staging/fbtft/flexfb.c |7 +++
  2 files changed, 14 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index 3638554..bd71487 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1438,6 +1438,13 @@ int fbtft_probe_common(struct fbtft_display *display,
if (par->spi && display->buswidth == 9) {
par->spi->bits_per_word = 9;
ret = spi_setup(par->spi);
+   if (!ret) {
+   struct spi_master *ma = par->spi->master;
+
+   if (!(ma->bits_per_word_mask & SPI_BPW_MASK(9)))
+   ret = -EINVAL;
+   }
+
if (ret) {


There's no point in calling spi_setup() when it doesn't check bits_per_word.
Apparently this changed with the commit:
spi: convert drivers to use bits_per_word_mask.
This has not been detected earlier, because FBTFT was previously mostly
used on the Raspberry Pi which had a downstream SPI driver that did this 
check.


How about this:

-par->spi->bits_per_word = 9;
-ret = par->spi->master->setup(par->spi);
+if (par->spi->master->bits_per_word_mask & 
SPI_BPW_MASK(9)) {

+par->spi->bits_per_word = 9;
-if (ret) {
+} else {
 dev_warn(&par->spi->dev,
 "9-bit SPI not available, emulating 
using 8-bit.\n");

-par->spi->bits_per_word = 8;
-ret = par->spi->master->setup(par->spi);
-if (ret)
-goto out_release;
 /* allocate buffer with room for dc bits */
 par->extra = devm_kzalloc(par->info->device,

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging/fbtft: use spi_setup instead of direct call to master->setup

[Noralf Trønnes]

Den 24.08.2015 00:24, skrev Stefan Brüns:

Avoid a crash, as master->setup may be NULL (e.g. xilinx controller).
Even if master->setup is set, spi_setup does several compatibility/
sanity checks which should not be skipped (fixes problems with
displays/controllers needing emulation for bits_per_word = 9).

Signed-off-by: Stefan Brüns 
---
  drivers/staging/fbtft/fb_watterott.c | 4 ++--
  drivers/staging/fbtft/fbtft-core.c   | 4 ++--
  drivers/staging/fbtft/flexfb.c   | 4 ++--
  3 files changed, 6 insertions(+), 6 deletions(-)



diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index 9cc8141..ba08da3 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1434,12 +1434,12 @@ int fbtft_probe_common(struct fbtft_display *display,
/* 9-bit SPI setup */
if (par->spi && display->buswidth == 9) {
par->spi->bits_per_word = 9;
-   ret = par->spi->master->setup(par->spi);
+   ret = spi_setup(par->spi);


You could also check against master->bits_per_word_mask to verify that
9-bit is supported before setting bits_per_word=9 and then drop
spi_setup() altogether. And if bits_per_word_mask is not set, assume
9-bit is not supported.


if (ret) {
dev_warn(&par->spi->dev,
"9-bit SPI not available, emulating using 
8-bit.\n");
par->spi->bits_per_word = 8;
-   ret = par->spi->master->setup(par->spi);
+   ret = spi_setup(par->spi);
if (ret)
goto out_release;
/* allocate buffer with room for dc bits */




___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: fbtft: Use a struct to describe each LCD controller

[Noralf Trønnes]

Den 02.08.2015 16:57, skrev Fabio Falzoi:

Use a struct flexfb_lcd_controller to holds chip properties, instead of
relying on a long 'if - else if' chain.
This allows to:
- use a simple linear search to verify if a certain LCD controller
model is supported or not.
- add support for a new LCD chip controller simply defining a new
flexfb_lcd_controller struct.

Signed-off-by: Fabio Falzoi 
Acked-by: Noralf Trønnes 
---


Seems I wasn't clear enough, you could use my ack if you put
struct flexfb_lcd_controller inside the driver and not in
fbtft.h


Noralf.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: core: Don't set device platform_data

[Noralf Trønnes]

Pass platform_data as an argument to fbtft_framebuffer_alloc()
instead of using dev->platform_data.

This fixes an issue where the device comes from Device Tree
and fbtft_probe_common() sets dev->platform_data to allocated
memory. When the module is reloaded, dev->platform_data points
to freed memory.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/fbtft-core.c | 12 +---
 drivers/staging/fbtft/fbtft.h  |  5 +++--
 drivers/staging/fbtft/flexfb.c |  2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index f04128f..23392eb 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -677,13 +677,13 @@ static void fbtft_merge_fbtftops(struct fbtft_ops *dst, 
struct fbtft_ops *src)
  *
  */
 struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
-   struct device *dev)
+   struct device *dev,
+   struct fbtft_platform_data *pdata)
 {
struct fb_info *info;
struct fbtft_par *par;
struct fb_ops *fbops = NULL;
struct fb_deferred_io *fbdefio = NULL;
-   struct fbtft_platform_data *pdata = dev->platform_data;
u8 *vmem = NULL;
void *txbuf = NULL;
void *buf = NULL;
@@ -828,7 +828,7 @@ struct fb_info *fbtft_framebuffer_alloc(struct 
fbtft_display *display,
 
par = info->par;
par->info = info;
-   par->pdata = dev->platform_data;
+   par->pdata = pdata;
par->debug = display->debug;
par->buf = buf;
spin_lock_init(&par->dirty_lock);
@@ -1265,12 +1265,11 @@ EXPORT_SYMBOL(fbtft_init_display);
  */
 static int fbtft_verify_gpios(struct fbtft_par *par)
 {
-   struct fbtft_platform_data *pdata;
+   struct fbtft_platform_data *pdata = par->pdata;
int i;
 
fbtft_par_dbg(DEBUG_VERIFY_GPIOS, par, "%s()\n", __func__);
 
-   pdata = par->info->device->platform_data;
if (pdata->display.buswidth != 9 && par->startbyte == 0 &&
par->gpio.dc < 0) {
dev_err(par->info->device,
@@ -1388,10 +1387,9 @@ int fbtft_probe_common(struct fbtft_display *display,
pdata = fbtft_probe_dt(dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
-   dev->platform_data = pdata;
}
 
-   info = fbtft_framebuffer_alloc(display, dev);
+   info = fbtft_framebuffer_alloc(display, dev, pdata);
if (!info)
return -ENOMEM;
 
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 7d817eb..ab4a658 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -264,8 +264,9 @@ struct fbtft_par {
 /* fbtft-core.c */
 extern void fbtft_dbg_hex(const struct device *dev,
int groupsize, void *buf, size_t len, const char *fmt, ...);
-extern struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
-   struct device *dev);
+struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
+   struct device *dev,
+   struct fbtft_platform_data *pdata);
 extern void fbtft_framebuffer_release(struct fb_info *info);
 extern int fbtft_register_framebuffer(struct fb_info *fb_info);
 extern int fbtft_unregister_framebuffer(struct fb_info *fb_info);
diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index ce6e3ae..5b4c712 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -379,7 +379,7 @@ static int flexfb_probe_common(struct spi_device *sdev,
fbtft_init_dbg(dev, "regwidth = %d\n", regwidth);
fbtft_init_dbg(dev, "buswidth = %d\n", buswidth);
 
-   info = fbtft_framebuffer_alloc(&flex_display, dev);
+   info = fbtft_framebuffer_alloc(&flex_display, dev, dev->platform_data);
if (!info)
return -ENOMEM;
 
-- 
2.2.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: fbtft: Add support for the Ultrachip UC1611 LCD controller

[Noralf Trønnes]

Den 14.07.2015 14:59, skrev Henri Chain:

This is a driver chip for 240x160 4-bit greyscale LCDs.
It is capable of 4-wire (8 bit) or 3-wire (9 bit) SPI that have both been
tested. (It also has a 6800 or 8080-style parallel interface, but I have
not included support for it.)

Signed-off-by: Henri Chain 
---



diff --git a/drivers/staging/fbtft/fb_uc1611.c 
b/drivers/staging/fbtft/fb_uc1611.c



+static int init_display(struct fbtft_par *par)
+{
+   int ret;
+
+   fbtft_par_dbg(DEBUG_INIT_DISPLAY, par, "%s()\n", __func__);
+
+   /* Set CS active high */
+   par->spi->mode |= SPI_CS_HIGH;


Why is this set here and not in fbtft_device along side SPI_MODE_3?


+static int set_var(struct fbtft_par *par)
+{
+   fbtft_par_dbg(DEBUG_INIT_DISPLAY, par, "%s()\n", __func__);
+
+   /* par->info->fix.visual = FB_VISUAL_PSEUDOCOLOR; */


This comment can be removed I guess.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Staging: fbtft: Add support for the Ultrachip UC1611 LCD controller

[Noralf Trønnes]

Den 15.07.2015 11:36, skrev Paul Bolle:

On di, 2015-07-14 at 14:59 +0200, Henri Chain wrote:

--- /dev/null
+++ b/drivers/staging/fbtft/fb_uc1611.c
+#define DRVNAME"fb_uc1611"
+MODULE_ALIAS("spi:" DRVNAME);
+MODULE_ALIAS("platform:" DRVNAME);
+MODULE_ALIAS("spi:uc1611");
+MODULE_ALIAS("platform:uc1611");

Many of the drivers under drivers/staging/fbtft use a comparable set of
aliases. But I wonder if they are all needed (here, and in the other
drivers).

In this case I think I understand how the "fb_uc1611" .modalias (see
below) will eventually trigger a "MODALIAS=spi:fb_uc1611" uevent. And
that uevent will make userspace load the fb_uc1611.ko module, right?

But is there a similar way that "spi:uc1611" fits into the system?
Because I couldn't spot anything similar for "uc1611".


If I remember correctly, this is used for autoloading the module when 
using Device Tree.



Likewise, "platform:fb_uc1611" and "platform:uc1611" require struct
platform_device's with "fb_uc1611" and "uc1611" .name's. But I couldn't
spot where platform_device's with those .name's are created. How do
these two aliases fit into the system?


Most of these display controllers support both SPI and 8080 parallel 
interface.

So the FBTFT_REGISTER_DRIVER macro sets up both a SPI and a platform driver.
I can't remember the reason why I couldn't put the MODULE_ALIAS'es inside
FBTFT_REGISTER_DRIVER.
If the chip only has a SPI interface, then the platform aliases are not 
needed.



Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 06/10] Staging: fbtft: Use a helper function to set set_addr_win op

[Noralf Trønnes]

Den 15.07.2015 04:14, skrev Greg KH:

On Tue, Jun 30, 2015 at 08:43:13AM +0200, Fabio Falzoi wrote:

Use a helper function to choose which set_addr_win implementation to
use, based on the value of the setaddrwin module parameter.

Signed-off-by: Fabio Falzoi 
---
  drivers/staging/fbtft/flexfb.c | 47 +-
  1 file changed, 28 insertions(+), 19 deletions(-)

Again, why?

I need a maintainer of the code to ack any of these...



I know this code fairly well, but I have to look up the details.
Moving these details from the probe function into small functions
makes it more difficult for me get to those details in a glance.
I would now have to scroll back and forth to see how flexfb is
doing things, especially since flexfb differs from the other drivers
which uses common functions in fbtft-core.c.
So I prefer to keep it as it is.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/10] Staging: fbtft: Use a struct to describe each LCD controller

[Noralf Trønnes]

Den 30.06.2015 08:43, skrev Fabio Falzoi:

Use a struct flexfb_lcd_controller to holds chip properties, instead of
relying on a long 'if - else if' chain.
This allows to:
- use a simple linear search to verify if a certain LCD controller
model is supported or not.
- add support for a new LCD chip controller simply defining a new
flexfb_lcd_controller struct.

Signed-off-by: Fabio Falzoi 
---
  drivers/staging/fbtft/fbtft.h  |  20 
  drivers/staging/fbtft/flexfb.c | 212 ++---
  2 files changed, 136 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 7d817eb..c96c06b 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -256,6 +256,26 @@ struct fbtft_par {
void *extra;
  };
  
+/**

+ * struct flexfb_lcd_controller - Describes the LCD controller properties
+ * @name: Model name of the chip
+ * @width: Width of display in pixels
+ * @height: Height of display in pixels
+ * @setaddrwin: Which set_addr_win() implementation to use
+ * @regwidth: LCD Controller Register width in bits
+ * @init_seq: LCD initialization sequence
+ * @init_seq_sz: Size of LCD initialization sequence
+ */
+struct flexfb_lcd_controller {
+   const char *name;
+   unsigned int width;
+   unsigned int height;
+   unsigned int setaddrwin;
+   unsigned int regwidth;
+   int *init_seq;
+   int init_seq_sz;
+};
+


Please put this in flexfb.c since it won't be used outside that file.


  #define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__})/sizeof(int))
  
  #define write_reg(par, ...)  \

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index ed867e7..25b394d 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -126,6 +126,89 @@ static int ssd1351_init[] = { -1, 0xfd, 0x12, -1, 0xfd, 
0xb1, -1, 0xae, -1, 0xb3
  -1, 0xab, 0x01, -1, 0xb1, 0x32, -1, 0xb4, 0xa0, 
0xb5, 0x55, -1, 0xbb, 0x17, -1, 0xbe, 0x05,
  -1, 0xc1, 0xc8, 0x80, 0xc8, -1, 0xc7, 0x0f, -1, 
0xb6, 0x01, -1, 0xa6, -1, 0xaf, -3 };
  
+static const struct flexfb_lcd_controller flexfb_chip_table[] = {

+   {
+   .name = "st7735r",
+   .width = 120,
+   .height = 160,
+   .init_seq = st7735r_init,
+   .init_seq_sz = ARRAY_SIZE(st7735r_init),
+   },
+   {


Can this be put on one line? }, {

With the struct moved:
Acked-by: Noralf Trønnes 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: Add reset to fbtft_init_display_dt()

[Noralf Trønnes]

When an init sequence is present in the Device Tree,
fbtft_init_display_dt() is used to initialize the display.
Add missing reset function call and activation of
chip select for parallel bus.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/fbtft-core.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index 37dcf7e..70fdd4d 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1086,6 +1086,11 @@ static int fbtft_init_display_dt(struct fbtft_par *par)
p = of_prop_next_u32(prop, NULL, &val);
if (!p)
return -EINVAL;
+
+   par->fbtftops.reset(par);
+   if (par->gpio.cs != -1)
+   gpio_set_value(par->gpio.cs, 0);  /* Activate chip */
+
while (p) {
if (val & FBTFT_OF_INIT_CMD) {
val &= 0x;
-- 
2.2.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC 0/7] staging: fbtft: minimize coupling to the fbdev subsystem

[Noralf Trønnes]

Den 02.03.2015 11:54, skrev Noralf Trønnes:

Hi,

This patchset introduces a new API to minimize the coupling to the
fbdev graphics subsystem. There have been calls to deprecate fbdev and
new fbdev drivers are discouraged.
Currently the FBTFT drivers are tightly coupled to fbdev through the
fbtft module and the fbtft_par structure.

What is FBTFT?
FBTFT is a framework for writing drivers for MIPI DCS/DBI [1] like controllers.
These controllers have the following characteristics:
- Onchip graphics RAM that is scanned out to the display panel.
- Registers to set operational parameters and write pixel data
- SPI, I2C and/or parallel interface

To provide better layering and provide opportunity for reuse,
I propose the following layers:

+-+
|  fbdev  |
+-+
|  fbtft  |
+-+---+
|  Display driver |   |
+---+-+   |
|   | lcdctrl |
|   +-+
| lcdreg  |
+-+
|  Interface (SPI, parallel)  |
+-+

lcdreg
--
This layer deals with the various LCD controller interface
implementations (SPI, Intel 8080, I2C). It provides a simple interface
for reading and writing to the LCD controller register.
Especially the SPI interface has several modes and variations that
makes it rather complex.

lcdreg can be reused by drivers that drive these same controllers in
digital RGB interface mode (MIPI DPI). Even when the parallel RGB
interface is used for pixeldata, the controller some times has to be
configured. This is usually done through a SPI interface.

lcdctrl
---
Layer providing an abstraction of the LCD controller and it's
principal funtions:
- Poweron initialization
- Set display rotation
- Set pixel format
- Display blanking
- Update grahics RAM

This decouples the controller implementation from the graphics subsystem,
and prepares for a possible future move to DRM or other grahics subsystems.
The controller specific functionality is kept in library modules.

fbtft
-
The fbtft module gets some helper functions in fbtft-lcdctrl to map
the lcdtrl functions into the fbtft_par structure.
Later when all the drivers have been converted, large parts of the
fbtft module can be removed.


The I2C implementation of lcdreg is used in this proposal, because of
it's simplicity. This is to keep focus on the architectural design.
Implementations for SPI and the Intel 8080 bus interface exists.

Display or Controller driver?
Currently FBTFT has drivers for each controller it supports.
Each driver has a default controller setup which often matches many
displays, but no all (fbtftops.init_display()). A different init
sequence (register values and delays) can be passed in using
platform_data or as a DT property.
I made a post to the DT mailinglist [2] about setting an init sequence
from the Device Tree, but didn't get any replies. Based on an earlier
post and comments I've read, it's looks like init sequence in DT is
not going to happen. That's why this proposal shifts to having display
drivers, with controller specific code in library modules.

Device Tree questions
- What about Device Tree property names that are used by all drivers
   (initialized, format, rotation)? Should they be prefixed in any way?
- Device Tree compatible string. If a driver supports both SPI and I2C,
   can the same compatible string be used for both busses (ada-ssd1306fb)?
   I have seen examples where the bus is added as a postfix (-i2c).
   And which one is preferred: 'ada-ssd1306' or 'ada-ssd1306fb'?

Why RFC?
I did this because of some uncertainties I have with this proposal:
1. This changes almost the entire FBTFT stack. Would it be better to
just move directly to DRM? DRM is the preferred way to write graphics
drivers now, but fbdev is really a perfect match for FBTFT.
And DRM doesn't have deferred io support AFAICT.
2. The change from controller centric drivers to display drivers is
that correct?
This will deprecate all of the current controller drivers.

So any help in pinning down the path for moving FBTFT forward is appreciated.


[1]: MIPI Alliance Display Interface Specifications:
  MIPI DCS - Display Command Set
  MIPI DBI - Display Bus Interface
  MIPI DPI - Display Pixel Interface
  http://mipi.org/specifications/display-interface
[2]: http://article.gmane.org/gmane.linux.drivers.devicetree/109103



Hi,

I didn't get any comments to my questions in this cover letter,
so I'm bumping it.
The fbdev mailinglist is included this time, as I'm hoping to receive
comments about a future move of fbtft into fbdev.

I have also started writing code for drm/kms integration
of this patchset, too see what that look's like.


Regards,
Noralf Trønnes

_

Re: [PATCH] Staging: fbtft: move externs to header file

[Noralf Trønnes]

Den 03.03.2015 11:19, skrev Drew Fustini:

Move extern declartions from fbtft-core.c to fbtft header file. This resovles
the warning from checkpatch.pl that "externs should be avoided in .c files".

Signed-off-by: Drew Fustini 
---
  drivers/staging/fbtft/fbtft-core.c | 5 -
  drivers/staging/fbtft/fbtft.h  | 7 +++
  2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index ac4287f..3422faf 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -42,11 +42,6 @@
  
  #include "fbtft.h"
  
-extern void fbtft_sysfs_init(struct fbtft_par *par);

-extern void fbtft_sysfs_exit(struct fbtft_par *par);
-extern void fbtft_expand_debug_value(unsigned long *debug);
-extern int fbtft_gamma_parse_str(struct fbtft_par *par, unsigned long *curves,
-   const char *str, int size);
  
  static unsigned long debug;

  module_param(debug, ulong, 0);
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 0dbf3f9..9e729e5 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -277,6 +277,13 @@ extern int fbtft_init_display(struct fbtft_par *par);
  extern int fbtft_probe_common(struct fbtft_display *display,
struct spi_device *sdev, struct platform_device *pdev);
  extern int fbtft_remove_common(struct device *dev, struct fb_info *info);
+extern void fbtft_sysfs_init(struct fbtft_par *par);
+extern void fbtft_sysfs_exit(struct fbtft_par *par);
+extern void fbtft_expand_debug_value(unsigned long *debug);
+extern int fbtft_gamma_parse_str(struct fbtft_par *par, unsigned long *curves,
+   const char *str, int size);
+
+
  
  /* fbtft-io.c */

  extern int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len);


These functions are defined in fbtft-sysfs.c and internal to the fbtft 
module.

I think it's better to put them in an 'internal.h' file.
The fbtft.h file will eventually live in include/linux/fbtft.h or 
something like that.



Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC 3/7] staging: fbtft: add lcd controller abstraction

[Noralf Trønnes]

Den 03.03.2015 13:04, skrev Dan Carpenter:

On Tue, Mar 03, 2015 at 12:57:29PM +0100, Noralf Trønnes wrote:

+   ret = ctrl->rotate(ctrl, rotation);
+   if (!ret)
+   ctrl->rotation = rotation;
+
+   return ret;

Better to check "if (ret)" consistently (error handling vs success
handling).

Like this?

ret = ctrl->rotate(ctrl, rotation);
if (ret)
return ret;

ctrl->rotation = rotation;

return 0;


Yeah.  This is a tiny nit.  I feel a bit silly for commenting on it now.


No need to be. If this enhances readability even just a tiny bit, I 
welcome it.
I've read enough kernel code now to be amazed of how readable and 
understandable it is.
This becomes very evident when reading some out-of-tree drivers, and 
*cough* fbtft.
I'm practically blind to the code in this patchset, as it has gone 
through several iterations
over the past months. To me it looks very good, but obviously not 
without room for

improvements :-)


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC 1/7] staging: fbtft: add lcd register abstraction

[Noralf Trønnes]

Den 02.03.2015 13:25, skrev Dan Carpenter:

On Mon, Mar 02, 2015 at 11:54:23AM +0100, Noralf Trønnes wrote:

diff --git a/drivers/staging/fbtft/lcdreg/lcdreg-debugfs.c 
b/drivers/staging/fbtft/lcdreg/lcdreg-debugfs.c

[snip]

+static int lcdreg_userbuf_to_u32(const char __user *user_buf, size_t count,
+u32 *dest, size_t dest_size)
+{
+   char *buf, *start;
+   unsigned long value;
+   int ret = 0;
+   int i;
+
+   buf = kmalloc(count, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+
+   if (copy_from_user(buf, user_buf, count)) {
+   kfree(buf);
+   return -EFAULT;
+   }
+   buf[count] = 0;

Off-by-one overflow.

Yes

+   for (i = 0; i < count; i++)
+   if (buf[i] == ' ' || buf[i] == '\n')
+   buf[i] = 0;
+   i = 0;
+   start = buf;
+   while (start < buf + count) {
+   if (*start == 0) {

Use '\0' instead of 0.

if (*start == '\0')

Ok



+   start++;
+   continue;
+   }
+   if (i == dest_size) {
+   ret = -EFBIG;
+   break;
+   }
+   if (kstrtoul(start, 16, &value)) {
+   ret = -EINVAL;
+   break;
+   }

Consider changing this to:

ret = kstrtou32(start, 16, value);
if (ret)
break;


+   dest[i++] = value;
+   while (*start != 0)
+   start++;

This while loop is not needed because of the if statement earlier.

I missed that in my many iterations of this debugfs code.




+   };
+   kfree(buf);
+   if (ret)
+   return ret;
+
+   return i ? i : -EINVAL;
+}
+
+static ssize_t lcdreg_debugfs_write_file(struct file *file,
+const char __user *user_buf,
+size_t count, loff_t *ppos)
+{
+   struct lcdreg *reg = file->private_data;
+   int ret;
+   u32 txbuf[128];
+
+   ret = lcdreg_userbuf_to_u32(user_buf, count, txbuf, ARRAY_SIZE(txbuf));
+   if (ret <= 0)
+   return -EINVAL;

This function can't return zero so preserve the error code.

if (ret < 0)
return ret;

Ok
[snip]

+static ssize_t lcdreg_debugfs_read_wr(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+   struct lcdreg *reg = file->private_data;
+   struct lcdreg_transfer tr = {
+   .index = (reg->quirks & LCDREG_INDEX0_ON_READ) ? 0 : 1,
+   .width = reg->debugfs_read_width,
+   .count = 1,
+   };
+   u32 txbuf[1];
+   char *buf = NULL;
+   int ret;
+
+   ret = lcdreg_userbuf_to_u32(user_buf, count, txbuf, ARRAY_SIZE(txbuf));
+   if (ret != 1)
+   return -EINVAL;
+
+   tr.buf = kmalloc(lcdreg_bytes_per_word(tr.width), GFP_KERNEL);
+   if (!tr.buf)
+   return -ENOMEM;
+
+   add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+
+   lcdreg_lock(reg);
+   ret = lcdreg_read(reg, txbuf[0], &tr);
+   lcdreg_unlock(reg);
+   if (ret)
+   goto error_out;
+
+   if (!reg->debugfs_read_result) {
+   reg->debugfs_read_result = kmalloc(16, GFP_KERNEL);
+   if (!reg->debugfs_read_result) {
+   ret = -ENOMEM;
+   goto error_out;
+   }
+   }

Allocating here is strange.  We only free ->debugfs_read_result on
error so it's sort of buggy as well.  Also is it racy if we have
multiple readers and writers at once?

I spent some time on figuring out how to do this.
To read a register, first write the register number to the 'read' file.
Then get the result by reading the 'read' file.
So I have to keep the result somewhere, but you're right this leaks
memory. Maybe I can use devm_kmalloc here.
And yes this is racy. I'm not sure if this is a problem.
This is to be used during development or debugging, so
the user should have this under control.

But I'm open to other ways of doing this. This is the best I could
come up with after several iterations on this code.
My other solutions where worse :-)


+
+   buf = reg->debugfs_read_result;
+
+   switch (tr.width) {
+   case 8:
+   snprintf(buf, 16, "%02x\n", *(u8 *)tr.buf);
+   break;
+   case 16:
+   snprintf(buf, 16, "%04x\n", *(u16 *)tr.buf);
+   break;
+   case 24:
+   case 32:
+   snprintf(buf, 16, "%08x\n", *(u32 *)tr.buf);
+   break;
+

Re: [RFC 3/7] staging: fbtft: add lcd controller abstraction

[Noralf Trønnes]

Den 02.03.2015 12:48, skrev Dan Carpenter:

[snip]


+   if (ctrl->power_supply) {
+   ret = regulator_enable(ctrl->power_supply);
+   if (ret) {
+   dev_err(ctrl->lcdreg->dev,
+   "failed to enable power supply: %d\n", ret);
+   goto enable_failed;

This kind of label naming where the label name is based on the function
which failed is a common anti-pattern.


+   }
+   }
+   if (ctrl->poweron) {
+   ret = ctrl->poweron(ctrl);
+   if (ret)
+   goto enable_failed;

It means that the other gotos don't make any sense.  It's better to
pick the label name based on the label location like err_unlock,
out_unlock.

Yes, I see that.

+/**
+ * Caller is responsible for locking.
+ */
+int _lcdctrl_rotate(struct lcdctrl *ctrl, u32 rotation)

Why the underscore?  I assume it's because of the locking.  Just
documentating it is sufficient, no need for the underscore.
Ok, I thought this was a common pattern based on other functions I have 
seen.



+{
+   int ret;
+
+   if (!ctrl->rotate)
+   return -ENOSYS;
+
+   switch (rotation) {
+   case 0:
+   case 90:
+   case 180:
+   case 270:
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   ret = ctrl->rotate(ctrl, rotation);
+   if (!ret)
+   ctrl->rotation = rotation;
+
+   return ret;

Better to check "if (ret)" consistently (error handling vs success
handling).

Like this?

ret = ctrl->rotate(ctrl, rotation);
if (ret)
return ret;

ctrl->rotation = rotation;

return 0;



+/**
+ * Description of a display update
+ *
+ * @base: Base address of video memory
+ * @ys: Horizontal line to start the transfer from (zero based)
+ * @ye: Last line to transfer (inclusive)
+ */
+struct lcdctrl_update {
+   void *base;
+   u32 ys;
+   u32 ye;

"ys" and "ye" are easy to mix up when you're reading the code.  They
are not especially self documenting.  Maybe use y_start and y_end.


Yes, they're kind on convoluted. Not suited for fast reading.


Thanks,
Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [RFC 5/7] staging: fbtft: don't require platform data

[Noralf Trønnes]

Den 02.03.2015 12:31, skrev Dan Carpenter:

On Mon, Mar 02, 2015 at 11:54:27AM +0100, Noralf Trønnes wrote:

Add dummy platform data when it's not present.

Signed-off-by: Noralf Trønnes 
---
  drivers/staging/fbtft/fbtft-core.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index ac4287f..59c17c1 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -719,10 +719,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct 
fbtft_display *display,
if (!bpp)
bpp = 16;
  
-	if (!pdata) {

-   dev_err(dev, "platform data is missing\n");
-   return NULL;
-   }
+   if (!pdata)
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);

This is weird.  pdata is zeroed out so the info is a bit useless.  We
don't use it outside this function.  Later in the function, then should
we do?

-   par->pdata = dev->platform_data;
+   par->pdata = pdata;


You're right. I missed that.

thanks,
Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Staging: fbtft: fix space errors

[Noralf Trønnes]

Den 02.03.2015 20:21, skrev Andrey Skvortsov:

On 02 Mar, Joe Perches wrote:

On Mon, 2015-03-02 at 12:37 +0300, Dan Carpenter wrote:

On Sat, Feb 28, 2015 at 06:59:19AM -0800, Joe Perches wrote:

If you're really going to change these, please
remove the unnecessary \ line continuations
indent the blocks properly and group the blocks
more intelligibly.  Maybe something like;

static const int st7735r_init[] = {
-1, 0x01,

[]

-2, 10,
-3
};

What's the logic here?  Why are we putting the negatives first?

Those are delimiters.  see fbtft-core.c:fbtft_init_display().

As far as I understand:

-1, start of block
-2, millisecond delay after block write
-3, end of blocks

Beyond that, I don't much care.
I just prefer intelligible over apparently random.

If it is correct, then it would be better to replace these magic numbers with 
meaningful
defines.



I agree with that in principal, but I'm not convinced that there is any
point in cleaning up this. It should be reworked in another way.
One problem with these drivers is that they are controller centric.
They have a default init sequence matching some, but not all display panels.
The result of this is that in the Device Tree case, the (ugly) init sequence
is passed in through a DT property string. This will never be accepted by
the DT guys. Register init seq. in any form in DT seem to be generally not
accepted. So I guess the drivers have to be display centric instead.
Each having hardcoded the register init sequence with necessary delays.

I have made an attempt to address this in a RFC I posted earlier today:

staging: fbtft: minimize coupling to the fbdev subsystem
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-March/065985.html


Noralf.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC 1/7] staging: fbtft: add lcd register abstraction

[Noralf Trønnes]

Add LCD register abstraction for MIPI DCS like controllers. This
hides LCD controller interface implementation details.
When built with debugfs, the register is available to userspace.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig |   2 +
 drivers/staging/fbtft/Makefile|   6 +-
 drivers/staging/fbtft/lcdreg/Kconfig  |   4 +
 drivers/staging/fbtft/lcdreg/Makefile |   3 +
 drivers/staging/fbtft/lcdreg/internal.h   |   9 +
 drivers/staging/fbtft/lcdreg/lcdreg-core.c| 187 ++
 drivers/staging/fbtft/lcdreg/lcdreg-debugfs.c | 272 ++
 drivers/staging/fbtft/lcdreg/lcdreg.h | 151 ++
 8 files changed, 632 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/fbtft/lcdreg/Kconfig
 create mode 100644 drivers/staging/fbtft/lcdreg/Makefile
 create mode 100644 drivers/staging/fbtft/lcdreg/internal.h
 create mode 100644 drivers/staging/fbtft/lcdreg/lcdreg-core.c
 create mode 100644 drivers/staging/fbtft/lcdreg/lcdreg-debugfs.c
 create mode 100644 drivers/staging/fbtft/lcdreg/lcdreg.h

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 995a910..2d1490f 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -167,3 +167,5 @@ config FB_FLEX
 config FB_TFT_FBTFT_DEVICE
tristate "Module to for adding FBTFT devices"
depends on FB_TFT
+
+source "drivers/staging/fbtft/lcdreg/Kconfig"
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index e773f0f..2769421 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -1,8 +1,10 @@
-# Core module
+# Core modules
 obj-$(CONFIG_FB_TFT) += fbtft.o
 fbtft-y  += fbtft-core.o fbtft-sysfs.o fbtft-bus.o 
fbtft-io.o
 
-# drivers
+obj-$(CONFIG_LCDREG) += lcdreg/
+
+# Controller drivers
 obj-$(CONFIG_FB_TFT_AGM1264K_FL) += fb_agm1264k-fl.o
 obj-$(CONFIG_FB_TFT_BD663474)+= fb_bd663474.o
 obj-$(CONFIG_FB_TFT_HX8340BN)+= fb_hx8340bn.o
diff --git a/drivers/staging/fbtft/lcdreg/Kconfig 
b/drivers/staging/fbtft/lcdreg/Kconfig
new file mode 100644
index 000..ec0c097
--- /dev/null
+++ b/drivers/staging/fbtft/lcdreg/Kconfig
@@ -0,0 +1,4 @@
+config LCDREG
+   tristate
+   depends on GPIOLIB
+   default n
diff --git a/drivers/staging/fbtft/lcdreg/Makefile 
b/drivers/staging/fbtft/lcdreg/Makefile
new file mode 100644
index 000..c9ea774
--- /dev/null
+++ b/drivers/staging/fbtft/lcdreg/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_LCDREG)   += lcdreg.o
+lcdreg-y   += lcdreg-core.o
+lcdreg-$(CONFIG_DEBUG_FS)  += lcdreg-debugfs.o
diff --git a/drivers/staging/fbtft/lcdreg/internal.h 
b/drivers/staging/fbtft/lcdreg/internal.h
new file mode 100644
index 000..04035b9
--- /dev/null
+++ b/drivers/staging/fbtft/lcdreg/internal.h
@@ -0,0 +1,9 @@
+#include "lcdreg.h"
+
+#ifdef CONFIG_DEBUG_FS
+extern void lcdreg_debugfs_init(struct lcdreg *reg);
+extern void lcdreg_debugfs_exit(struct lcdreg *reg);
+#else
+static inline void lcdreg_debugfs_init(struct lcdreg *reg) { }
+static inline void lcdreg_debugfs_exit(struct lcdreg *reg) { }
+#endif
diff --git a/drivers/staging/fbtft/lcdreg/lcdreg-core.c 
b/drivers/staging/fbtft/lcdreg/lcdreg-core.c
new file mode 100644
index 000..4a1131c
--- /dev/null
+++ b/drivers/staging/fbtft/lcdreg/lcdreg-core.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2015 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "internal.h"
+#include "lcdreg.h"
+
+/**
+ * Write to LCD register
+ *
+ * @reg: LCD register
+ * @regnr: Register number
+ * @transfer: Transfer to write
+ */
+int lcdreg_write(struct lcdreg *reg, unsigned regnr,
+struct lcdreg_transfer *transfer)
+{
+   if (!transfer)
+   return -EINVAL;
+
+   if (!transfer->width)
+   transfer->width = reg->def_width;
+
+   dev_dbg(reg->dev,
+   "lcdreg_write: regnr=0x%02x, index=%u, count=%u, width=%u\n",
+   regnr, transfer->index, transfer->count, transfer->width);
+   lcdreg_dbg_transfer_buf(transfer);
+
+   return reg->write(reg, regnr, transfer);
+}
+EXPORT_SYMBOL(lcdreg_write);
+
+/**
+ * Write 32-bit wide buffer to LCD register
+ * @reg: lcdreg
+ * @regnr: Register number
+ * @buf: Buffer to write
+ * @count: Number of words to write
+ */
+int lcdreg_write_buf32(struct lcdreg *reg, unsigned regnr, const u32 *buf,
+  unsigned count)
+{
+   struct lcdreg_transfer tr = {
+  

[RFC 3/7] staging: fbtft: add lcd controller abstraction

[Noralf Trønnes]

Add abstraction for MIPI DCS/DBI like LCD controllers.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig   |   1 +
 drivers/staging/fbtft/Makefile  |   1 +
 drivers/staging/fbtft/lcdctrl/Kconfig   |   3 +
 drivers/staging/fbtft/lcdctrl/Makefile  |   1 +
 drivers/staging/fbtft/lcdctrl/lcdctrl.c | 207 
 drivers/staging/fbtft/lcdctrl/lcdctrl.h | 104 
 6 files changed, 317 insertions(+)
 create mode 100644 drivers/staging/fbtft/lcdctrl/Kconfig
 create mode 100644 drivers/staging/fbtft/lcdctrl/Makefile
 create mode 100644 drivers/staging/fbtft/lcdctrl/lcdctrl.c
 create mode 100644 drivers/staging/fbtft/lcdctrl/lcdctrl.h

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 2d1490f..0ac1b41 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -168,4 +168,5 @@ config FB_TFT_FBTFT_DEVICE
tristate "Module to for adding FBTFT devices"
depends on FB_TFT
 
+source "drivers/staging/fbtft/lcdctrl/Kconfig"
 source "drivers/staging/fbtft/lcdreg/Kconfig"
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 2769421..69f7c2c 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_FB_TFT) += fbtft.o
 fbtft-y  += fbtft-core.o fbtft-sysfs.o fbtft-bus.o 
fbtft-io.o
 
 obj-$(CONFIG_LCDREG) += lcdreg/
+obj-$(CONFIG_LCDCTRL)+= lcdctrl/
 
 # Controller drivers
 obj-$(CONFIG_FB_TFT_AGM1264K_FL) += fb_agm1264k-fl.o
diff --git a/drivers/staging/fbtft/lcdctrl/Kconfig 
b/drivers/staging/fbtft/lcdctrl/Kconfig
new file mode 100644
index 000..5272847
--- /dev/null
+++ b/drivers/staging/fbtft/lcdctrl/Kconfig
@@ -0,0 +1,3 @@
+config LCDCTRL
+   tristate
+   default n
diff --git a/drivers/staging/fbtft/lcdctrl/Makefile 
b/drivers/staging/fbtft/lcdctrl/Makefile
new file mode 100644
index 000..e6e4e8c
--- /dev/null
+++ b/drivers/staging/fbtft/lcdctrl/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_LCDCTRL)  += lcdctrl.o
diff --git a/drivers/staging/fbtft/lcdctrl/lcdctrl.c 
b/drivers/staging/fbtft/lcdctrl/lcdctrl.c
new file mode 100644
index 000..028fc6b
--- /dev/null
+++ b/drivers/staging/fbtft/lcdctrl/lcdctrl.c
@@ -0,0 +1,207 @@
+/*
+ * Copyright (C) 2015 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+
+#include "lcdctrl.h"
+
+void lcdctrl_of_get_format(struct lcdctrl *ctrl)
+{
+   struct device *dev = ctrl->lcdreg->dev;
+   const char *fmt_str;
+   int ret;
+
+   if (!ctrl->set_format)
+   return;
+
+   ret = of_property_read_string(dev->of_node, "format", &fmt_str);
+   if (ret)
+   return;
+
+   if (!strcmp(fmt_str, "monochrome"))
+   ctrl->format = LCDCTRL_FORMAT_MONO10;
+   else if (!strcmp(fmt_str, "rgb565"))
+   ctrl->format = LCDCTRL_FORMAT_RGB565;
+   else if (!strcmp(fmt_str, "rgb888"))
+   ctrl->format = LCDCTRL_FORMAT_RGB888;
+   else if (!strcmp(fmt_str, "xrgb"))
+   ctrl->format = LCDCTRL_FORMAT_XRGB;
+   else
+   dev_err(dev, "Invalid format: %s. Using default.\n", fmt_str);
+}
+EXPORT_SYMBOL(lcdctrl_of_get_format);
+
+void lcdctrl_of_get_rotation(struct lcdctrl *ctrl)
+{
+   struct device *dev = ctrl->lcdreg->dev;
+   u32 val;
+   int ret;
+
+   if (!ctrl->rotate)
+   return;
+
+   ret = of_property_read_u32(dev->of_node, "rotation", &val);
+   if (ret) {
+   if (ret != -EINVAL)
+   dev_err(dev, "error reading property 'rotation': %i\n",
+   ret);
+   return;
+   }
+
+   switch (val) {
+   case 0:
+   case 90:
+   case 180:
+   case 270:
+   ctrl->rotation = val;
+   break;
+   default:
+   dev_err(dev, "illegal rotation value: %u\n", val);
+   break;
+   }
+}
+EXPORT_SYMBOL(lcdctrl_of_get_rotation);
+
+int lcdctrl_enable(struct lcdctrl *ctrl, void *videomem)
+{
+   struct lcdctrl_update update = {
+   .base = videomem,
+   .ys = 0,
+   .ye = lcdctrl_yres(ctrl) - 1,
+   };
+   int ret;
+
+   if (WARN_ON_ONCE(!ctrl->update))
+   return -EINVAL;
+
+   if (ctrl->initialized)
+   return 0;
+
+   lcdreg_lock(ctrl->lcdreg);
+
+   if (ctrl->power_supp

[RFC 7/7] staging: fbtft: add driver for Adafruit ssd1306 based displays

[Noralf Trønnes]

The Adafuit SSD1306 based displays comes in either 128x32 or 128x64
resolutions and uses I2C or SPI interface. This driver has support
for the I2C interface. Support for SPI can be added when lcdreg has
SPI support.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Kconfig |   9 ++
 drivers/staging/fbtft/Makefile|   3 +
 drivers/staging/fbtft/ada-ssd1306fb.c | 173 ++
 3 files changed, 185 insertions(+)
 create mode 100644 drivers/staging/fbtft/ada-ssd1306fb.c

diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 0ac1b41..b53ed5b 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -168,5 +168,14 @@ config FB_TFT_FBTFT_DEVICE
tristate "Module to for adding FBTFT devices"
depends on FB_TFT
 
+config FB_ADA_SSD1306
+   tristate "Framebuffer driver for Adafruit SSD1306 displays"
+   depends on FB_TFT && I2C
+   select LCDREG_I2C
+   select LCDCTRL_SSD1306
+   help
+ Framebuffer Driver for Adafruit SSD1306 based
+ Monochrome OLED displays
+
 source "drivers/staging/fbtft/lcdctrl/Kconfig"
 source "drivers/staging/fbtft/lcdreg/Kconfig"
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 6f002a6..f862810 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -36,3 +36,6 @@ obj-$(CONFIG_FB_FLEX)+= flexfb.o
 
 # Device modules
 obj-$(CONFIG_FB_TFT_FBTFT_DEVICE) += fbtft_device.o
+
+# Display drivers
+obj-$(CONFIG_FB_ADA_SSD1306) += ada-ssd1306fb.o
diff --git a/drivers/staging/fbtft/ada-ssd1306fb.c 
b/drivers/staging/fbtft/ada-ssd1306fb.c
new file mode 100644
index 000..eea5978
--- /dev/null
+++ b/drivers/staging/fbtft/ada-ssd1306fb.c
@@ -0,0 +1,173 @@
+/*
+ * Framebuffer Driver for Adafruit SSD1306 based Monochrome OLED displays
+ *
+ * Copyright 2015 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "fbtft.h"
+#include "lcdreg/lcdreg.h"
+#include "lcdctrl/ssd1306.h"
+
+
+/* Init sequence from github.com/adafruit/Adafruit_SSD1306 */
+static int ada_ssd1306_poweron(struct lcdctrl *ctrl)
+{
+   struct lcdreg *reg = ctrl->lcdreg;
+   int ret;
+
+   lcdreg_reset(reg);
+
+   ret = lcdreg_writereg(reg, SSD1306_DISPLAY_OFF);
+   if (ret) {
+   dev_err(reg->dev,
+   "lcdreg_writereg failed: %d\n", ret);
+   return ret;
+   }
+
+   if (lcdreg_is_readable(reg))
+   ssd1306_check_status(reg, false);
+
+   lcdreg_writereg(reg, SSD1306_CLOCK_FREQ);
+   lcdreg_writereg(reg, 0x80);
+
+   lcdreg_writereg(reg, SSD1306_MULTIPLEX_RATIO);
+   if (lcdctrl_yres(ctrl) == 64)
+   lcdreg_writereg(reg, 0x3f);
+   else
+   lcdreg_writereg(reg, 0x1f);
+
+   lcdreg_writereg(reg, SSD1306_DISPLAY_OFFSET);
+   lcdreg_writereg(reg, 0x0);
+
+   lcdreg_writereg(reg, SSD1306_DISPLAY_START_LINE);
+
+   lcdreg_writereg(reg, SSD1306_CHARGE_PUMP);
+   lcdreg_writereg(reg, 0x14);
+
+   lcdreg_writereg(reg, SSD1306_ADDRESS_MODE);
+   lcdreg_writereg(reg, 0x01); /* vertical */
+
+   lcdreg_writereg(reg, SSD1306_COL_RANGE);
+   lcdreg_writereg(reg, 0x00);
+   lcdreg_writereg(reg, lcdctrl_xres(ctrl) - 1);
+
+   lcdreg_writereg(reg, SSD1306_PAGE_RANGE);
+   lcdreg_writereg(reg, 0x00);
+   lcdreg_writereg(reg, (lcdctrl_yres(ctrl) / 8) - 1);
+
+   lcdreg_writereg(reg, SSD1306_SEG_REMAP_ON);
+
+   lcdreg_writereg(reg, SSD1306_COM_SCAN_REMAP);
+
+   lcdreg_writereg(reg, SSD1306_COM_PINS_CONFIG);
+   if (lcdctrl_yres(ctrl) == 64)
+   lcdreg_writereg(reg, 0x12);
+   else
+   lcdreg_writereg(reg, 0x02);
+
+   lcdreg_writereg(reg, SSD1306_PRECHARGE_PERIOD);
+   lcdreg_writereg(reg, 0xf1);
+
+   lcdreg_writereg(reg, SSD1306_VCOMH);
+   lcdreg_writereg(reg, 0x40);
+
+   lcdreg_writereg(reg, SSD1306_RESUME_TO_RAM);
+
+   lcdreg_writereg(reg, SSD1306_NORMAL_DISPLAY);
+
+   lcdreg_writereg(reg, SSD1306_CONTRAST);
+   if (lcdctrl_yres(ctrl) == 64)
+   lcdreg_writereg(reg, 0xcf);
+   else
+   lcdreg_writereg(reg, 0x8f);
+
+   /* The display is turned on later by core */
+
+   return 0;
+}
+
+static const struct of_device_id ada_ssd1306_ids[] = {
+   { .compatible = "ada,ssd1306-128x32", .data = (void *)32 },
+   { .compatible = "ada,ssd1306-128x64", .data = (void *)64 },
+   {},
+};
+MODULE_DEVICE_TABLE(of, ada_ssd1306_ids);
+
+s

[RFC 6/7] staging: fbtft: extend core to use lcdctrl

[Noralf Trønnes]

Add lcdctrl support in core fbtft module.
Provide new API for drivers to use.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/Makefile|   1 +
 drivers/staging/fbtft/fbtft-lcdctrl.c | 311 ++
 drivers/staging/fbtft/fbtft.h |  13 ++
 3 files changed, 325 insertions(+)
 create mode 100644 drivers/staging/fbtft/fbtft-lcdctrl.c

diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 69f7c2c..6f002a6 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -1,6 +1,7 @@
 # Core modules
 obj-$(CONFIG_FB_TFT) += fbtft.o
 fbtft-y  += fbtft-core.o fbtft-sysfs.o fbtft-bus.o 
fbtft-io.o
+fbtft-$(if $(CONFIG_LCDCTRL),y,) += fbtft-lcdctrl.o
 
 obj-$(CONFIG_LCDREG) += lcdreg/
 obj-$(CONFIG_LCDCTRL)+= lcdctrl/
diff --git a/drivers/staging/fbtft/fbtft-lcdctrl.c 
b/drivers/staging/fbtft/fbtft-lcdctrl.c
new file mode 100644
index 000..b4e6011
--- /dev/null
+++ b/drivers/staging/fbtft/fbtft-lcdctrl.c
@@ -0,0 +1,311 @@
+#include 
+#include 
+
+#include "fbtft.h"
+
+extern void fbtft_sysfs_init(struct fbtft_par *par);
+
+static int fbtft_lcdctrl_blank(struct fbtft_par *par, bool on)
+{
+   struct lcdctrl *ctrl = par->lcdctrl;
+
+   if (ctrl->blank)
+   return lcdctrl_blank(ctrl, on);
+   else if (par->info->bl_dev)
+   return 0; /* backlight subsystem handles blanking */
+   else
+   return -EINVAL; /* no blanking support */
+}
+
+static void fbtft_lcdctrl_update_display(struct fbtft_par *par,
+   unsigned start_line, unsigned end_line)
+{
+   struct lcdctrl *ctrl = par->lcdctrl;
+   struct lcdctrl_update update = {
+   .base = (void __force *)par->info->screen_base,
+   .ys = start_line,
+   .ye = end_line,
+   };
+   int ret;
+
+   ret = lcdctrl_update(ctrl, &update);
+   if (ret)
+   pr_err_once("%s %s: lcdctrl_update failed: %i\n",
+   dev_driver_string(ctrl->lcdreg->dev),
+   dev_name(ctrl->lcdreg->dev), ret);
+}
+
+static int fbtft_fb_check_var(struct fb_var_screeninfo *var, struct fb_info 
*info)
+{
+   u32 rotate = var->rotate;
+
+   *var = info->var;
+   var->rotate = rotate;
+
+   switch (var->rotate) {
+   case 0:
+   case 90:
+   case 180:
+   case 270:
+   return 0;
+   default:
+   return -EINVAL;
+   }
+}
+
+static int fbtft_fb_set_par(struct fb_info *info)
+{
+   struct fbtft_par *par = info->par;
+   struct lcdctrl *ctrl = par->lcdctrl;
+   int ret;
+
+   lcdreg_lock(ctrl->lcdreg);
+
+   ret = _lcdctrl_rotate(ctrl, info->var.rotate);
+   if (ret) {
+   info->var.rotate = ctrl->rotation;
+   goto rotate_failed;
+   }
+
+   info->var.xres = lcdctrl_xres(ctrl);
+   info->var.yres = lcdctrl_yres(ctrl);
+   info->var.xres_virtual = info->var.xres;
+   info->var.yres_virtual = info->var.yres;
+
+   switch (info->var.bits_per_pixel) {
+   case 1:
+   info->fix.line_length = info->var.xres / 8;
+   break;
+   case 8:
+   info->fix.line_length = info->var.xres;
+   break;
+   case 16:
+   info->fix.line_length = info->var.xres * 2;
+   break;
+   case 24:
+   info->fix.line_length = info->var.xres * 3;
+   break;
+   case 32:
+   info->fix.line_length = info->var.xres * 4;
+   break;
+   }
+
+rotate_failed:
+   lcdreg_unlock(ctrl->lcdreg);
+
+   return ret;
+}
+
+static struct fb_info *fbtft_lcdctrl_register(struct lcdctrl *ctrl)
+{
+   struct device *dev = ctrl->lcdreg->dev;
+   struct fb_info *info;
+   struct fbtft_par *par;
+   int ret;
+   struct fbtft_display display = {
+   .width = lcdctrl_xres(ctrl),
+   .height = lcdctrl_yres(ctrl),
+   .fps = 20,
+   .txbuflen = -2, /* don't allocate txbuf */
+   };
+
+   switch (ctrl->format) {
+   case LCDCTRL_FORMAT_MONO10:
+   display.bpp = 1;
+   break;
+   case LCDCTRL_FORMAT_RGB565:
+   display.bpp = 16;
+   break;
+   case LCDCTRL_FORMAT_RGB888:
+   display.bpp = 24;
+   break;
+   case LCDCTRL_FORMAT_XRGB:
+   display.bpp = 32;
+   break;
+   default:
+   return ERR_PTR(-EINVAL);
+   }
+
+   info = fbtft_framebuffer_alloc(&display, dev);
+   if (!info)
+   return ERR_PTR(-ENOMEM);
+
+   switch (ctrl->format) {
+

[RFC 5/7] staging: fbtft: don't require platform data

[Noralf Trønnes]

Add dummy platform data when it's not present.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/fbtft-core.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-core.c 
b/drivers/staging/fbtft/fbtft-core.c
index ac4287f..59c17c1 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -719,10 +719,8 @@ struct fb_info *fbtft_framebuffer_alloc(struct 
fbtft_display *display,
if (!bpp)
bpp = 16;
 
-   if (!pdata) {
-   dev_err(dev, "platform data is missing\n");
-   return NULL;
-   }
+   if (!pdata)
+   pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 
/* override driver values? */
if (pdata->fps)
-- 
2.2.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[RFC 2/7] staging: fbtft: lcdreg: add i2c support

[Noralf Trønnes]

Add I2C bus support to lcdreg.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/lcdreg/Kconfig  |   6 ++
 drivers/staging/fbtft/lcdreg/Makefile |   2 +
 drivers/staging/fbtft/lcdreg/lcdreg-i2c.c | 129 ++
 drivers/staging/fbtft/lcdreg/lcdreg.h |   3 +
 4 files changed, 140 insertions(+)
 create mode 100644 drivers/staging/fbtft/lcdreg/lcdreg-i2c.c

diff --git a/drivers/staging/fbtft/lcdreg/Kconfig 
b/drivers/staging/fbtft/lcdreg/Kconfig
index ec0c097..8f000b5 100644
--- a/drivers/staging/fbtft/lcdreg/Kconfig
+++ b/drivers/staging/fbtft/lcdreg/Kconfig
@@ -2,3 +2,9 @@ config LCDREG
tristate
depends on GPIOLIB
default n
+
+config LCDREG_I2C
+   tristate
+   depends on I2C
+   select LCDREG
+   default n
diff --git a/drivers/staging/fbtft/lcdreg/Makefile 
b/drivers/staging/fbtft/lcdreg/Makefile
index c9ea774..1ec65af 100644
--- a/drivers/staging/fbtft/lcdreg/Makefile
+++ b/drivers/staging/fbtft/lcdreg/Makefile
@@ -1,3 +1,5 @@
 obj-$(CONFIG_LCDREG)   += lcdreg.o
 lcdreg-y   += lcdreg-core.o
 lcdreg-$(CONFIG_DEBUG_FS)  += lcdreg-debugfs.o
+
+obj-$(CONFIG_LCDREG_I2C)   += lcdreg-i2c.o
diff --git a/drivers/staging/fbtft/lcdreg/lcdreg-i2c.c 
b/drivers/staging/fbtft/lcdreg/lcdreg-i2c.c
new file mode 100644
index 000..297926f
--- /dev/null
+++ b/drivers/staging/fbtft/lcdreg/lcdreg-i2c.c
@@ -0,0 +1,129 @@
+/*
+ * lcdreg I2C support
+ *
+ * Copyright 2015 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lcdreg.h"
+
+struct lcdreg_i2c {
+   struct lcdreg reg;
+   struct i2c_client *client;
+   struct gpio_desc *reset;
+};
+
+static inline struct lcdreg_i2c *to_lcdreg_i2c(struct lcdreg *reg)
+{
+   return reg ? container_of(reg, struct lcdreg_i2c, reg) : NULL;
+}
+
+static int lcdreg_i2c_send(struct i2c_client *client, unsigned index,
+  void *buf, size_t len)
+{
+   u8 *txbuf;
+   int ret;
+
+   txbuf = kmalloc(1 + len, GFP_KERNEL);
+   if (!txbuf)
+   return -ENOMEM;
+
+   txbuf[0] = index ? 0x40 : 0x80;
+   memcpy(&txbuf[1], buf, len);
+
+   ret = i2c_master_send(client, txbuf, 1 + len);
+   kfree(txbuf);
+
+   return ret < 0 ? ret : 0;
+}
+
+static int lcdreg_i2c_write(struct lcdreg *reg, unsigned regnr,
+   struct lcdreg_transfer *transfer)
+{
+   struct lcdreg_i2c *i2c = to_lcdreg_i2c(reg);
+   u8 regnr_buf[1] = { regnr };
+   int ret;
+
+   ret = lcdreg_i2c_send(i2c->client, 0, regnr_buf, 1);
+   if (ret)
+   return ret;
+
+   if (!transfer || !transfer->count)
+   return 0;
+
+   if (WARN_ON(transfer->width != 8))
+   return -EINVAL;
+
+   if (!transfer->count)
+   return 0;
+
+   return lcdreg_i2c_send(i2c->client, transfer->index, transfer->buf,
+  transfer->count);
+}
+
+static int lcdreg_i2c_read(struct lcdreg *reg, unsigned regnr,
+  struct lcdreg_transfer *transfer)
+{
+   struct lcdreg_i2c *i2c = to_lcdreg_i2c(reg);
+   int ret;
+
+   if (WARN_ON(regnr != 0 || !transfer || transfer->width != 8))
+   return -EINVAL;
+
+   if (!reg->readable)
+   return -EACCES;
+
+   ret = i2c_master_recv(i2c->client, transfer->buf, transfer->count);
+
+   return ret < 0 ? ret : 0;
+}
+
+static void lcdreg_i2c_reset(struct lcdreg *reg)
+{
+   struct lcdreg_i2c *i2c = to_lcdreg_i2c(reg);
+
+   if (!i2c->reset)
+   return;
+
+   gpiod_set_value_cansleep(i2c->reset, 0);
+   msleep(20);
+   gpiod_set_value_cansleep(i2c->reset, 1);
+   msleep(120);
+}
+
+struct lcdreg *devm_lcdreg_i2c_init(struct i2c_client *client)
+{
+   struct lcdreg_i2c *i2c;
+
+   i2c = devm_kzalloc(&client->dev, sizeof(*i2c), GFP_KERNEL);
+   if (i2c == NULL)
+   return ERR_PTR(-ENOMEM);
+
+   i2c->reg.readable = true;
+   i2c->client = client;
+   i2c->reset = devm_gpiod_get_optional(&client->dev, "reset",
+GPIOD_OUT_HIGH);
+   if (IS_ERR(i2c->reset))
+   return ERR_PTR(PTR_ERR(i2c->reset));
+
+   i2c->reg.write = lcdreg_i2c_write;
+   i2c->reg.read = lcdreg_i2c_read;
+   i2c->reg.reset = lcdreg_i2c_reset;
+
+   return devm_lcdreg_init(&client->dev, &i2c->reg);
+}
+EXPORT_SYMBOL(devm_lcdreg_i2c_init);
+
+MODULE_

[RFC 0/7] staging: fbtft: minimize coupling to the fbdev subsystem

[Noralf Trønnes]

Hi,

This patchset introduces a new API to minimize the coupling to the
fbdev graphics subsystem. There have been calls to deprecate fbdev and
new fbdev drivers are discouraged.
Currently the FBTFT drivers are tightly coupled to fbdev through the
fbtft module and the fbtft_par structure.

What is FBTFT?
FBTFT is a framework for writing drivers for MIPI DCS/DBI [1] like controllers.
These controllers have the following characteristics:
- Onchip graphics RAM that is scanned out to the display panel.
- Registers to set operational parameters and write pixel data
- SPI, I2C and/or parallel interface

To provide better layering and provide opportunity for reuse,
I propose the following layers:

+-+
|  fbdev  |
+-+
|  fbtft  |
+-+---+
|  Display driver |   |
+---+-+   |
|   | lcdctrl |
|   +-+
| lcdreg  |
+-+
|  Interface (SPI, parallel)  |
+-+

lcdreg
--
This layer deals with the various LCD controller interface
implementations (SPI, Intel 8080, I2C). It provides a simple interface
for reading and writing to the LCD controller register.
Especially the SPI interface has several modes and variations that
makes it rather complex.

lcdreg can be reused by drivers that drive these same controllers in
digital RGB interface mode (MIPI DPI). Even when the parallel RGB
interface is used for pixeldata, the controller some times has to be
configured. This is usually done through a SPI interface.

lcdctrl
---
Layer providing an abstraction of the LCD controller and it's
principal funtions:
- Poweron initialization
- Set display rotation
- Set pixel format
- Display blanking
- Update grahics RAM

This decouples the controller implementation from the graphics subsystem,
and prepares for a possible future move to DRM or other grahics subsystems.
The controller specific functionality is kept in library modules.

fbtft
-
The fbtft module gets some helper functions in fbtft-lcdctrl to map
the lcdtrl functions into the fbtft_par structure.
Later when all the drivers have been converted, large parts of the
fbtft module can be removed.


The I2C implementation of lcdreg is used in this proposal, because of
it's simplicity. This is to keep focus on the architectural design.
Implementations for SPI and the Intel 8080 bus interface exists.

Display or Controller driver?
Currently FBTFT has drivers for each controller it supports.
Each driver has a default controller setup which often matches many
displays, but no all (fbtftops.init_display()). A different init
sequence (register values and delays) can be passed in using
platform_data or as a DT property.
I made a post to the DT mailinglist [2] about setting an init sequence
from the Device Tree, but didn't get any replies. Based on an earlier
post and comments I've read, it's looks like init sequence in DT is
not going to happen. That's why this proposal shifts to having display
drivers, with controller specific code in library modules.

Device Tree questions
- What about Device Tree property names that are used by all drivers
  (initialized, format, rotation)? Should they be prefixed in any way?
- Device Tree compatible string. If a driver supports both SPI and I2C,
  can the same compatible string be used for both busses (ada-ssd1306fb)?
  I have seen examples where the bus is added as a postfix (-i2c).
  And which one is preferred: 'ada-ssd1306' or 'ada-ssd1306fb'?

Why RFC?
I did this because of some uncertainties I have with this proposal:
1. This changes almost the entire FBTFT stack. Would it be better to
   just move directly to DRM? DRM is the preferred way to write graphics
   drivers now, but fbdev is really a perfect match for FBTFT.
   And DRM doesn't have deferred io support AFAICT.
2. The change from controller centric drivers to display drivers is
   that correct?
   This will deprecate all of the current controller drivers.

So any help in pinning down the path for moving FBTFT forward is appreciated.


[1]: MIPI Alliance Display Interface Specifications:
 MIPI DCS - Display Command Set
 MIPI DBI - Display Bus Interface
 MIPI DPI - Display Pixel Interface
 http://mipi.org/specifications/display-interface
[2]: http://article.gmane.org/gmane.linux.drivers.devicetree/109103


Regards,
Noralf Trønnes


Noralf Trønnes (7):
  staging: fbtft: add lcd register abstraction
  staging: fbtft: lcdreg: add i2c support
  staging: fbtft: add lcd controller abstraction
  staging: fbtft: lcdctrl: add ssd1306 support
  staging: fbtft: don't require platform data
  staging: fbtft: extend core to use lcdctrl
  staging: fbtft: add driver for Adafruit ssd1306 based displays

 drivers/staging/fbtft/Kconfig |  12 +
 drivers/sta

[RFC 4/7] staging: fbtft: lcdctrl: add ssd1306 support

[Noralf Trønnes]

Add support for Solomon SSD1306 Monochrome OLED controller.
The controller supports up to 128x64 pixles and supports interfaces:
Parallel 6800, parallel 8080, SPI (3/4), I2C.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/lcdctrl/Kconfig   |   5 +
 drivers/staging/fbtft/lcdctrl/Makefile  |   2 +
 drivers/staging/fbtft/lcdctrl/ssd1306.c | 168 
 drivers/staging/fbtft/lcdctrl/ssd1306.h |  51 ++
 4 files changed, 226 insertions(+)
 create mode 100644 drivers/staging/fbtft/lcdctrl/ssd1306.c
 create mode 100644 drivers/staging/fbtft/lcdctrl/ssd1306.h

diff --git a/drivers/staging/fbtft/lcdctrl/Kconfig 
b/drivers/staging/fbtft/lcdctrl/Kconfig
index 5272847..61bfef1 100644
--- a/drivers/staging/fbtft/lcdctrl/Kconfig
+++ b/drivers/staging/fbtft/lcdctrl/Kconfig
@@ -1,3 +1,8 @@
 config LCDCTRL
tristate
default n
+
+config LCDCTRL_SSD1306
+   tristate
+   select LCDCTRL
+   default n
diff --git a/drivers/staging/fbtft/lcdctrl/Makefile 
b/drivers/staging/fbtft/lcdctrl/Makefile
index e6e4e8c..42a61f9 100644
--- a/drivers/staging/fbtft/lcdctrl/Makefile
+++ b/drivers/staging/fbtft/lcdctrl/Makefile
@@ -1 +1,3 @@
 obj-$(CONFIG_LCDCTRL)  += lcdctrl.o
+
+obj-$(CONFIG_LCDCTRL_SSD1306)  += ssd1306.o
diff --git a/drivers/staging/fbtft/lcdctrl/ssd1306.c 
b/drivers/staging/fbtft/lcdctrl/ssd1306.c
new file mode 100644
index 000..251bcc2
--- /dev/null
+++ b/drivers/staging/fbtft/lcdctrl/ssd1306.c
@@ -0,0 +1,168 @@
+/*
+ * SSD1306 LCD controller support
+ *
+ * Copyright 2015 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+
+#include "ssd1306.h"
+
+/*
+ * TODO: contrast control is not implemented
+ *   If gamma control is to be user configurable, maybe the same
+ *   mechanism can be used for contrast.
+ */
+
+struct ssd1306_controller {
+   struct lcdctrl lcdctrl;
+   void *buf;
+};
+
+static inline struct ssd1306_controller *to_ssd1306(struct lcdctrl *ctrl)
+{
+   return container_of(ctrl, struct ssd1306_controller, lcdctrl);
+}
+
+static int ssd1306_update(struct lcdctrl *ctrl, struct lcdctrl_update *update)
+{
+   struct lcdreg *reg = ctrl->lcdreg;
+   u8 *buf = to_ssd1306(ctrl)->buf;
+   u32 xres = lcdctrl_xres(ctrl);
+   u32 yres = lcdctrl_yres(ctrl);
+   int x, y, i;
+   struct lcdreg_transfer tr = {
+   .index = 1,
+   .width = 8,
+   .buf = buf,
+   .count = xres * yres / 8,
+   };
+
+   /*
+* RGB565 is implemented on this monochrome controller for 2 reasons:
+* 1. sys_imageblit and friends doesn't honour
+*CONFIG_FB_CFB_REV_PIXELS_IN_BYTE which results in mirrored or
+*garbeled fonts with fbcon on systems like the Raspberry Pi.
+* 2. Very few applications and no grahics libraries supports
+*monochrome framebuffers.
+*/
+   if (ctrl->format == LCDCTRL_FORMAT_RGB565) {
+   u16 *vmem16 = (u16 *)update->base;
+
+   /* TODO: add better conversion as done in fb_agm1264k-fl */
+   for (x = 0; x < xres; x++) {
+   for (y = 0; y < yres / 8; y++) {
+   *buf = 0x00;
+   for (i = 0; i < 8; i++) {
+   int y1 = y * 8 + i;
+
+   if (vmem16[y1 * xres + x])
+   *buf |= 1 << i;
+   }
+   buf++;
+   }
+   }
+   } else { /* LCDCTRL_FORMAT_MONO10 */
+   u8 *vmem8 = (u8 *)update->base;
+
+   for (x = 0; x < xres; x++) {
+   for (y = 0; y < yres / 8; y++) {
+   *buf = 0x00;
+   for (i = 0; i < 8; i++) {
+   int y1 = y * 8 + i;
+   int idx = y1 * xres / 8 + x / 8;
+   int mask = (1 << (7 - (x % 8)));
+
+   if (vmem8[idx] & mask)
+   *buf |= 1 << i;
+   }
+   buf++;
+   }
+   }
+   }
+   return lcdreg_write(reg, SSD1306_DISPLAY_START_LINE, &tr);
+}
+
+static int ssd1306_blank(struct lcdctrl *ctrl, bool blank)
+{
+   return lcdreg_writereg(ctrl->lcdreg, blank ? SSD1306_DISPLAY_OFF :
+   SSD1306_DISPLAY_ON);
+}
+

[PATCH] MAINTAINERS: add entry for staging/fbtft/

[Noralf Trønnes]

Add MAINTAINERS entry for staging/fbtft/
FBTFT is a framework for writing framebuffer drivers
for displays with LCD controllers having onchip RAM.

Signed-off-by: Noralf Trønnes 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2fa3853..0dbe230 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3872,6 +3872,12 @@ S:   Supported
 F: Documentation/fault-injection/
 F: lib/fault-inject.c
 
+FBTFT Framebuffer drivers
+M: Thomas Petazzoni 
+M: Noralf Trønnes 
+S: Maintained
+F: drivers/staging/fbtft/
+
 FCOE SUBSYSTEM (libfc, libfcoe, fcoe)
 M: Robert Love 
 L: fcoe-de...@open-fcoe.org
-- 
2.2.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: fb_agm1264k-fl: add static keyword

[Noralf Trønnes]

Add missing static keyword to function reset().
This was detected by the kbuild test robot.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c 
b/drivers/staging/fbtft/fb_agm1264k-fl.c
index 7fe4fa0..9cc7d25 100644
--- a/drivers/staging/fbtft/fb_agm1264k-fl.c
+++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
@@ -89,7 +89,7 @@ static int init_display(struct fbtft_par *par)
return 0;
 }
 
-void reset(struct fbtft_par *par)
+static void reset(struct fbtft_par *par)
 {
if (par->gpio.reset == -1)
return;
-- 
2.2.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: remove ARCH_BCM2708 optimization

[Noralf Trønnes]

ARCH_BCM2708 is not present in mainline so remove optimization.

Signed-off-by: Noralf Trønnes 
---
 drivers/staging/fbtft/fbtft-io.c | 170 ---
 1 file changed, 170 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-io.c b/drivers/staging/fbtft/fbtft-io.c
index dfa2c46..32155a7 100644
--- a/drivers/staging/fbtft/fbtft-io.c
+++ b/drivers/staging/fbtft/fbtft-io.c
@@ -2,9 +2,6 @@
 #include 
 #include 
 #include 
-#ifdef CONFIG_ARCH_BCM2708
-#include 
-#endif
 #include "fbtft.h"
 
 int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len)
@@ -129,171 +126,6 @@ int fbtft_read_spi(struct fbtft_par *par, void *buf, 
size_t len)
 }
 EXPORT_SYMBOL(fbtft_read_spi);
 
-
-#ifdef CONFIG_ARCH_BCM2708
-
-/*
- *  Raspberry Pi
- *  -  writing directly to the registers is 40-50% faster than
- * optimized use of gpiolib
- */
-
-#define GPIOSET(no, ishigh)   \
-do {  \
-   if (ishigh)   \
-   set |= (1 << (no));   \
-   else  \
-   reset |= (1 << (no)); \
-} while (0)
-
-int fbtft_write_gpio8_wr(struct fbtft_par *par, void *buf, size_t len)
-{
-   unsigned int set = 0;
-   unsigned int reset = 0;
-   u8 data;
-
-   fbtft_par_dbg_hex(DEBUG_WRITE, par, par->info->device, u8, buf, len,
-   "%s(len=%d): ", __func__, len);
-
-   while (len--) {
-   data = *(u8 *) buf;
-   buf++;
-
-   /* Set data */
-   GPIOSET(par->gpio.db[0], (data&0x01));
-   GPIOSET(par->gpio.db[1], (data&0x02));
-   GPIOSET(par->gpio.db[2], (data&0x04));
-   GPIOSET(par->gpio.db[3], (data&0x08));
-   GPIOSET(par->gpio.db[4], (data&0x10));
-   GPIOSET(par->gpio.db[5], (data&0x20));
-   GPIOSET(par->gpio.db[6], (data&0x40));
-   GPIOSET(par->gpio.db[7], (data&0x80));
-   writel(set, __io_address(GPIO_BASE+0x1C));
-   writel(reset, __io_address(GPIO_BASE+0x28));
-
-   /* Pulse /WR low */
-   writel((1<gpio.wr),  __io_address(GPIO_BASE+0x28));
-   writel(0,  __io_address(GPIO_BASE+0x28)); /* used as a delay */
-   writel((1<gpio.wr),  __io_address(GPIO_BASE+0x1C));
-
-   set = 0;
-   reset = 0;
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(fbtft_write_gpio8_wr);
-
-int fbtft_write_gpio16_wr(struct fbtft_par *par, void *buf, size_t len)
-{
-   unsigned int set = 0;
-   unsigned int reset = 0;
-   u16 data;
-
-   fbtft_par_dbg_hex(DEBUG_WRITE, par, par->info->device, u8, buf, len,
-   "%s(len=%d): ", __func__, len);
-
-   while (len) {
-   len -= 2;
-   data = *(u16 *) buf;
-   buf += 2;
-
-   /* Start writing by pulling down /WR */
-   gpio_set_value(par->gpio.wr, 0);
-
-   /* Set data */
-   GPIOSET(par->gpio.db[0],  (data&0x0001));
-   GPIOSET(par->gpio.db[1],  (data&0x0002));
-   GPIOSET(par->gpio.db[2],  (data&0x0004));
-   GPIOSET(par->gpio.db[3],  (data&0x0008));
-   GPIOSET(par->gpio.db[4],  (data&0x0010));
-   GPIOSET(par->gpio.db[5],  (data&0x0020));
-   GPIOSET(par->gpio.db[6],  (data&0x0040));
-   GPIOSET(par->gpio.db[7],  (data&0x0080));
-
-   GPIOSET(par->gpio.db[8],  (data&0x0100));
-   GPIOSET(par->gpio.db[9],  (data&0x0200));
-   GPIOSET(par->gpio.db[10], (data&0x0400));
-   GPIOSET(par->gpio.db[11], (data&0x0800));
-   GPIOSET(par->gpio.db[12], (data&0x1000));
-   GPIOSET(par->gpio.db[13], (data&0x2000));
-   GPIOSET(par->gpio.db[14], (data&0x4000));
-   GPIOSET(par->gpio.db[15], (data&0x8000));
-
-   writel(set, __io_address(GPIO_BASE+0x1C));
-   writel(reset, __io_address(GPIO_BASE+0x28));
-
-   /* Pullup /WR */
-   gpio_set_value(par->gpio.wr, 1);
-
-   set = 0;
-   reset = 0;
-   }
-
-   return 0;
-}
-EXPORT_SYMBOL(fbtft_write_gpio16_wr);
-
-int fbtft_write_gpio16_wr_latched(struct fbtft_par *par, void *buf, size_t len)
-{
-   unsigned int set = 0;
-   unsigned int reset = 0;
-   u16 data;
-
-   fbtft_par_dbg_hex(DEBUG_WRITE, par, par->info->device, u8, buf, len,
-   "%s(len=%d): ", __func__, len);
-
-   while (len) {
-   len -= 2;
-   data = *(u16 *) buf;
-   buf += 2;
-
-   /* Start writing by pulling down /WR */
- 

Need help setting up git

[Noralf Trønnes]

Hi,

I can't get my name: Noralf Trønnes, to come out correctly in From when I
format and send a patch. The 'ø' becomes a question mark when received
in my email client.

This is the head of the patch generated by git format-patch:

From b2a4f6abdb097c4dc092b56995a2af8e42fbea79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Noralf=20Tr=F8nnes?= 
Date: Tue, 20 Jan 2015 18:34:47 +0100
Subject: [PATCH] staging: fbtft: remove ARCH_BCM2708 optimization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ARCH_BCM2708 is not present in mainline so remove optimization.

Signed-off-by: Noralf Trønnes 
---

This is how it looks when I send from my Thunderbird email client:
From: =?UTF-8?B?Tm9yYWxmIFRyw7hubmVz?= 


$ git log -1
commit b2a4f6abdb097c4dc092b56995a2af8e42fbea79
Author: Noralf Trnnes 
Date:   Tue Jan 20 18:34:47 2015 +0100

staging: fbtft: remove ARCH_BCM2708 optimization

ARCH_BCM2708 is not present in mainline so remove optimization.

Signed-off-by: Noralf Trnnes 


Setup:

Ubuntu server
$ cat /etc/issue
Ubuntu 12.04.3 LTS \n \l

$ git --version
git version 2.2.2

$ git config -l
user.name=Noralf Trønnes
user.email=no...@tronnes.org
core.editor=nano
alias.serve=daemon --verbose --export-all --base-path=/home/pi --reuseaddr
sendemail.smtpserver=smtp.ebnett.no
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.url=git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
branch.master.remote=origin
branch.master.merge=refs/heads/master
branch.staging-testing.remote=origin
branch.staging-testing.merge=refs/heads/staging-testing


Regards,
Noralf Trønnes

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: fbtft: CONFIG_ARCH_BCM2708?

[Noralf Trønnes]

Den 20.01.2015 09:46, skrev Paul Bolle:

Thomas, Noralf,

Your commit c296d5f9957c ("staging: fbtft: core support") is included in
today's linux-next (ie, next-20150120). I noticed because a script I use
to check linux-next spotted a problem in it.

See, that commit adds two checks for CONFIG_ARCH_BCM2708. But there's no
Kconfig symbol ARCH_BCM2708. (A comment in
drivers/mmc/host/sdhci-bcm2835.c does suggest there's an out of tree
"Main bcm2708 linux port".)

Is the code to add the Kconfig symbol ARCH_BCM2708 queued somewhere?

Hi Paul,

ARCH_BCM2708 is used instead of ARCH_BCM2835 in the Raspberry Pi kernel 
fork.
The optimization code enabled by ARCH_BCM2708 in 
drivers/staging/fbtft/fbtft-io.c can be removed.

There will be another take on the I/O stage for those drivers later.

I can make a patch for this. Just need to find out how to setup git to 
send paches for me.


Thanks for spotting this.


Noralf.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel