[PATCH 00/10] Staging: fbtft: flexfb.c file clean up

[PATCH 00/10] Staging: fbtft: flexfb.c file clean up

[Fabio Falzoi]

This patchset aims to refactor the code in flexfb.c, making it more
readable and maintanable.
Various checkpatch.pl issues are fixed.
These patches are created on top of greg-kh's linux-staging tree,
staging-testing branch.

Fabio Falzoi (10):
  Staging: fbtft: Remove paragraph about writing to FSF
  Staging: fbtft: Remove unnecessary multiple blank lines
  Staging: fbtft: Use a struct to describe each LCD controller
  Staging: fbtft: Use a helper function to set write_register op
  Staging: fbtft: Set bus specific ops using separate functions
  Staging: fbtft: Use a helper function to set set_addr_win op
  Staging: fbtft: Remove useless newline
  Staging: fbtft: Avoid duplicating code to check gpio.dc value
  Staging: fbtft: Fix parenthesis alignment coding style issue
  Staging: fbtft: Fix spacing coding style issue

 drivers/staging/fbtft/fbtft.h  |  20 ++
 drivers/staging/fbtft/flexfb.c | 485 +++++++++++++++++++++++------------------
 2 files changed, 289 insertions(+), 216 deletions(-)

-- 
2.1.4

[PATCH 01/10] Staging: fbtft: Remove paragraph about writing to FSF

[Fabio Falzoi]

Remove paragraph about writing to the Free Software Foundation's
mailing address from GPL notice.
This patch fixes the following checkpatch error:

CHECK:FSF_MAILING_ADDRESS at line 17.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index 2c4ce07..80b6620 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -12,10 +12,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 #include <linux/module.h>
-- 
2.1.4

[PATCH 02/10] Staging: fbtft: Remove unnecessary multiple blank lines

[Fabio Falzoi]

This patch removes some unnecessary multiple blank lines to fix the
following checkpatch errors:

CHECK:LINE_SPACING at lines 29, 67, 131, 287, 299, 312, 326, 351 and
364.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index 80b6620..ed867e7 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -26,7 +26,6 @@
 
 #define DRVNAME	    "flexfb"
 
-
 static char *chip;
 module_param(chip, charp, 0);
 MODULE_PARM_DESC(chip, "LCD controller");
@@ -64,7 +63,6 @@ static bool latched;
 module_param(latched, bool, 0);
 MODULE_PARM_DESC(latched, "Use with latched 16-bit databus");
 
-
 static int *initp;
 static int initp_num;
 
@@ -128,7 +126,6 @@ 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 };
 
-
 /* ili9320, ili9325 */
 static void flexfb_set_addr_win_1(struct fbtft_par *par,
 				  int xs, int ys, int xe, int ye)
@@ -284,7 +281,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(st7735r_init);
 			}
 
-
 		} else if (!strcmp(chip, "hx8340bn")) {
 			if (!width)
 				width = 176;
@@ -296,7 +292,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(hx8340bn_init);
 			}
 
-
 		} else if (!strcmp(chip, "ili9225")) {
 			if (!width)
 				width = 176;
@@ -309,8 +304,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(ili9225_init);
 			}
 
-
-
 		} else if (!strcmp(chip, "ili9320")) {
 			if (!width)
 				width = 240;
@@ -323,7 +316,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(ili9320_init);
 			}
 
-
 		} else if (!strcmp(chip, "ili9325")) {
 			if (!width)
 				width = 240;
@@ -348,7 +340,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(ili9341_init);
 			}
 
-
 		} else if (!strcmp(chip, "ssd1289")) {
 			if (!width)
 				width = 240;
@@ -361,8 +352,6 @@ static int flexfb_probe_common(struct spi_device *sdev,
 				initp_num = ARRAY_SIZE(ssd1289_init);
 			}
 
-
-
 		} else if (!strcmp(chip, "ssd1351")) {
 			if (!width)
 				width = 128;
-- 
2.1.4

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

[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 <fabio.falzoi84@gmail.com>
---
 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;
+};
+
 #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),
+	},
+	{
+		.name = "hx8340bn",
+		.width = 176,
+		.height = 220,
+		.init_seq = hx8340bn_init,
+		.init_seq_sz = ARRAY_SIZE(hx8340bn_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9320",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9320_init,
+		.init_seq_sz = ARRAY_SIZE(ili9320_init),
+	},
+	{
+		.name = "ili9325",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9325_init,
+		.init_seq_sz = ARRAY_SIZE(ili9325_init),
+	},
+	{
+		.name = "ili9341",
+		.width = 240,
+		.height = 320,
+		.init_seq = ili9341_init,
+		.init_seq_sz = ARRAY_SIZE(ili9341_init),
+	},
+	{
+		.name = "ssd1289",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 2,
+		.regwidth = 16,
+		.init_seq = ssd1289_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1289_init),
+	},
+	{
+		.name = "ssd1351",
+		.width = 128,
+		.height = 128,
+		.setaddrwin = 3,
+		.init_seq = ssd1351_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1351_init),
+	},
+};
+
 /* ili9320, ili9325 */
 static void flexfb_set_addr_win_1(struct fbtft_par *par,
 				  int xs, int ys, int xe, int ye)
@@ -248,8 +331,38 @@ static int flexfb_verify_gpios_db(struct fbtft_par *par)
 	return 0;
 }
 
+static void flexfb_chip_load_param(const struct flexfb_lcd_controller *chip)
+{
+	if (!width)
+		width = chip->width;
+	if (!height)
+		height = chip->height;
+	setaddrwin = chip->setaddrwin;
+	if (chip->regwidth)
+		regwidth = chip->regwidth;
+	if (!init_num) {
+		initp = chip->init_seq;
+		initp_num = chip->init_seq_sz;
+	}
+}
+
 static struct fbtft_display flex_display = { };
 
+static int flexfb_chip_init(const struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(flexfb_chip_table); i++)
+		if (!strcmp(chip, flexfb_chip_table[i].name)) {
+			flexfb_chip_load_param(&flexfb_chip_table[i]);
+			return 0;
+		}
+
+	dev_err(dev, "chip=%s is not supported\n", chip);
+
+	return -EINVAL;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -270,102 +383,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
 		       sdev ? "'SPI device'" : "'Platform device'");
 
 	if (chip) {
-
-		if (!strcmp(chip, "st7735r")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 160;
-			if (init_num == 0) {
-				initp = st7735r_init;
-				initp_num = ARRAY_SIZE(st7735r_init);
-			}
-
-		} else if (!strcmp(chip, "hx8340bn")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			if (init_num == 0) {
-				initp = hx8340bn_init;
-				initp_num = ARRAY_SIZE(hx8340bn_init);
-			}
-
-		} else if (!strcmp(chip, "ili9225")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9225_init;
-				initp_num = ARRAY_SIZE(ili9225_init);
-			}
-
-		} else if (!strcmp(chip, "ili9320")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9320_init;
-				initp_num = ARRAY_SIZE(ili9320_init);
-			}
-
-		} else if (!strcmp(chip, "ili9325")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9325_init;
-				initp_num = ARRAY_SIZE(ili9325_init);
-			}
-
-		} else if (!strcmp(chip, "ili9341")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 0;
-			regwidth = 8;
-			if (init_num == 0) {
-				initp = ili9341_init;
-				initp_num = ARRAY_SIZE(ili9341_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1289")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 2;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ssd1289_init;
-				initp_num = ARRAY_SIZE(ssd1289_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1351")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 128;
-			setaddrwin = 3;
-			if (init_num == 0) {
-				initp = ssd1351_init;
-				initp_num = ARRAY_SIZE(ssd1351_init);
-			}
-		} else {
-			dev_err(dev, "chip=%s is not supported\n", chip);
-			return -EINVAL;
-		}
+		ret = flexfb_chip_init(dev);
+		if (ret)
+			return ret;
 	}
 
 	if (width == 0 || height == 0) {
-- 
2.1.4

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

[Greg KH]

On Tue, Jun 30, 2015 at 08:43:10AM +0200, Fabio Falzoi wrote:
> 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 <fabio.falzoi84@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft.h  |  20 ++++
>  drivers/staging/fbtft/flexfb.c | 212 ++++++++++++++++++++++-------------------
>  2 files changed, 136 insertions(+), 96 deletions(-)

I need the maintainers to sign off on this before I can take it, as it's
not just a "simple" coding style fix.

thanks,

greg k-h

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 <fabio.falzoi84@gmail.com>
> ---
>   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 <noralf@tronnes.org>

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

[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 <fabio.falzoi84@gmail.com>
Acked-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/staging/fbtft/fbtft.h  |  20 ++++
 drivers/staging/fbtft/flexfb.c | 203 ++++++++++++++++++++++-------------------
 2 files changed, 127 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;
+};
+
 #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 ce6e3ae..e70678f 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -126,6 +126,80 @@ 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),
+	}, {
+		.name = "hx8340bn",
+		.width = 176,
+		.height = 220,
+		.init_seq = hx8340bn_init,
+		.init_seq_sz = ARRAY_SIZE(hx8340bn_init),
+	}, {
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	}, {
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	}, {
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	}, {
+		.name = "ili9320",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9320_init,
+		.init_seq_sz = ARRAY_SIZE(ili9320_init),
+	}, {
+		.name = "ili9325",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9325_init,
+		.init_seq_sz = ARRAY_SIZE(ili9325_init),
+	}, {
+		.name = "ili9341",
+		.width = 240,
+		.height = 320,
+		.init_seq = ili9341_init,
+		.init_seq_sz = ARRAY_SIZE(ili9341_init),
+	}, {
+		.name = "ssd1289",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 2,
+		.regwidth = 16,
+		.init_seq = ssd1289_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1289_init),
+	}, {
+		.name = "ssd1351",
+		.width = 128,
+		.height = 128,
+		.setaddrwin = 3,
+		.init_seq = ssd1351_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1351_init),
+	},
+};
+
 /* ili9320, ili9325 */
 static void flexfb_set_addr_win_1(struct fbtft_par *par,
 				  int xs, int ys, int xe, int ye)
@@ -247,8 +321,38 @@ static int flexfb_verify_gpios_db(struct fbtft_par *par)
 	return 0;
 }
 
+static void flexfb_chip_load_param(const struct flexfb_lcd_controller *chip)
+{
+	if (!width)
+		width = chip->width;
+	if (!height)
+		height = chip->height;
+	setaddrwin = chip->setaddrwin;
+	if (chip->regwidth)
+		regwidth = chip->regwidth;
+	if (!init_num) {
+		initp = chip->init_seq;
+		initp_num = chip->init_seq_sz;
+	}
+}
+
 static struct fbtft_display flex_display = { };
 
+static int flexfb_chip_init(const struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(flexfb_chip_table); i++)
+		if (!strcmp(chip, flexfb_chip_table[i].name)) {
+			flexfb_chip_load_param(&flexfb_chip_table[i]);
+			return 0;
+		}
+
+	dev_err(dev, "chip=%s is not supported\n", chip);
+
+	return -EINVAL;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -269,102 +373,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
 		       sdev ? "'SPI device'" : "'Platform device'");
 
 	if (chip) {
-
-		if (!strcmp(chip, "st7735r")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 160;
-			if (init_num == 0) {
-				initp = st7735r_init;
-				initp_num = ARRAY_SIZE(st7735r_init);
-			}
-
-		} else if (!strcmp(chip, "hx8340bn")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			if (init_num == 0) {
-				initp = hx8340bn_init;
-				initp_num = ARRAY_SIZE(hx8340bn_init);
-			}
-
-		} else if (!strcmp(chip, "ili9225")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9225_init;
-				initp_num = ARRAY_SIZE(ili9225_init);
-			}
-
-		} else if (!strcmp(chip, "ili9320")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9320_init;
-				initp_num = ARRAY_SIZE(ili9320_init);
-			}
-
-		} else if (!strcmp(chip, "ili9325")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9325_init;
-				initp_num = ARRAY_SIZE(ili9325_init);
-			}
-
-		} else if (!strcmp(chip, "ili9341")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 0;
-			regwidth = 8;
-			if (init_num == 0) {
-				initp = ili9341_init;
-				initp_num = ARRAY_SIZE(ili9341_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1289")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 2;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ssd1289_init;
-				initp_num = ARRAY_SIZE(ssd1289_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1351")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 128;
-			setaddrwin = 3;
-			if (init_num == 0) {
-				initp = ssd1351_init;
-				initp_num = ARRAY_SIZE(ssd1351_init);
-			}
-		} else {
-			dev_err(dev, "chip=%s is not supported\n", chip);
-			return -EINVAL;
-		}
+		ret = flexfb_chip_init(dev);
+		if (ret)
+			return ret;
 	}
 
 	if (width == 0 || height == 0) {
-- 
2.1.4

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 <fabio.falzoi84@gmail.com>
> Acked-by: Noralf Trønnes <noralf@tronnes.org>
> ---

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.

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

[Fabio Falzoi]

On Sun, Aug 02, 2015 at 07:54:48PM +0200, Noralf Trønnes wrote:
> 
> 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.
> 
>

Sorry Noralf, I misunderstood your review.
I will modify the patch and send an updated version.

Thank you,
Fabio

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

[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 <fabio.falzoi84@gmail.com>
Acked-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/staging/fbtft/flexfb.c | 232 ++++++++++++++++++++++++-----------------
 1 file changed, 136 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index ce6e3ae..b3a68c9 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -126,6 +126,109 @@ 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 };
 
+/**
+ * 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;
+};
+
+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),
+	},
+	{
+		.name = "hx8340bn",
+		.width = 176,
+		.height = 220,
+		.init_seq = hx8340bn_init,
+		.init_seq_sz = ARRAY_SIZE(hx8340bn_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9225",
+		.width = 176,
+		.height = 220,
+		.regwidth = 16,
+		.init_seq = ili9225_init,
+		.init_seq_sz = ARRAY_SIZE(ili9225_init),
+	},
+	{
+		.name = "ili9320",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9320_init,
+		.init_seq_sz = ARRAY_SIZE(ili9320_init),
+	},
+	{
+		.name = "ili9325",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 1,
+		.regwidth = 16,
+		.init_seq = ili9325_init,
+		.init_seq_sz = ARRAY_SIZE(ili9325_init),
+	},
+	{
+		.name = "ili9341",
+		.width = 240,
+		.height = 320,
+		.init_seq = ili9341_init,
+		.init_seq_sz = ARRAY_SIZE(ili9341_init),
+	},
+	{
+		.name = "ssd1289",
+		.width = 240,
+		.height = 320,
+		.setaddrwin = 2,
+		.regwidth = 16,
+		.init_seq = ssd1289_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1289_init),
+	},
+	{
+		.name = "ssd1351",
+		.width = 128,
+		.height = 128,
+		.setaddrwin = 3,
+		.init_seq = ssd1351_init,
+		.init_seq_sz = ARRAY_SIZE(ssd1351_init),
+	},
+};
+
 /* ili9320, ili9325 */
 static void flexfb_set_addr_win_1(struct fbtft_par *par,
 				  int xs, int ys, int xe, int ye)
@@ -247,8 +350,38 @@ static int flexfb_verify_gpios_db(struct fbtft_par *par)
 	return 0;
 }
 
+static void flexfb_chip_load_param(const struct flexfb_lcd_controller *chip)
+{
+	if (!width)
+		width = chip->width;
+	if (!height)
+		height = chip->height;
+	setaddrwin = chip->setaddrwin;
+	if (chip->regwidth)
+		regwidth = chip->regwidth;
+	if (!init_num) {
+		initp = chip->init_seq;
+		initp_num = chip->init_seq_sz;
+	}
+}
+
 static struct fbtft_display flex_display = { };
 
+static int flexfb_chip_init(const struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(flexfb_chip_table); i++)
+		if (!strcmp(chip, flexfb_chip_table[i].name)) {
+			flexfb_chip_load_param(&flexfb_chip_table[i]);
+			return 0;
+		}
+
+	dev_err(dev, "chip=%s is not supported\n", chip);
+
+	return -EINVAL;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -269,102 +402,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
 		       sdev ? "'SPI device'" : "'Platform device'");
 
 	if (chip) {
-
-		if (!strcmp(chip, "st7735r")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 160;
-			if (init_num == 0) {
-				initp = st7735r_init;
-				initp_num = ARRAY_SIZE(st7735r_init);
-			}
-
-		} else if (!strcmp(chip, "hx8340bn")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			if (init_num == 0) {
-				initp = hx8340bn_init;
-				initp_num = ARRAY_SIZE(hx8340bn_init);
-			}
-
-		} else if (!strcmp(chip, "ili9225")) {
-			if (!width)
-				width = 176;
-			if (!height)
-				height = 220;
-			setaddrwin = 0;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9225_init;
-				initp_num = ARRAY_SIZE(ili9225_init);
-			}
-
-		} else if (!strcmp(chip, "ili9320")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9320_init;
-				initp_num = ARRAY_SIZE(ili9320_init);
-			}
-
-		} else if (!strcmp(chip, "ili9325")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 1;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ili9325_init;
-				initp_num = ARRAY_SIZE(ili9325_init);
-			}
-
-		} else if (!strcmp(chip, "ili9341")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 0;
-			regwidth = 8;
-			if (init_num == 0) {
-				initp = ili9341_init;
-				initp_num = ARRAY_SIZE(ili9341_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1289")) {
-			if (!width)
-				width = 240;
-			if (!height)
-				height = 320;
-			setaddrwin = 2;
-			regwidth = 16;
-			if (init_num == 0) {
-				initp = ssd1289_init;
-				initp_num = ARRAY_SIZE(ssd1289_init);
-			}
-
-		} else if (!strcmp(chip, "ssd1351")) {
-			if (!width)
-				width = 128;
-			if (!height)
-				height = 128;
-			setaddrwin = 3;
-			if (init_num == 0) {
-				initp = ssd1351_init;
-				initp_num = ARRAY_SIZE(ssd1351_init);
-			}
-		} else {
-			dev_err(dev, "chip=%s is not supported\n", chip);
-			return -EINVAL;
-		}
+		ret = flexfb_chip_init(dev);
+		if (ret)
+			return ret;
 	}
 
 	if (width == 0 || height == 0) {
-- 
2.1.4

[PATCH 04/10] Staging: fbtft: Use a helper function to set write_register op

[Fabio Falzoi]

Use a helper function to set the correct write_register function, based
on the width of the registers.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index 25b394d..dae092a 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -363,6 +363,25 @@ static int flexfb_chip_init(const struct device *dev)
 	return -EINVAL;
 }
 
+static int flexfb_set_regwrite_func(const struct device *dev,
+				    struct fbtft_par *par)
+{
+	switch (regwidth) {
+	case 8:
+		par->fbtftops.write_register = fbtft_write_reg8_bus8;
+		break;
+	case 16:
+		par->fbtftops.write_register = fbtft_write_reg16_bus8;
+		break;
+	default:
+		dev_err(dev, "argument 'regwidth': %d is not supported.\n",
+			regwidth);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -413,20 +432,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
 		par->init_sequence = initp;
 	par->fbtftops.init_display = fbtft_init_display;
 
-	/* registerwrite functions */
-	switch (regwidth) {
-	case 8:
-		par->fbtftops.write_register = fbtft_write_reg8_bus8;
-		break;
-	case 16:
-		par->fbtftops.write_register = fbtft_write_reg16_bus8;
-		break;
-	default:
-		dev_err(dev,
-			"argument 'regwidth': %d is not supported.\n",
-			regwidth);
-		return -EINVAL;
-	}
+	ret = flexfb_set_regwrite_func(dev, par);
+	if (ret)
+		goto out_release;
 
 	/* bus functions */
 	if (sdev) {
-- 
2.1.4

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

[Greg KH]

On Tue, Jun 30, 2015 at 08:43:11AM +0200, Fabio Falzoi wrote:
> Use a helper function to set the correct write_register function, based
> on the width of the registers.
> 
> Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
> ---
>  drivers/staging/fbtft/flexfb.c | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
> index 25b394d..dae092a 100644
> --- a/drivers/staging/fbtft/flexfb.c
> +++ b/drivers/staging/fbtft/flexfb.c
> @@ -363,6 +363,25 @@ static int flexfb_chip_init(const struct device *dev)
>  	return -EINVAL;
>  }
>  
> +static int flexfb_set_regwrite_func(const struct device *dev,
> +				    struct fbtft_par *par)
> +{
> +	switch (regwidth) {
> +	case 8:
> +		par->fbtftops.write_register = fbtft_write_reg8_bus8;
> +		break;
> +	case 16:
> +		par->fbtftops.write_register = fbtft_write_reg16_bus8;
> +		break;
> +	default:
> +		dev_err(dev, "argument 'regwidth': %d is not supported.\n",
> +			regwidth);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int flexfb_probe_common(struct spi_device *sdev,
>  			       struct platform_device *pdev)
>  {
> @@ -413,20 +432,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
>  		par->init_sequence = initp;
>  	par->fbtftops.init_display = fbtft_init_display;
>  
> -	/* registerwrite functions */
> -	switch (regwidth) {
> -	case 8:
> -		par->fbtftops.write_register = fbtft_write_reg8_bus8;
> -		break;
> -	case 16:
> -		par->fbtftops.write_register = fbtft_write_reg16_bus8;
> -		break;
> -	default:
> -		dev_err(dev,
> -			"argument 'regwidth': %d is not supported.\n",
> -			regwidth);
> -		return -EINVAL;
> -	}
> +	ret = flexfb_set_regwrite_func(dev, par);
> +	if (ret)
> +		goto out_release;
>  
>  	/* bus functions */
>  	if (sdev) {

Why?  You aren't calling this function anywhere else, so why move it?

thanks,

greg k-h

[PATCH 05/10] Staging: fbtft: Set bus specific ops using separate functions

[Fabio Falzoi]

Use two separate functions for spi and platform bus
(flexfb_set_spi_bus_func and flexfb_set_platform_bus_func,
respectively) to set the appropriate write operations.

This patch corrects the following checkpatch errors:
WARNING:LONG_LiINE at lines 446, 450, 466, 476, 489 and 495.
CHECK:PARENTHESIS_ALIGNMENT at line 459.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 154 +++++++++++++++++++++++++----------------
 1 file changed, 94 insertions(+), 60 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index dae092a..1b833f9 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -382,6 +382,94 @@ static int flexfb_set_regwrite_func(const struct device *dev,
 	return 0;
 }
 
+static int flexfb_emulate_spi_8(struct fbtft_par *par, struct spi_device *sdev)
+{
+	struct device *dev = &sdev->dev;
+	int ret;
+
+	dev_warn(dev, "9-bit SPI not available, emulating using 8-bit.\n");
+	sdev->bits_per_word = 8;
+	ret = sdev->master->setup(sdev);
+	if (ret)
+		return ret;
+
+	/* allocate buffer with room for dc bits */
+	par->extra = devm_kzalloc(par->info->device,
+				  par->txbuf.len + (par->txbuf.len / 8) + 8,
+				  GFP_KERNEL);
+	if (!par->extra)
+		return -ENOMEM;
+	par->fbtftops.write = fbtft_write_spi_emulate_9;
+
+	return 0;
+}
+
+static int flexfb_set_spi_bus_func(struct fbtft_par *par,
+				   struct spi_device *sdev)
+{
+	struct device *dev = &sdev->dev;
+	int ret;
+
+	par->fbtftops.write = fbtft_write_spi;
+	switch (buswidth) {
+	case 8:
+		par->fbtftops.write_vmem = fbtft_write_vmem16_bus8;
+		if (!par->startbyte)
+			par->fbtftops.verify_gpios = flexfb_verify_gpios_dc;
+		break;
+	case 9:
+		if (regwidth == 16) {
+			dev_err(dev, "argument 'regwidth': %d is not supported with buswidth=%d and SPI.\n",
+				regwidth, buswidth);
+			return -EINVAL;
+		}
+		par->fbtftops.write_register = fbtft_write_reg8_bus9;
+		par->fbtftops.write_vmem = fbtft_write_vmem16_bus9;
+		sdev->bits_per_word = 9;
+		ret = sdev->master->setup(sdev);
+		if (ret) {
+			ret = flexfb_emulate_spi_8(par, sdev);
+			if (ret)
+				return ret;
+		}
+		break;
+	default:
+		dev_err(dev, "argument 'buswidth': %d is not supported with SPI.\n",
+			buswidth);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int flexfb_set_platform_bus_func(struct fbtft_par *par,
+					struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	par->fbtftops.verify_gpios = flexfb_verify_gpios_db;
+	switch (buswidth) {
+	case 8:
+		par->fbtftops.write = fbtft_write_gpio8_wr;
+		par->fbtftops.write_vmem = fbtft_write_vmem16_bus8;
+		break;
+	case 16:
+		par->fbtftops.write_register = fbtft_write_reg16_bus16;
+		if (latched)
+			par->fbtftops.write = fbtft_write_gpio16_wr_latched;
+		else
+			par->fbtftops.write = fbtft_write_gpio16_wr;
+		par->fbtftops.write_vmem = fbtft_write_vmem16_bus16;
+		break;
+	default:
+		dev_err(dev, "argument 'buswidth': %d is not supported with parallel.\n",
+			buswidth);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -436,66 +524,12 @@ static int flexfb_probe_common(struct spi_device *sdev,
 	if (ret)
 		goto out_release;
 
-	/* bus functions */
-	if (sdev) {
-		par->fbtftops.write = fbtft_write_spi;
-		switch (buswidth) {
-		case 8:
-			par->fbtftops.write_vmem = fbtft_write_vmem16_bus8;
-			if (!par->startbyte)
-				par->fbtftops.verify_gpios = flexfb_verify_gpios_dc;
-			break;
-		case 9:
-			if (regwidth == 16) {
-				dev_err(dev, "argument 'regwidth': %d is not supported with buswidth=%d and SPI.\n", regwidth, buswidth);
-				return -EINVAL;
-			}
-			par->fbtftops.write_register = fbtft_write_reg8_bus9;
-			par->fbtftops.write_vmem = fbtft_write_vmem16_bus9;
-			sdev->bits_per_word = 9;
-			ret = sdev->master->setup(sdev);
-			if (ret) {
-				dev_warn(dev,
-					"9-bit SPI not available, emulating using 8-bit.\n");
-				sdev->bits_per_word = 8;
-				ret = sdev->master->setup(sdev);
-				if (ret)
-					goto out_release;
-				/* allocate buffer with room for dc bits */
-				par->extra = devm_kzalloc(par->info->device,
-						par->txbuf.len + (par->txbuf.len / 8) + 8,
-						GFP_KERNEL);
-				if (!par->extra) {
-					ret = -ENOMEM;
-					goto out_release;
-				}
-				par->fbtftops.write = fbtft_write_spi_emulate_9;
-			}
-			break;
-		default:
-			dev_err(dev, "argument 'buswidth': %d is not supported with SPI.\n", buswidth);
-			return -EINVAL;
-		}
-	} else {
-		par->fbtftops.verify_gpios = flexfb_verify_gpios_db;
-		switch (buswidth) {
-		case 8:
-			par->fbtftops.write = fbtft_write_gpio8_wr;
-			par->fbtftops.write_vmem = fbtft_write_vmem16_bus8;
-			break;
-		case 16:
-			par->fbtftops.write_register = fbtft_write_reg16_bus16;
-			if (latched)
-				par->fbtftops.write = fbtft_write_gpio16_wr_latched;
-			else
-				par->fbtftops.write = fbtft_write_gpio16_wr;
-			par->fbtftops.write_vmem = fbtft_write_vmem16_bus16;
-			break;
-		default:
-			dev_err(dev, "argument 'buswidth': %d is not supported with parallel.\n", buswidth);
-			return -EINVAL;
-		}
-	}
+	if (sdev)
+		ret = flexfb_set_spi_bus_func(par, sdev);
+	else
+		ret = flexfb_set_platform_bus_func(par, pdev);
+	if (ret)
+		goto out_release;
 
 	/* set_addr_win function */
 	switch (setaddrwin) {
-- 
2.1.4

Re: [PATCH 05/10] Staging: fbtft: Set bus specific ops using separate functions

[Greg KH]

On Tue, Jun 30, 2015 at 08:43:12AM +0200, Fabio Falzoi wrote:
> Use two separate functions for spi and platform bus
> (flexfb_set_spi_bus_func and flexfb_set_platform_bus_func,
> respectively) to set the appropriate write operations.
> 
> This patch corrects the following checkpatch errors:
> WARNING:LONG_LiINE at lines 446, 450, 466, 476, 489 and 495.
> CHECK:PARENTHESIS_ALIGNMENT at line 459.
> 
> Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>

Can't you fix the line lengths without having to move to separate
functions?

thanks,

greg k-h

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

[Fabio Falzoi]

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 <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 47 +++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index 1b833f9..2a15da1 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -470,6 +470,31 @@ static int flexfb_set_platform_bus_func(struct fbtft_par *par,
 	return 0;
 }
 
+static int flexfb_set_addr_win_func(const struct device *dev,
+				    struct fbtft_par *par)
+{
+	switch (setaddrwin) {
+	case 0:
+		/* use default */
+		break;
+	case 1:
+		par->fbtftops.set_addr_win = flexfb_set_addr_win_1;
+		break;
+	case 2:
+		par->fbtftops.set_addr_win = flexfb_set_addr_win_2;
+		break;
+	case 3:
+		par->fbtftops.set_addr_win = set_addr_win_3;
+		break;
+	default:
+		dev_err(dev, "argument 'setaddrwin': unknown value %d.\n",
+			setaddrwin);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int flexfb_probe_common(struct spi_device *sdev,
 			       struct platform_device *pdev)
 {
@@ -531,25 +556,9 @@ static int flexfb_probe_common(struct spi_device *sdev,
 	if (ret)
 		goto out_release;
 
-	/* set_addr_win function */
-	switch (setaddrwin) {
-	case 0:
-		/* use default */
-		break;
-	case 1:
-		par->fbtftops.set_addr_win = flexfb_set_addr_win_1;
-		break;
-	case 2:
-		par->fbtftops.set_addr_win = flexfb_set_addr_win_2;
-		break;
-	case 3:
-		par->fbtftops.set_addr_win = set_addr_win_3;
-		break;
-	default:
-		dev_err(dev, "argument 'setaddrwin': unknown value %d.\n",
-			setaddrwin);
-		return -EINVAL;
-	}
+	ret = flexfb_set_addr_win_func(dev, par);
+	if (ret)
+		goto out_release;
 
 	if (!nobacklight)
 		par->fbtftops.register_backlight = fbtft_register_backlight;
-- 
2.1.4

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

[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 <fabio.falzoi84@gmail.com>
> ---
>  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...

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 <fabio.falzoi84@gmail.com>
>> ---
>>   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.

[PATCH 07/10] Staging: fbtft: Remove useless newline

[Fabio Falzoi]

No newline is needed since checkpatch doesn't complain about line longer
than 80 characters for string literals.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index 2a15da1..cccb0e4 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -290,8 +290,7 @@ static int flexfb_verify_gpios_dc(struct fbtft_par *par)
 	fbtft_par_dbg(DEBUG_VERIFY_GPIOS, par, "%s()\n", __func__);
 
 	if (par->gpio.dc < 0) {
-		dev_err(par->info->device,
-			"Missing info about 'dc' gpio. Aborting.\n");
+		dev_err(par->info->device, "Missing info about 'dc' gpio. Aborting.\n");
 		return -EINVAL;
 	}
 
-- 
2.1.4

Re: [PATCH 07/10] Staging: fbtft: Remove useless newline

[Dan Carpenter]

On Tue, Jun 30, 2015 at 08:43:14AM +0200, Fabio Falzoi wrote:
> No newline is needed since checkpatch doesn't complain about line longer
> than 80 characters for string literals.
> 
> Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>

The original was correct.  There was no need to go over 80 characters
just because of a limitation in checkpatch.

regards,
dan carpenter

Re: [PATCH 07/10] Staging: fbtft: Remove useless newline

[Joe Perches]

On Tue, 2015-06-30 at 10:42 +0300, Dan Carpenter wrote:
> On Tue, Jun 30, 2015 at 08:43:14AM +0200, Fabio Falzoi wrote:
> > No newline is needed since checkpatch doesn't complain about line longer
> > than 80 characters for string literals.
[]
> The original was correct.

The original was fine.

> There was no need to go over 80 characters
> just because of a limitation in checkpatch.

How is checkpatch not emitting a message on
either form a limitation?

Re: [PATCH 07/10] Staging: fbtft: Remove useless newline

[Dan Carpenter]

On Tue, Jun 30, 2015 at 07:55:42AM -0700, Joe Perches wrote:
> On Tue, 2015-06-30 at 10:42 +0300, Dan Carpenter wrote:
> > On Tue, Jun 30, 2015 at 08:43:14AM +0200, Fabio Falzoi wrote:
> > > No newline is needed since checkpatch doesn't complain about line longer
> > > than 80 characters for string literals.
> []
> > The original was correct.
> 
> The original was fine.
> 
> > There was no need to go over 80 characters
> > just because of a limitation in checkpatch.
> 
> How is checkpatch not emitting a message on
> either form a limitation?

It's not an easily solvable limitation, but the original was better than
the new patch.  This patch is all like "I've found a way to do something
bad and checkpatch.pl doesn't catch it so woohoo!"

regards,
dan carpenter

Re: [PATCH 07/10] Staging: fbtft: Remove useless newline

[Joe Perches]

On Tue, 2015-06-30 at 20:45 +0300, Dan Carpenter wrote:
> On Tue, Jun 30, 2015 at 07:55:42AM -0700, Joe Perches wrote:
> > On Tue, 2015-06-30 at 10:42 +0300, Dan Carpenter wrote:
> > > On Tue, Jun 30, 2015 at 08:43:14AM +0200, Fabio Falzoi wrote:
> > > > No newline is needed since checkpatch doesn't complain about line longer
> > > > than 80 characters for string literals.
> > []
> > > The original was correct.
> > 
> > The original was fine.
> > 
> > > There was no need to go over 80 characters
> > > just because of a limitation in checkpatch.
> > 
> > How is checkpatch not emitting a message on
> > either form a limitation?
> 
> It's not an easily solvable limitation,

It's not a limitation at all.

> but the original was better than
> the new patch.  This patch is all like "I've found a way to do something
> bad and checkpatch.pl doesn't catch it so woohoo!"

I think either form is fine, but because either is fine,
it's not useful/better to change either.

[PATCH 08/10] Staging: fbtft: Avoid duplicating code to check gpio.dc value

[Fabio Falzoi]

Avoid duplicating code to verify gpios.dc and call
flexfb_verify_gpios_dc instead.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index cccb0e4..ca14919 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -299,15 +299,14 @@ static int flexfb_verify_gpios_dc(struct fbtft_par *par)
 
 static int flexfb_verify_gpios_db(struct fbtft_par *par)
 {
-	int i;
+	int i, ret;
 	int num_db = buswidth;
 
 	fbtft_par_dbg(DEBUG_VERIFY_GPIOS, par, "%s()\n", __func__);
+	ret = flexfb_verify_gpios_dc(par);
+	if (ret)
+		return ret;
 
-	if (par->gpio.dc < 0) {
-		dev_err(par->info->device, "Missing info about 'dc' gpio. Aborting.\n");
-		return -EINVAL;
-	}
 	if (par->gpio.wr < 0) {
 		dev_err(par->info->device, "Missing info about 'wr' gpio. Aborting.\n");
 		return -EINVAL;
-- 
2.1.4

[PATCH 09/10] Staging: fbtft: Fix parenthesis alignment coding style issue

[Fabio Falzoi]

This patch fixes the following checkpatch.pl error:

CHECK:PARENTHESIS_ALIGNMENT at line 217.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index ca14919..b161050 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -213,9 +213,8 @@ static const struct flexfb_lcd_controller flexfb_chip_table[] = {
 static void flexfb_set_addr_win_1(struct fbtft_par *par,
 				  int xs, int ys, int xe, int ye)
 {
-	fbtft_par_dbg(DEBUG_SET_ADDR_WIN, par,
-		     "%s(xs=%d, ys=%d, xe=%d, ye=%d)\n",
-		     __func__, xs, ys, xe, ye);
+	fbtft_par_dbg(DEBUG_SET_ADDR_WIN, par, "%s(xs=%d, ys=%d, xe=%d, ye=%d)\n",
+		      __func__, xs, ys, xe, ye);
 	switch (par->info->var.rotate) {
 	/* R20h = Horizontal GRAM Start Address */
 	/* R21h = Vertical GRAM Start Address */
@@ -581,8 +580,8 @@ static int flexfb_remove_common(struct device *dev, struct fb_info *info)
 		return -EINVAL;
 	par = info->par;
 	if (par)
-		fbtft_par_dbg(DEBUG_DRIVER_INIT_FUNCTIONS, par,
-			"%s()\n", __func__);
+		fbtft_par_dbg(DEBUG_DRIVER_INIT_FUNCTIONS, par, "%s()\n",
+			      __func__);
 	fbtft_unregister_framebuffer(info);
 	fbtft_framebuffer_release(info);
 
-- 
2.1.4

[PATCH 10/10] Staging: fbtft: Fix spacing coding style issue

[Fabio Falzoi]

This patch fixes the following checkpatch.pl error:

CHECK:SPACING at line 318.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/fbtft/flexfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/flexfb.c b/drivers/staging/fbtft/flexfb.c
index b161050..6cd02b5 100644
--- a/drivers/staging/fbtft/flexfb.c
+++ b/drivers/staging/fbtft/flexfb.c
@@ -315,7 +315,7 @@ static int flexfb_verify_gpios_db(struct fbtft_par *par)
 		return -EINVAL;
 	}
 	if (latched)
-		num_db = buswidth/2;
+		num_db = buswidth / 2;
 	for (i = 0; i < num_db; i++) {
 		if (par->gpio.db[i] < 0) {
 			dev_err(par->info->device,
-- 
2.1.4