[PATCH 0/6] arm: sa1100: refactor h3xxx GPIO handling

[PATCH 0/6] arm: sa1100: refactor h3xxx GPIO handling

[Dmitry Eremin-Solenikov]

Linus,

I have been thinking abbout h3100/h3600 and GPIO issues.Could you please
review and test these patches. It is an attempt to cleanup and refactor GPIO
handling not to call gpio_request at init_machine time.

  git://github.com/lumag/linux h3xxx-gpios

for you to fetch changes up to c9027b88b6e79556599ca625d2c674704985474a:

  arm: sa1100: h3xxx: drop hand-coded gpio_request_array analogue (2013-11-21 01:51:02 +0400)

----------------------------------------------------------------
Dmitry Eremin-Solenikov (6):
      arm: sa1100: h3100: refactor LCD GPIO handling
      arm: sa1100: h3600: refactor LCD GPIO handling
      arm: sa1100: h3100: refactor IrDA GPIO handling
      arm: sa1100: h3600: refactor IrDA GPIO handling
      arm: sa1100: h3xxx: move serial port GPIO handling to common place
      arm: sa1100: h3xxx: drop hand-coded gpio_request_array analogue

 arch/arm/mach-sa1100/h3100.c              | 61 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 arch/arm/mach-sa1100/h3600.c              | 81 +++++++++++++++++++++++++++++++++------------------------------------------------
 arch/arm/mach-sa1100/h3xxx.c              | 58 +++++++++++++++++++++++++++-------------------------------
 arch/arm/mach-sa1100/include/mach/h3xxx.h | 11 -----------
 4 files changed, 104 insertions(+), 107 deletions(-)

[PATCH 1/6] arm: sa1100: h3100: refactor LCD GPIO handling

[Dmitry Eremin-Solenikov]

As GPIOs are going to move to platform device, there is no guarantee
that they will be available at init_machine time.

Request all GPIOs directly in lcd_power callback and not at init_machine
time.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3100.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3100.c b/arch/arm/mach-sa1100/h3100.c
index b8f2b15..5b78c9f 100644
--- a/arch/arm/mach-sa1100/h3100.c
+++ b/arch/arm/mach-sa1100/h3100.c
@@ -28,15 +28,35 @@
 /*
  * helper for sa1100fb
  */
+static struct gpio h3100_lcd_gpio[] = {
+	{ H3100_GPIO_LCD_3V_ON, GPIOF_OUT_INIT_LOW, "LCD 3V" },
+	{ H3XXX_EGPIO_LCD_ON, GPIOF_OUT_INIT_LOW, "LCD ON" },
+};
+
+static bool h3100_lcd_request(void)
+{
+	static bool h3100_lcd_ok;
+	int rc;
+
+	if (h3100_lcd_ok)
+		return true;
+
+	rc = gpio_request_array(h3100_lcd_gpio, ARRAY_SIZE(h3100_lcd_gpio));
+	if (rc)
+		pr_err("%s: can't request GPIOs\n", __func__);
+	else
+		h3100_lcd_ok = true;
+
+	return h3100_lcd_ok;
+}
+
 static void h3100_lcd_power(int enable)
 {
-	if (!gpio_request(H3XXX_EGPIO_LCD_ON, "LCD ON")) {
-		gpio_set_value(H3100_GPIO_LCD_3V_ON, enable);
-		gpio_direction_output(H3XXX_EGPIO_LCD_ON, enable);
-		gpio_free(H3XXX_EGPIO_LCD_ON);
-	} else {
-		pr_err("%s: can't request H3XXX_EGPIO_LCD_ON\n", __func__);
-	}
+	if (!h3100_lcd_request())
+		return;
+
+	gpio_set_value(H3100_GPIO_LCD_3V_ON, enable);
+	gpio_set_value(H3XXX_EGPIO_LCD_ON, enable);
 }
 
 static struct sa1100fb_mach_info h3100_lcd_info = {
@@ -91,7 +111,6 @@ static struct gpio_default_state h3100_default_gpio[] = {
 	{ H3XXX_GPIO_COM_DCD,	GPIO_MODE_IN,	"COM DCD" },
 	{ H3XXX_GPIO_COM_CTS,	GPIO_MODE_IN,	"COM CTS" },
 	{ H3XXX_GPIO_COM_RTS,	GPIO_MODE_OUT0,	"COM RTS" },
-	{ H3100_GPIO_LCD_3V_ON,	GPIO_MODE_OUT0,	"LCD 3v" },
 };
 
 static void __init h3100_mach_init(void)
-- 
1.8.4.2

[PATCH 2/6] arm: sa1100: h3600: refactor LCD GPIO handling

[Dmitry Eremin-Solenikov]

Use gpio_request_array to request all GPIOs at once. Also don't call
gpio_free. There is little point freeing LCD gpios once they are
requested. Instead guard them with bool variable.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3600.c | 48 ++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3600.c b/arch/arm/mach-sa1100/h3600.c
index b8dc5bd..559c2a0 100644
--- a/arch/arm/mach-sa1100/h3600.c
+++ b/arch/arm/mach-sa1100/h3600.c
@@ -28,35 +28,39 @@
 /*
  * helper for sa1100fb
  */
+static struct gpio h3600_lcd_gpio[] = {
+	{ H3XXX_EGPIO_LCD_ON,	GPIOF_OUT_INIT_LOW,	"LCD power" },
+	{ H3600_EGPIO_LCD_PCI,	GPIOF_OUT_INIT_LOW,	"LCD control" },
+	{ H3600_EGPIO_LCD_5V_ON, GPIOF_OUT_INIT_LOW,	"LCD 5v" },
+	{ H3600_EGPIO_LVDD_ON,	GPIOF_OUT_INIT_LOW,	"LCD 9v/-6.5v" },
+};
+
+static bool h3600_lcd_request(void)
+{
+	static bool h3600_lcd_ok;
+	int rc;
+
+	if (h3600_lcd_ok)
+		return true;
+
+	rc = gpio_request_array(h3600_lcd_gpio, ARRAY_SIZE(h3600_lcd_gpio));
+	if (rc)
+		pr_err("%s: can't request GPIOs\n", __func__);
+	else
+		h3600_lcd_ok = true;
+
+	return h3600_lcd_ok;
+}
+
 static void h3600_lcd_power(int enable)
 {
-	if (gpio_request(H3XXX_EGPIO_LCD_ON, "LCD power")) {
-		pr_err("%s: can't request H3XXX_EGPIO_LCD_ON\n", __func__);
-		goto err1;
-	}
-	if (gpio_request(H3600_EGPIO_LCD_PCI, "LCD control")) {
-		pr_err("%s: can't request H3XXX_EGPIO_LCD_PCI\n", __func__);
-		goto err2;
-	}
-	if (gpio_request(H3600_EGPIO_LCD_5V_ON, "LCD 5v")) {
-		pr_err("%s: can't request H3XXX_EGPIO_LCD_5V_ON\n", __func__);
-		goto err3;
-	}
-	if (gpio_request(H3600_EGPIO_LVDD_ON, "LCD 9v/-6.5v")) {
-		pr_err("%s: can't request H3600_EGPIO_LVDD_ON\n", __func__);
-		goto err4;
-	}
+	if (!h3600_lcd_request())
+		return;
 
 	gpio_direction_output(H3XXX_EGPIO_LCD_ON, enable);
 	gpio_direction_output(H3600_EGPIO_LCD_PCI, enable);
 	gpio_direction_output(H3600_EGPIO_LCD_5V_ON, enable);
 	gpio_direction_output(H3600_EGPIO_LVDD_ON, enable);
-
-	gpio_free(H3600_EGPIO_LVDD_ON);
-err4:	gpio_free(H3600_EGPIO_LCD_5V_ON);
-err3:	gpio_free(H3600_EGPIO_LCD_PCI);
-err2:	gpio_free(H3XXX_EGPIO_LCD_ON);
-err1:	return;
 }
 
 static const struct sa1100fb_rgb h3600_rgb_16 = {
-- 
1.8.4.2

[PATCH 3/6] arm: sa1100: h3100: refactor IrDA GPIO handling

[Dmitry Eremin-Solenikov]

As GPIOs are going to move to platform device, there is no guarantee
that they will be available at init_machine time.

Request and free all GPIOs from IrDA startup/shutdown callbacks and not
at init_machine time.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3100.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3100.c b/arch/arm/mach-sa1100/h3100.c
index 5b78c9f..daa27c4 100644
--- a/arch/arm/mach-sa1100/h3100.c
+++ b/arch/arm/mach-sa1100/h3100.c
@@ -89,6 +89,11 @@ static void __init h3100_map_io(void)
 /*
  * This turns the IRDA power on or off on the Compaq H3100
  */
+static struct gpio h3100_irda_gpio[] = {
+	{ H3100_GPIO_IR_ON,	GPIOF_OUT_INIT_LOW, "IrDA power" },
+	{ H3100_GPIO_IR_FSEL,	GPIOF_OUT_INIT_LOW, "IrDA fsel" },
+};
+
 static int h3100_irda_set_power(struct device *dev, unsigned int state)
 {
 	gpio_set_value(H3100_GPIO_IR_ON, state);
@@ -100,14 +105,24 @@ static void h3100_irda_set_speed(struct device *dev, unsigned int speed)
 	gpio_set_value(H3100_GPIO_IR_FSEL, !(speed < 4000000));
 }
 
+static int h3100_irda_startup(struct device *dev)
+{
+	return gpio_request_array(h3100_irda_gpio, sizeof(h3100_irda_gpio));
+}
+
+static void h3100_irda_shutdown(struct device *dev)
+{
+	return gpio_free_array(h3100_irda_gpio, sizeof(h3100_irda_gpio));
+}
+
 static struct irda_platform_data h3100_irda_data = {
 	.set_power	= h3100_irda_set_power,
 	.set_speed	= h3100_irda_set_speed,
+	.startup	= h3100_irda_startup,
+	.shutdown	= h3100_irda_shutdown,
 };
 
 static struct gpio_default_state h3100_default_gpio[] = {
-	{ H3100_GPIO_IR_ON,	GPIO_MODE_OUT0, "IrDA power" },
-	{ H3100_GPIO_IR_FSEL,	GPIO_MODE_OUT0, "IrDA fsel" },
 	{ H3XXX_GPIO_COM_DCD,	GPIO_MODE_IN,	"COM DCD" },
 	{ H3XXX_GPIO_COM_CTS,	GPIO_MODE_IN,	"COM CTS" },
 	{ H3XXX_GPIO_COM_RTS,	GPIO_MODE_OUT0,	"COM RTS" },
-- 
1.8.4.2

[PATCH 4/6] arm: sa1100: h3600: refactor IrDA GPIO handling

[Dmitry Eremin-Solenikov]

Use gpio_request_array/gpio_free_array to request all GPIOs at once.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3600.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3600.c b/arch/arm/mach-sa1100/h3600.c
index 559c2a0..a663e72 100644
--- a/arch/arm/mach-sa1100/h3600.c
+++ b/arch/arm/mach-sa1100/h3600.c
@@ -97,6 +97,11 @@ static void __init h3600_map_io(void)
 /*
  * This turns the IRDA power on or off on the Compaq H3600
  */
+static struct gpio h3600_irda_gpio[] = {
+	{ H3600_EGPIO_IR_ON,	GPIOF_OUT_INIT_LOW, "IrDA power" },
+	{ H3600_EGPIO_IR_FSEL,	GPIOF_OUT_INIT_LOW, "IrDA fsel" },
+};
+
 static int h3600_irda_set_power(struct device *dev, unsigned int state)
 {
 	gpio_set_value(H3600_EGPIO_IR_ON, state);
@@ -110,29 +115,12 @@ static void h3600_irda_set_speed(struct device *dev, unsigned int speed)
 
 static int h3600_irda_startup(struct device *dev)
 {
-	int err = gpio_request(H3600_EGPIO_IR_ON, "IrDA power");
-	if (err)
-		goto err1;
-	err = gpio_direction_output(H3600_EGPIO_IR_ON, 0);
-	if (err)
-		goto err2;
-	err = gpio_request(H3600_EGPIO_IR_FSEL, "IrDA fsel");
-	if (err)
-		goto err2;
-	err = gpio_direction_output(H3600_EGPIO_IR_FSEL, 0);
-	if (err)
-		goto err3;
-	return 0;
-
-err3:	gpio_free(H3600_EGPIO_IR_FSEL);
-err2:	gpio_free(H3600_EGPIO_IR_ON);
-err1:	return err;
+	return gpio_request_array(h3600_irda_gpio, sizeof(h3600_irda_gpio));
 }
 
 static void h3600_irda_shutdown(struct device *dev)
 {
-	gpio_free(H3600_EGPIO_IR_ON);
-	gpio_free(H3600_EGPIO_IR_FSEL);
+	return gpio_free_array(h3600_irda_gpio, sizeof(h3600_irda_gpio));
 }
 
 static struct irda_platform_data h3600_irda_data = {
-- 
1.8.4.2

[PATCH 5/6] arm: sa1100: h3xxx: move serial port GPIO handling to common place

[Dmitry Eremin-Solenikov]

Both h3100 and h3600 request UART gpios during init_machine time. As
sa1100 gpio driver is going to become proper machine driver, move gpio
handling to UART port functions. Request all gpios using gpio_request
array once and then guard them from rerequesting with bool variable.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3100.c |  7 -------
 arch/arm/mach-sa1100/h3600.c |  7 -------
 arch/arm/mach-sa1100/h3xxx.c | 27 +++++++++++++++++++++++++++
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3100.c b/arch/arm/mach-sa1100/h3100.c
index daa27c4..3c43219 100644
--- a/arch/arm/mach-sa1100/h3100.c
+++ b/arch/arm/mach-sa1100/h3100.c
@@ -122,15 +122,8 @@ static struct irda_platform_data h3100_irda_data = {
 	.shutdown	= h3100_irda_shutdown,
 };
 
-static struct gpio_default_state h3100_default_gpio[] = {
-	{ H3XXX_GPIO_COM_DCD,	GPIO_MODE_IN,	"COM DCD" },
-	{ H3XXX_GPIO_COM_CTS,	GPIO_MODE_IN,	"COM CTS" },
-	{ H3XXX_GPIO_COM_RTS,	GPIO_MODE_OUT0,	"COM RTS" },
-};
-
 static void __init h3100_mach_init(void)
 {
-	h3xxx_init_gpio(h3100_default_gpio, ARRAY_SIZE(h3100_default_gpio));
 	h3xxx_mach_init();
 
 	sa11x0_register_lcd(&h3100_lcd_info);
diff --git a/arch/arm/mach-sa1100/h3600.c b/arch/arm/mach-sa1100/h3600.c
index a663e72..5be54c2 100644
--- a/arch/arm/mach-sa1100/h3600.c
+++ b/arch/arm/mach-sa1100/h3600.c
@@ -130,15 +130,8 @@ static struct irda_platform_data h3600_irda_data = {
 	.shutdown	= h3600_irda_shutdown,
 };
 
-static struct gpio_default_state h3600_default_gpio[] = {
-	{ H3XXX_GPIO_COM_DCD,	GPIO_MODE_IN,	"COM DCD" },
-	{ H3XXX_GPIO_COM_CTS,	GPIO_MODE_IN,	"COM CTS" },
-	{ H3XXX_GPIO_COM_RTS,	GPIO_MODE_OUT0,	"COM RTS" },
-};
-
 static void __init h3600_mach_init(void)
 {
-	h3xxx_init_gpio(h3600_default_gpio, ARRAY_SIZE(h3600_default_gpio));
 	h3xxx_mach_init();
 
 	sa11x0_register_lcd(&h3600_lcd_info);
diff --git a/arch/arm/mach-sa1100/h3xxx.c b/arch/arm/mach-sa1100/h3xxx.c
index f17e738..82e8702 100644
--- a/arch/arm/mach-sa1100/h3xxx.c
+++ b/arch/arm/mach-sa1100/h3xxx.c
@@ -116,9 +116,34 @@ static struct resource h3xxx_flash_resource =
 /*
  * H3xxx uart support
  */
+static struct gpio h3xxx_uart_gpio[] = {
+	{ H3XXX_GPIO_COM_DCD,	GPIOF_IN,		"COM DCD" },
+	{ H3XXX_GPIO_COM_CTS,	GPIOF_IN,		"COM CTS" },
+	{ H3XXX_GPIO_COM_RTS,	GPIOF_OUT_INIT_LOW,	"COM RTS" },
+};
+
+static bool h3xxx_uart_request_gpios(void)
+{
+	static bool h3xxx_uart_gpio_ok;
+	int rc;
+
+	if (h3xxx_uart_gpio_ok)
+		return true;
+
+	rc = gpio_request_array(h3xxx_uart_gpio, ARRAY_SIZE(h3xxx_uart_gpio));
+	if (rc)
+		pr_err("h3xxx_uart_request_gpios: error %d\n", rc);
+	else
+		h3xxx_uart_gpio_ok = true;
+
+	return h3xxx_uart_gpio_ok;
+}
+
 static void h3xxx_uart_set_mctrl(struct uart_port *port, u_int mctrl)
 {
 	if (port->mapbase == _Ser3UTCR0) {
+		if (!h3xxx_uart_request_gpios())
+			return;
 		gpio_set_value(H3XXX_GPIO_COM_RTS, !(mctrl & TIOCM_RTS));
 	}
 }
@@ -128,6 +153,8 @@ static u_int h3xxx_uart_get_mctrl(struct uart_port *port)
 	u_int ret = TIOCM_CD | TIOCM_CTS | TIOCM_DSR;
 
 	if (port->mapbase == _Ser3UTCR0) {
+		if (!h3xxx_uart_request_gpios())
+			return ret;
 		/*
 		 * DCD and CTS bits are inverted in GPLR by RS232 transceiver
 		 */
-- 
1.8.4.2

[PATCH 6/6] arm: sa1100: h3xxx: drop hand-coded gpio_request_array analogue

[Dmitry Eremin-Solenikov]

h3xxx_init_gpio() behaves alsmost like gpio_request_array. Also after
all GPIO refactoring it is unused. Drop it now.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/arm/mach-sa1100/h3xxx.c              | 31 -------------------------------
 arch/arm/mach-sa1100/include/mach/h3xxx.h | 11 -----------
 2 files changed, 42 deletions(-)

diff --git a/arch/arm/mach-sa1100/h3xxx.c b/arch/arm/mach-sa1100/h3xxx.c
index 82e8702..c79bf46 100644
--- a/arch/arm/mach-sa1100/h3xxx.c
+++ b/arch/arm/mach-sa1100/h3xxx.c
@@ -28,37 +28,6 @@
 
 #include "generic.h"
 
-void h3xxx_init_gpio(struct gpio_default_state *s, size_t n)
-{
-	while (n--) {
-		const char *name = s->name;
-		int err;
-
-		if (!name)
-			name = "[init]";
-		err = gpio_request(s->gpio, name);
-		if (err) {
-			printk(KERN_ERR "gpio%u: unable to request: %d\n",
-				s->gpio, err);
-			continue;
-		}
-		if (s->mode >= 0) {
-			err = gpio_direction_output(s->gpio, s->mode);
-		} else {
-			err = gpio_direction_input(s->gpio);
-		}
-		if (err) {
-			printk(KERN_ERR "gpio%u: unable to set direction: %d\n",
-				s->gpio, err);
-			continue;
-		}
-		if (!s->name)
-			gpio_free(s->gpio);
-		s++;
-	}
-}
-
-
 /*
  * H3xxx flash support
  */
diff --git a/arch/arm/mach-sa1100/include/mach/h3xxx.h b/arch/arm/mach-sa1100/include/mach/h3xxx.h
index c810620..603d434 100644
--- a/arch/arm/mach-sa1100/include/mach/h3xxx.h
+++ b/arch/arm/mach-sa1100/include/mach/h3xxx.h
@@ -79,17 +79,6 @@
 #define H3600_EGPIO_LCD_5V_ON		(H3XXX_EGPIO_BASE + 14) /* enable 5V to LCD. active high. */
 #define H3600_EGPIO_LVDD_ON		(H3XXX_EGPIO_BASE + 15) /* enable 9V and -6.5V to LCD. */
 
-struct gpio_default_state {
-	int gpio;
-	int mode;
-	const char *name;
-};
-
-#define GPIO_MODE_IN	-1
-#define GPIO_MODE_OUT0	0
-#define GPIO_MODE_OUT1	1
-
-void h3xxx_init_gpio(struct gpio_default_state *s, size_t n);
 void __init h3xxx_map_io(void);
 void __init h3xxx_mach_init(void);
 
-- 
1.8.4.2

[PATCH 0/6] arm: sa1100: refactor h3xxx GPIO handling

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> I have been thinking abbout h3100/h3600 and GPIO issues.Could you please
> review and test these patches. It is an attempt to cleanup and refactor GPIO
> handling not to call gpio_request at init_machine time.

Tested on the h3600 and works flawlessly!

Tested-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 1/6] arm: sa1100: h3100: refactor LCD GPIO handling

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> +static bool h3100_lcd_request(void)
> +{
> +       static bool h3100_lcd_ok;
> +       int rc;
> +
> +       if (h3100_lcd_ok)
> +               return true;
> +
> +       rc = gpio_request_array(h3100_lcd_gpio, ARRAY_SIZE(h3100_lcd_gpio));
> +       if (rc)
> +               pr_err("%s: can't request GPIOs\n", __func__);
> +       else
> +               h3100_lcd_ok = true;
> +
> +       return h3100_lcd_ok;
> +}

Hm hm this design pattern is somewhat strange, a run-once
construct, but OK then.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/6] arm: sa1100: h3600: refactor LCD GPIO handling

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> Use gpio_request_array to request all GPIOs at once. Also don't call
> gpio_free. There is little point freeing LCD gpios once they are
> requested. Instead guard them with bool variable.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 3/6] arm: sa1100: h3100: refactor IrDA GPIO handling

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> As GPIOs are going to move to platform device, there is no guarantee
> that they will be available at init_machine time.
>
> Request and free all GPIOs from IrDA startup/shutdown callbacks and not
> at init_machine time.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 4/6] arm: sa1100: h3600: refactor IrDA GPIO handling

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> Use gpio_request_array/gpio_free_array to request all GPIOs at once.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 5/6] arm: sa1100: h3xxx: move serial port GPIO handling to common place

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> Both h3100 and h3600 request UART gpios during init_machine time. As
> sa1100 gpio driver is going to become proper machine driver, move gpio
> handling to UART port functions. Request all gpios using gpio_request
> array once and then guard them from rerequesting with bool variable.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 6/6] arm: sa1100: h3xxx: drop hand-coded gpio_request_array analogue

[Linus Walleij]

On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:

> h3xxx_init_gpio() behaves alsmost like gpio_request_array. Also after

speling.

> all GPIO refactoring it is unused. Drop it now.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Please proceed to submit these to Russell by the patch tracker
or pull request.

Yours,
Linus Walleij

[PATCH 1/6] arm: sa1100: h3100: refactor LCD GPIO handling

[Dmitry Eremin-Solenikov]

On Tue, Nov 26, 2013 at 12:52 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>
>> +static bool h3100_lcd_request(void)
>> +{
>> +       static bool h3100_lcd_ok;
>> +       int rc;
>> +
>> +       if (h3100_lcd_ok)
>> +               return true;
>> +
>> +       rc = gpio_request_array(h3100_lcd_gpio, ARRAY_SIZE(h3100_lcd_gpio));
>> +       if (rc)
>> +               pr_err("%s: can't request GPIOs\n", __func__);
>> +       else
>> +               h3100_lcd_ok = true;
>> +
>> +       return h3100_lcd_ok;
>> +}
>
> Hm hm this design pattern is somewhat strange, a run-once
> construct, but OK then.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Another possibility would be to extend sa11x0-fb driver to also
provide lcd_init and lcd_shutdown callbacks (to request GPIOs).
Would it be better to do so?

It is really unfortunate that lcd_class drivers are not directly controlled
from fbdev drivers. Hopefully CDF can solve that.

-- 
With best wishes
Dmitry

[PATCH 0/6] arm: sa1100: refactor h3xxx GPIO handling

[Dmitry Eremin-Solenikov]

Hello,

On Tue, Nov 26, 2013 at 12:49 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>
>> I have been thinking abbout h3100/h3600 and GPIO issues.Could you please
>> review and test these patches. It is an attempt to cleanup and refactor GPIO
>> handling not to call gpio_request at init_machine time.
>
> Tested on the h3600 and works flawlessly!
>
> Tested-by: Linus Walleij <linus.walleij@linaro.org>

Thank you!



-- 
With best wishes
Dmitry

[PATCH 1/6] arm: sa1100: h3100: refactor LCD GPIO handling

[Linus Walleij]

On Tue, Nov 26, 2013 at 10:07 AM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> On Tue, Nov 26, 2013 at 12:52 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>> On Thu, Nov 21, 2013 at 4:40 PM, Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com> wrote:
>>
>>> +static bool h3100_lcd_request(void)
>>> +{
>>> +       static bool h3100_lcd_ok;
>>> +       int rc;
>>> +
>>> +       if (h3100_lcd_ok)
>>> +               return true;
>>> +
>>> +       rc = gpio_request_array(h3100_lcd_gpio, ARRAY_SIZE(h3100_lcd_gpio));
>>> +       if (rc)
>>> +               pr_err("%s: can't request GPIOs\n", __func__);
>>> +       else
>>> +               h3100_lcd_ok = true;
>>> +
>>> +       return h3100_lcd_ok;
>>> +}
>>
>> Hm hm this design pattern is somewhat strange, a run-once
>> construct, but OK then.
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Another possibility would be to extend sa11x0-fb driver to also
> provide lcd_init and lcd_shutdown callbacks (to request GPIOs).
> Would it be better to do so?
>
> It is really unfortunate that lcd_class drivers are not directly controlled
> from fbdev drivers. Hopefully CDF can solve that.

Hm it looks like there are no good solutions as long as LCDs
are no separate devices. :-(

Nominally you would use the new gpiod* interface and tie the
GPIOs to a struct device * in this case the LCD, and since that
currently has no struct device, this is OK for now...

Yours,
Linus Walleij