Commit ec03c210 authored by Andy Shevchenko's avatar Andy Shevchenko Committed by Greg Kroah-Hartman

staging: fbtft: Rectify GPIO handling

The infamous commit c440eee1 ("Staging: staging: fbtft: Switch to
the GPIO descriptor interface") broke GPIO handling completely.
It has already four commits to rectify and it seems not enough.
In order to fix the mess here we:

  1) Set default to "inactive" for all requested pins

  2) Fix CS#, RD#, and WR# pins polarity since it's active low
     and GPIO descriptor interface takes it into consideration
     from the Device Tree or ACPI

  3) Consolidate chip activation (CS# assertion) under default
     ->reset() callback

To summarize the expectations about polarity for GPIOs:

   RD#			Low
   WR#			Low
   CS#			Low
   RESET#		Low
   DC or RS		High
   RW			High
   Data	0 .. 15		High

See also Adafruit learning course [1] for the example of the schematics.

While at it, drop unneeded NULL checks, since GPIO API is tolerant to that.

[1]: https://learn.adafruit.com/adafruit-2-8-and-3-2-color-tft-touchscreen-breakout-v2/downloads

Fixes: 92e3e884 ("Staging: fbtft: Fix GPIO handling")
Fixes: b918d1c2 ("Staging: fbtft: Fix reset assertion when using gpio descriptor")
Fixes: dbc4f989 ("Staging: fbtft: Fix probing of gpio descriptor")
Fixes: c440eee1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Cc: Jan Sebastian Götte <linux@jaseg.net>
Cc: Nishad Kamdar <nishadkamdar@gmail.com>
Reviewed-by: default avatarPhil Reid <preid@electromag.com.au>
Signed-off-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20210503172114.27891-2-andriy.shevchenko@linux.intel.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 0d59ca5a
...@@ -84,9 +84,9 @@ static void reset(struct fbtft_par *par) ...@@ -84,9 +84,9 @@ static void reset(struct fbtft_par *par)
dev_dbg(par->info->device, "%s()\n", __func__); dev_dbg(par->info->device, "%s()\n", __func__);
gpiod_set_value(par->gpio.reset, 0);
udelay(20);
gpiod_set_value(par->gpio.reset, 1); gpiod_set_value(par->gpio.reset, 1);
udelay(20);
gpiod_set_value(par->gpio.reset, 0);
mdelay(120); mdelay(120);
} }
...@@ -194,12 +194,12 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...) ...@@ -194,12 +194,12 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
/* select chip */ /* select chip */
if (*buf) { if (*buf) {
/* cs1 */ /* cs1 */
gpiod_set_value(par->CS0, 1);
gpiod_set_value(par->CS1, 0);
} else {
/* cs0 */
gpiod_set_value(par->CS0, 0); gpiod_set_value(par->CS0, 0);
gpiod_set_value(par->CS1, 1); gpiod_set_value(par->CS1, 1);
} else {
/* cs0 */
gpiod_set_value(par->CS0, 1);
gpiod_set_value(par->CS1, 0);
} }
gpiod_set_value(par->RS, 0); /* RS->0 (command mode) */ gpiod_set_value(par->RS, 0); /* RS->0 (command mode) */
...@@ -397,8 +397,8 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len) ...@@ -397,8 +397,8 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
} }
kfree(convert_buf); kfree(convert_buf);
gpiod_set_value(par->CS0, 1); gpiod_set_value(par->CS0, 0);
gpiod_set_value(par->CS1, 1); gpiod_set_value(par->CS1, 0);
return ret; return ret;
} }
...@@ -419,10 +419,10 @@ static int write(struct fbtft_par *par, void *buf, size_t len) ...@@ -419,10 +419,10 @@ static int write(struct fbtft_par *par, void *buf, size_t len)
for (i = 0; i < 8; ++i) for (i = 0; i < 8; ++i)
gpiod_set_value(par->gpio.db[i], data & (1 << i)); gpiod_set_value(par->gpio.db[i], data & (1 << i));
/* set E */ /* set E */
gpiod_set_value(par->EPIN, 1); gpiod_set_value(par->EPIN, 0);
udelay(5); udelay(5);
/* unset E - write */ /* unset E - write */
gpiod_set_value(par->EPIN, 0); gpiod_set_value(par->EPIN, 1);
udelay(1); udelay(1);
} }
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
...@@ -24,9 +23,6 @@ ...@@ -24,9 +23,6 @@
static int init_display(struct fbtft_par *par) static int init_display(struct fbtft_par *par)
{ {
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
par->fbtftops.reset(par); par->fbtftops.reset(par);
/* Initialization sequence from Lib_UTFT */ /* Initialization sequence from Lib_UTFT */
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include <video/mipi_display.h> #include <video/mipi_display.h>
...@@ -77,9 +76,6 @@ static int init_display(struct fbtft_par *par) ...@@ -77,9 +76,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
write_reg(par, MIPI_DCS_SOFT_RESET); /* software reset */ write_reg(par, MIPI_DCS_SOFT_RESET); /* software reset */
mdelay(500); mdelay(500);
write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); /* exit sleep */ write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); /* exit sleep */
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/spi/spi.h> #include <linux/spi/spi.h>
#include <linux/delay.h> #include <linux/delay.h>
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
...@@ -85,9 +84,6 @@ static int init_display(struct fbtft_par *par) ...@@ -85,9 +84,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
bt &= 0x07; bt &= 0x07;
vc &= 0x07; vc &= 0x07;
vrh &= 0x0f; vrh &= 0x0f;
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include <video/mipi_display.h> #include <video/mipi_display.h>
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
...@@ -29,9 +28,6 @@ static int init_display(struct fbtft_par *par) ...@@ -29,9 +28,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
/* Initialization sequence from Lib_UTFT */ /* Initialization sequence from Lib_UTFT */
write_reg(par, 0x0011, 0x2004); write_reg(par, 0x0011, 0x2004);
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include "fbtft.h" #include "fbtft.h"
...@@ -28,9 +27,6 @@ static int init_display(struct fbtft_par *par) ...@@ -28,9 +27,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
write_reg(par, 0x00, 0x0001); write_reg(par, 0x00, 0x0001);
write_reg(par, 0x03, 0xA8A4); write_reg(par, 0x03, 0xA8A4);
write_reg(par, 0x0C, 0x0000); write_reg(par, 0x0C, 0x0000);
......
...@@ -35,8 +35,6 @@ static int init_display(struct fbtft_par *par) ...@@ -35,8 +35,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
gpiod_set_value(par->gpio.cs, 0);
write_reg(par, 0xb3); write_reg(par, 0xb3);
write_reg(par, 0xf0); write_reg(par, 0xf0);
write_reg(par, 0xae); write_reg(par, 0xae);
......
...@@ -81,8 +81,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...) ...@@ -81,8 +81,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
va_start(args, len); va_start(args, len);
*buf = (u8)va_arg(args, unsigned int); *buf = (u8)va_arg(args, unsigned int);
if (par->gpio.dc) gpiod_set_value(par->gpio.dc, 0);
gpiod_set_value(par->gpio.dc, 0);
ret = par->fbtftops.write(par, par->buf, sizeof(u8)); ret = par->fbtftops.write(par, par->buf, sizeof(u8));
if (ret < 0) { if (ret < 0) {
va_end(args); va_end(args);
...@@ -104,8 +103,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...) ...@@ -104,8 +103,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len, ...)
return; return;
} }
} }
if (par->gpio.dc) gpiod_set_value(par->gpio.dc, 1);
gpiod_set_value(par->gpio.dc, 1);
va_end(args); va_end(args);
} }
......
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/spi/spi.h> #include <linux/spi/spi.h>
#include <linux/delay.h> #include <linux/delay.h>
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
...@@ -26,9 +25,6 @@ static int init_display(struct fbtft_par *par) ...@@ -26,9 +25,6 @@ static int init_display(struct fbtft_par *par)
{ {
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
/* Initialization sequence from Lib_UTFT */ /* Initialization sequence from Lib_UTFT */
/* register reset */ /* register reset */
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/init.h> #include <linux/init.h>
#include <linux/gpio/consumer.h>
#include <linux/delay.h> #include <linux/delay.h>
#include "fbtft.h" #include "fbtft.h"
......
...@@ -135,8 +135,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len) ...@@ -135,8 +135,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
remain = len / 2; remain = len / 2;
vmem16 = (u16 *)(par->info->screen_buffer + offset); vmem16 = (u16 *)(par->info->screen_buffer + offset);
if (par->gpio.dc) gpiod_set_value(par->gpio.dc, 1);
gpiod_set_value(par->gpio.dc, 1);
/* non buffered write */ /* non buffered write */
if (!par->txbuf.buf) if (!par->txbuf.buf)
......
...@@ -38,8 +38,7 @@ int fbtft_write_buf_dc(struct fbtft_par *par, void *buf, size_t len, int dc) ...@@ -38,8 +38,7 @@ int fbtft_write_buf_dc(struct fbtft_par *par, void *buf, size_t len, int dc)
{ {
int ret; int ret;
if (par->gpio.dc) gpiod_set_value(par->gpio.dc, dc);
gpiod_set_value(par->gpio.dc, dc);
ret = par->fbtftops.write(par, buf, len); ret = par->fbtftops.write(par, buf, len);
if (ret < 0) if (ret < 0)
...@@ -79,7 +78,7 @@ static int fbtft_request_one_gpio(struct fbtft_par *par, ...@@ -79,7 +78,7 @@ static int fbtft_request_one_gpio(struct fbtft_par *par,
int ret = 0; int ret = 0;
*gpiop = devm_gpiod_get_index_optional(dev, name, index, *gpiop = devm_gpiod_get_index_optional(dev, name, index,
GPIOD_OUT_HIGH); GPIOD_OUT_LOW);
if (IS_ERR(*gpiop)) { if (IS_ERR(*gpiop)) {
ret = PTR_ERR(*gpiop); ret = PTR_ERR(*gpiop);
dev_err(dev, dev_err(dev,
...@@ -226,11 +225,15 @@ static void fbtft_reset(struct fbtft_par *par) ...@@ -226,11 +225,15 @@ static void fbtft_reset(struct fbtft_par *par)
{ {
if (!par->gpio.reset) if (!par->gpio.reset)
return; return;
fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__); fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__);
gpiod_set_value_cansleep(par->gpio.reset, 1); gpiod_set_value_cansleep(par->gpio.reset, 1);
usleep_range(20, 40); usleep_range(20, 40);
gpiod_set_value_cansleep(par->gpio.reset, 0); gpiod_set_value_cansleep(par->gpio.reset, 0);
msleep(120); msleep(120);
gpiod_set_value_cansleep(par->gpio.cs, 1); /* Activate chip */
} }
static void fbtft_update_display(struct fbtft_par *par, unsigned int start_line, static void fbtft_update_display(struct fbtft_par *par, unsigned int start_line,
...@@ -922,8 +925,6 @@ static int fbtft_init_display_from_property(struct fbtft_par *par) ...@@ -922,8 +925,6 @@ static int fbtft_init_display_from_property(struct fbtft_par *par)
goto out_free; goto out_free;
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
index = -1; index = -1;
val = values[++index]; val = values[++index];
...@@ -1018,8 +1019,6 @@ int fbtft_init_display(struct fbtft_par *par) ...@@ -1018,8 +1019,6 @@ int fbtft_init_display(struct fbtft_par *par)
} }
par->fbtftops.reset(par); par->fbtftops.reset(par);
if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
i = 0; i = 0;
while (i < FBTFT_MAX_INIT_SEQUENCE) { while (i < FBTFT_MAX_INIT_SEQUENCE) {
......
...@@ -142,12 +142,12 @@ int fbtft_write_gpio8_wr(struct fbtft_par *par, void *buf, size_t len) ...@@ -142,12 +142,12 @@ int fbtft_write_gpio8_wr(struct fbtft_par *par, void *buf, size_t len)
data = *(u8 *)buf; data = *(u8 *)buf;
/* Start writing by pulling down /WR */ /* Start writing by pulling down /WR */
gpiod_set_value(par->gpio.wr, 0); gpiod_set_value(par->gpio.wr, 1);
/* Set data */ /* Set data */
#ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO #ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO
if (data == prev_data) { if (data == prev_data) {
gpiod_set_value(par->gpio.wr, 0); /* used as delay */ gpiod_set_value(par->gpio.wr, 1); /* used as delay */
} else { } else {
for (i = 0; i < 8; i++) { for (i = 0; i < 8; i++) {
if ((data & 1) != (prev_data & 1)) if ((data & 1) != (prev_data & 1))
...@@ -165,7 +165,7 @@ int fbtft_write_gpio8_wr(struct fbtft_par *par, void *buf, size_t len) ...@@ -165,7 +165,7 @@ int fbtft_write_gpio8_wr(struct fbtft_par *par, void *buf, size_t len)
#endif #endif
/* Pullup /WR */ /* Pullup /WR */
gpiod_set_value(par->gpio.wr, 1); gpiod_set_value(par->gpio.wr, 0);
#ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO #ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO
prev_data = *(u8 *)buf; prev_data = *(u8 *)buf;
...@@ -192,12 +192,12 @@ int fbtft_write_gpio16_wr(struct fbtft_par *par, void *buf, size_t len) ...@@ -192,12 +192,12 @@ int fbtft_write_gpio16_wr(struct fbtft_par *par, void *buf, size_t len)
data = *(u16 *)buf; data = *(u16 *)buf;
/* Start writing by pulling down /WR */ /* Start writing by pulling down /WR */
gpiod_set_value(par->gpio.wr, 0); gpiod_set_value(par->gpio.wr, 1);
/* Set data */ /* Set data */
#ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO #ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO
if (data == prev_data) { if (data == prev_data) {
gpiod_set_value(par->gpio.wr, 0); /* used as delay */ gpiod_set_value(par->gpio.wr, 1); /* used as delay */
} else { } else {
for (i = 0; i < 16; i++) { for (i = 0; i < 16; i++) {
if ((data & 1) != (prev_data & 1)) if ((data & 1) != (prev_data & 1))
...@@ -215,7 +215,7 @@ int fbtft_write_gpio16_wr(struct fbtft_par *par, void *buf, size_t len) ...@@ -215,7 +215,7 @@ int fbtft_write_gpio16_wr(struct fbtft_par *par, void *buf, size_t len)
#endif #endif
/* Pullup /WR */ /* Pullup /WR */
gpiod_set_value(par->gpio.wr, 1); gpiod_set_value(par->gpio.wr, 0);
#ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO #ifndef DO_NOT_OPTIMIZE_FBTFT_WRITE_GPIO
prev_data = *(u16 *)buf; prev_data = *(u16 *)buf;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment