[PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables

Scope of the change is limited to GPIO pins used by board specific
device drivers which will be updated by follow-up patches of the
series. Those are some OMAP GPIO (gpio-0-15) and most of Amstrad Delta
latch2 GPIO bank pins. Remaining pins of those banks, as well as
Amstrad Delta latch1 pins, will be addressed later.

Assign a label ("latch2") to the bank, enumerate its pins and put that
information, together with OMAP GPIO bank pins, in GPIO lookup tables.
Assign lookup tables to devices as soon as those devices are registered
and their names can be obtained.

A step froward in:
- removal of hard-coded GPIO numbers from drivers,
- removal of board mach includes from drivers,
- switching to dynamically assigned GPIO numbers.

Created and compile tested agains linux-4.17-rc3

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c | 102 ++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 52e8e53ca154..4b78e73f8bf7 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c

@@ -12,6 +12,7 @@
  * published by the Free Software Foundation.
  */
 #include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
 #include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/init.h>

@@ -202,7 +203,10 @@ static struct resource latch2_resources[] = {
 	},
 };
 
+#define LATCH2_LABEL	"latch2"
+
 static struct bgpio_pdata latch2_pdata = {
+	.label	= LATCH2_LABEL,
 	.base	= AMS_DELTA_LATCH2_GPIO_BASE,
 	.ngpio	= AMS_DELTA_LATCH2_NGPIO,
 };

@@ -217,6 +221,23 @@ static struct platform_device latch2_gpio_device = {
 	},
 };
 
+#define LATCH2_PIN_LCD_VBLEN		0
+#define LATCH2_PIN_LCD_NDISP		1
+#define LATCH2_PIN_NAND_NCE		2
+#define LATCH2_PIN_NAND_NRE		3
+#define LATCH2_PIN_NAND_NWP		4
+#define LATCH2_PIN_NAND_NWE		5
+#define LATCH2_PIN_NAND_ALE		6
+#define LATCH2_PIN_NAND_CLE		7
+#define LATCH2_PIN_KEYBRD_PWR		8
+#define LATCH2_PIN_KEYBRD_DATAOUT	9
+#define LATCH2_PIN_SCARD_RSTIN		10
+#define LATCH2_PIN_SCARD_CMDVCC		11
+#define LATCH2_PIN_MODEM_NRESET		12
+#define LATCH2_PIN_MODEM_CODEC		13
+#define LATCH2_PIN_HOOKFLASH1		14
+#define LATCH2_PIN_HOOKFLASH2		15
+
 static const struct gpio latch_gpios[] __initconst = {
 	{
 		.gpio	= LATCH1_GPIO_BASE + 6,

@@ -323,6 +344,22 @@ static struct platform_device ams_delta_nand_device = {
 	.resource	= ams_delta_nand_resources,
 };
 
+#define OMAP_GPIO_LABEL	"gpio-0-15"
+
+static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_NAND_RB, "rdy",
+			    0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NCE, "nce", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NRE, "nre", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWP, "nwp", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
+		{ },
+	},
+};
+
 static struct resource ams_delta_kp_resources[] = {
 	[0] = {
 		.start	= INT_KEYBOARD,

@@ -358,6 +395,14 @@ static struct platform_device ams_delta_lcd_device = {
 	.id	= -1,
 };
 
+static struct gpiod_lookup_table ams_delta_lcd_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_LCD_VBLEN, "vblen", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_LCD_NDISP, "ndisp", 0),
+		{ },
+	},
+};
+
 static const struct gpio_led gpio_leds[] __initconst = {
 	{
 		.name		 = "camera",

@@ -449,11 +494,35 @@ static struct platform_device ams_delta_audio_device = {
 	.id     = -1,
 };
 
+static struct gpiod_lookup_table ams_delta_audio_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_HOOK_SWITCH,
+			    "hook_switch", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_MODEM_CODEC,
+			    "modem_codec", 0),
+		{ },
+	},
+};
+
 static struct platform_device cx20442_codec_device = {
 	.name   = "cx20442-codec",
 	.id     = -1,
 };
 
+static struct gpiod_lookup_table ams_delta_serio_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
+			    "data", 0),
+		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
+			    "clock", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR,
+			    "power", 0),
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT,
+			    "dataout", 0),
+		{ },
+	},
+};
+
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,

@@ -468,6 +537,16 @@ static struct platform_device *late_devices[] __initdata = {
 	&cx20442_codec_device,
 };
 
+static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
+	&ams_delta_audio_gpio_table,
+	&ams_delta_serio_gpio_table,
+};
+
+static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
+	&ams_delta_lcd_gpio_table,
+	&ams_delta_nand_gpio_table,
+};
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */

@@ -500,6 +579,20 @@ static void __init ams_delta_init(void)
 	gpio_led_register_device(-1, &leds_pdata);
 	platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
 
+	/*
+	 * As soon as devices have been registered, assign their dev_names
+	 * to respective GPIO lookup tables before they are added.
+	 */
+	ams_delta_audio_gpio_table.dev_id +			dev_name(&ams_delta_audio_device.dev);
+	/*
+	 * No device name is assigned to GPIO lookup table for serio device
+	 * as long as serio driver is not converted to platform device driver.
+	 */
+
+	gpiod_add_lookup_tables(ams_delta_gpio_tables,
+				ARRAY_SIZE(ams_delta_gpio_tables));
+
 	ams_delta_init_fiq();
 
 	omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1);

@@ -570,6 +663,15 @@ static int __init late_init(void)
 
 	platform_add_devices(late_devices, ARRAY_SIZE(late_devices));
 
+	/*
+	 * As soon as devices have been registered, assign their dev_names
+	 * to respective GPIO lookup tables before they are added.
+	 */
+	ams_delta_lcd_gpio_table.dev_id = dev_name(&ams_delta_lcd_device.dev);
+	ams_delta_nand_gpio_table.dev_id = dev_name(&ams_delta_nand_device.dev);
+
+	gpiod_add_lookup_tables(late_gpio_tables, ARRAY_SIZE(late_gpio_tables));
+
 	err = platform_device_register(&modem_nreset_device);
 	if (err) {
 		pr_err("Couldn't register the modem regulator device\n");

-- 
2.16.1

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents.

Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.

Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/input/serio/ams_delta_serio.c | 98 +++++++++++++++++++----------------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 3df501c3421b..dd1f8f118c08 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c

@@ -20,14 +20,13 @@
  * However, when used with the E3 mailboard that producecs non-standard
  * scancodes, a custom key table must be prepared and loaded from userspace.
  */
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/irq.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
 #include <asm/mach-types.h>
-#include <mach/board-ams-delta.h>
 
 #include <mach/ams-delta-fiq.h>
 

@@ -36,6 +35,10 @@ MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
 MODULE_LICENSE("GPL");
 
 static struct serio *ams_delta_serio;
+static struct gpio_desc *gpiod_data;
+static struct gpio_desc *gpiod_clock;
+static struct gpio_desc *gpiod_power;
+static struct gpio_desc *gpiod_dataout;
 
 static int check_data(int data)
 {

@@ -92,7 +95,7 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 static int ams_delta_serio_open(struct serio *serio)
 {
 	/* enable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
+	gpiod_set_value(gpiod_power, 1);
 
 	return 0;
 }

@@ -100,32 +103,9 @@ static int ams_delta_serio_open(struct serio *serio)
 static void ams_delta_serio_close(struct serio *serio)
 {
 	/* disable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
+	gpiod_set_value(gpiod_power, 0);
 }
 
-static const struct gpio ams_delta_gpios[] __initconst_or_module = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-data",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-clock",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-power",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-dataout",
-	},
-};
-
 static int __init ams_delta_serio_init(void)
 {
 	int err;

@@ -145,36 +125,62 @@ static int __init ams_delta_serio_init(void)
 	strlcpy(ams_delta_serio->phys, "GPIO/serio0",
 			sizeof(ams_delta_serio->phys));
 
-	err = gpio_request_array(ams_delta_gpios,
-				ARRAY_SIZE(ams_delta_gpios));
-	if (err) {
-		pr_err("ams_delta_serio: Couldn't request gpio pins\n");
+	gpiod_data = gpiod_get(NULL, "data", GPIOD_IN);
+	if (IS_ERR(gpiod_data)) {
+		err = PTR_ERR(gpiod_data);
+		pr_err("%s: 'data' GPIO request failed (%d)\n", __func__,
+		       err);
 		goto serio;
 	}
+	gpiod_clock = gpiod_get(NULL, "clock", GPIOD_IN);
+	if (IS_ERR(gpiod_clock)) {
+		err = PTR_ERR(gpiod_clock);
+		pr_err("%s: 'clock' GPIO request failed (%d)\n", __func__,
+		       err);
+		goto gpio_data;
+	}
+	gpiod_power = gpiod_get(NULL, "power", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_power)) {
+		err = PTR_ERR(gpiod_power);
+		pr_err("%s: 'power' GPIO request failed (%d)\n", __func__,
+		       err);
+		goto gpio_clock;
+	}
+	gpiod_dataout = gpiod_get(NULL, "dataout", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_dataout)) {
+		err = PTR_ERR(gpiod_dataout);
+		pr_err("%s: 'dataout' GPIO request failed (%d)\n",
+		       __func__, err);
+		goto gpio_power;
+	}
 
-	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
-			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			"ams-delta-serio", 0);
+	err = request_irq(gpiod_to_irq(gpiod_clock),
+			  ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
+			  "ams-delta-serio", 0);
 	if (err < 0) {
-		pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
-				gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
-		goto gpio;
+		pr_err("%s: 'clock' GPIO interrupt request failed (%d)\n",
+		       __func__, err);
+		goto gpio_dataout;
 	}
 	/*
 	 * Since GPIO register handling for keyboard clock pin is performed
 	 * at FIQ level, switch back from edge to simple interrupt handler
 	 * to avoid bad interaction.
 	 */
-	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
-			handle_simple_irq);
+	irq_set_handler(gpiod_to_irq(gpiod_clock), handle_simple_irq);
 
 	serio_register_port(ams_delta_serio);
 	dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
 
 	return 0;
-gpio:
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
+gpio_dataout:
+	gpiod_put(gpiod_dataout);
+gpio_power:
+	gpiod_put(gpiod_power);
+gpio_clock:
+	gpiod_put(gpiod_clock);
+gpio_data:
+	gpiod_put(gpiod_data);
 serio:
 	kfree(ams_delta_serio);
 	return err;

@@ -184,8 +190,10 @@ module_init(ams_delta_serio_init);
 static void __exit ams_delta_serio_exit(void)
 {
 	serio_unregister_port(ams_delta_serio);
-	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
+	free_irq(gpiod_to_irq(gpiod_clock), 0);
+	gpiod_put(gpiod_dataout);
+	gpiod_put(gpiod_power);
+	gpiod_put(gpiod_clock);
+	gpiod_put(gpiod_data);
 }
 module_exit(ams_delta_serio_exit);

-- 
2.16.1

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

The card uses two pins, one for jack and the other for voice modem
codec DAI control.

For jack pin, remove hardcoded GPIO number and use GPIO descriptor
based variant of jack GPIO initialization.

For modem_codec pin, declare static variable for storing its GPIO
descriptor, obtain it on card initialization and replace obsolete
ams_delta_latch2_write() with gpiod_set_value().  For that to work,
don't request the modem_codec pin from the board init code anymore.

If the modem_codec GPIO lookup fails, skip initialization of
functionality of the card which depends on its availability.

Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.

Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c |  5 -----
 sound/soc/omap/ams-delta.c            | 38 +++++++++++++++++++----------------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 4b78e73f8bf7..80f54cb54276 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c

@@ -259,11 +259,6 @@ static const struct gpio latch_gpios[] __initconst = {
 		.flags	= GPIOF_OUT_INIT_LOW,
 		.label	= "scard_cmdvcc",
 	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_MODEM_CODEC,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "modem_codec",
-	},
 	{
 		.gpio	= AMS_DELTA_LATCH2_GPIO_BASE + 14,
 		.flags	= GPIOF_OUT_INIT_LOW,

diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 77a30f0f0c96..4dce494dfbd3 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c

@@ -22,7 +22,7 @@
  *
  */
 
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/spinlock.h>
 #include <linux/tty.h>
 #include <linux/module.h>

@@ -32,7 +32,6 @@
 
 #include <asm/mach-types.h>
 
-#include <mach/board-ams-delta.h>
 #include <linux/platform_data/asoc-ti-mcbsp.h>
 
 #include "omap-mcbsp.h"

@@ -213,7 +212,6 @@ static const struct snd_kcontrol_new ams_delta_audio_controls[] = {
 static struct snd_soc_jack ams_delta_hook_switch;
 static struct snd_soc_jack_gpio ams_delta_hook_switch_gpios[] = {
 	{
-		.gpio = 4,
 		.name = "hook_switch",
 		.report = SND_JACK_HEADSET,
 		.invert = 1,

@@ -259,6 +257,7 @@ static struct timer_list cx81801_timer;
 static bool cx81801_cmd_pending;
 static bool ams_delta_muted;
 static DEFINE_SPINLOCK(ams_delta_lock);
+static struct gpio_desc *gpiod_modem_codec;
 
 static void cx81801_timeout(struct timer_list *unused)
 {

@@ -272,7 +271,7 @@ static void cx81801_timeout(struct timer_list *unused)
 	/* Reconnect the codec DAI back from the modem to the CPU DAI
 	 * only if digital mute still off */
 	if (!muted)
-		ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC, 0);
+		gpiod_set_value(gpiod_modem_codec, 0);
 }
 
 /* Line discipline .open() */

@@ -381,8 +380,7 @@ static void cx81801_receive(struct tty_struct *tty,
 		/* Apply config pulse by connecting the codec to the modem
 		 * if not already done */
 		if (apply)
-			ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
-						AMS_DELTA_LATCH2_MODEM_CODEC);
+			gpiod_set_value(gpiod_modem_codec, 1);
 		break;
 	}
 }

@@ -432,8 +430,7 @@ static int ams_delta_digital_mute(struct snd_soc_dai *dai, int mute)
 	spin_unlock_bh(&ams_delta_lock);
 
 	if (apply)
-		ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
-				mute ? AMS_DELTA_LATCH2_MODEM_CODEC : 0);
+		gpiod_set_value(gpiod_modem_codec, !!mute);
 	return 0;
 }
 

@@ -469,14 +466,6 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 	/* Store a pointer to the codec structure for tty ldisc use */
 	cx20442_codec = rtd->codec_dai->component;
 
-	/* Set up digital mute if not provided by the codec */
-	if (!codec_dai->driver->ops) {
-		codec_dai->driver->ops = &ams_delta_dai_ops;
-	} else {
-		ams_delta_ops.startup = ams_delta_startup;
-		ams_delta_ops.shutdown = ams_delta_shutdown;
-	}
-
 	/* Add hook switch - can be used to control the codec from userspace
 	 * even if line discipline fails */
 	ret = snd_soc_card_jack_new(card, "hook_switch", SND_JACK_HEADSET,

@@ -486,7 +475,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 				"Failed to allocate resources for hook switch, "
 				"will continue without one.\n");
 	else {
-		ret = snd_soc_jack_add_gpios(&ams_delta_hook_switch,
+		ret = snd_soc_jack_add_gpiods(card->dev, &ams_delta_hook_switch,
 					ARRAY_SIZE(ams_delta_hook_switch_gpios),
 					ams_delta_hook_switch_gpios);
 		if (ret)

@@ -495,6 +484,21 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 				"will continue with hook switch inactive.\n");
 	}
 
+	gpiod_modem_codec = devm_gpiod_get(card->dev, "modem_codec",
+					   GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod_modem_codec)) {
+		dev_warn(card->dev, "Failed to obtain modem_codec GPIO\n");
+		return 0;
+	}
+
+	/* Set up digital mute if not provided by the codec */
+	if (!codec_dai->driver->ops) {
+		codec_dai->driver->ops = &ams_delta_dai_ops;
+	} else {
+		ams_delta_ops.startup = ams_delta_startup;
+		ams_delta_ops.shutdown = ams_delta_shutdown;
+	}
+
 	/* Register optional line discipline for over the modem control */
 	ret = tty_register_ldisc(N_V253, &cx81801_ops);
 	if (ret) {

-- 
2.16.1

Now as the AMS Delta board header file is no longer included by
drivers, move it to the root directory of mach-omap1.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/ams-delta-fiq-handler.S              | 2 +-
 arch/arm/mach-omap1/ams-delta-fiq.c                      | 3 +--
 arch/arm/mach-omap1/board-ams-delta.c                    | 2 +-
 arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h | 0
 4 files changed, 3 insertions(+), 4 deletions(-)
 rename arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h (100%)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
index bf608441b357..9005c00db948 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
+++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S

@@ -16,9 +16,9 @@
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-#include <mach/board-ams-delta.h>
 #include <mach/ams-delta-fiq.h>
 
+#include "board-ams-delta.h"
 #include "iomap.h"
 #include "soc.h"
 

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index d7ca9e2b40d2..30aedcc3f2b3 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c

@@ -19,11 +19,10 @@
 #include <linux/module.h>
 #include <linux/io.h>
 
-#include <mach/board-ams-delta.h>
-
 #include <asm/fiq.h>
 
 #include <mach/ams-delta-fiq.h>
+#include "board-ams-delta.h"
 
 static struct fiq_handler fh = {
 	.name	= "ams-delta-fiq"

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276..17d69eb64df3 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c

@@ -36,7 +36,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
-#include <mach/board-ams-delta.h>
 #include <linux/platform_data/keypad-omap.h>
 #include <mach/mux.h>
 

@@ -45,6 +44,7 @@
 #include "camera.h"
 #include <mach/usb.h>
 
+#include "board-ams-delta.h"
 #include "iomap.h"
 #include "common.h"
 

diff --git a/arch/arm/mach-omap1/include/mach/board-ams-delta.h b/arch/arm/mach-omap1/board-ams-delta.h
similarity index 100%
rename from arch/arm/mach-omap1/include/mach/board-ams-delta.h
rename to arch/arm/mach-omap1/board-ams-delta.h
-- 
2.16.1

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents. Return -EPROBE_DEFER
if the GPIO pins are not yet available so device initialization is
postponed instead of aborted.

Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.

Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/mtd/nand/raw/ams-delta.c | 110 +++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..c44be2f5a65c 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c

@@ -20,23 +20,28 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/partitions.h>
-#include <linux/gpio.h>
 #include <linux/platform_data/gpio-omap.h>
 
 #include <asm/io.h>
 #include <asm/sizes.h>
 
-#include <mach/board-ams-delta.h>
-
 #include <mach/hardware.h>
 
 /*
  * MTD structure for E3 (Delta)
  */
 static struct mtd_info *ams_delta_mtd = NULL;
+static struct gpio_desc *gpiod_rdy;
+static struct gpio_desc *gpiod_nce;
+static struct gpio_desc *gpiod_nre;
+static struct gpio_desc *gpiod_nwp;
+static struct gpio_desc *gpiod_nwe;
+static struct gpio_desc *gpiod_ale;
+static struct gpio_desc *gpiod_cle;
 
 /*
  * Define partitions for flash devices

@@ -70,9 +75,9 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
 
 	writew(0, io_base + OMAP_MPUIO_IO_CNTL);
 	writew(byte, this->IO_ADDR_W);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NWE, 0);
+	gpiod_set_value(gpiod_nwe, 0);
 	ndelay(40);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NWE, 1);
+	gpiod_set_value(gpiod_nwe, 1);
 }
 
 static u_char ams_delta_read_byte(struct mtd_info *mtd)

@@ -81,11 +86,11 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
 	struct nand_chip *this = mtd_to_nand(mtd);
 	void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
 
-	gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NRE, 0);
+	gpiod_set_value(gpiod_nre, 0);
 	ndelay(40);
 	writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
 	res = readw(this->IO_ADDR_R);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NRE, 1);
+	gpiod_set_value(gpiod_nre, 1);
 
 	return res;
 }

@@ -120,12 +125,9 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
 {
 
 	if (ctrl & NAND_CTRL_CHANGE) {
-		gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NCE,
-				(ctrl & NAND_NCE) = 0);
-		gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_CLE,
-				(ctrl & NAND_CLE) != 0);
-		gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_ALE,
-				(ctrl & NAND_ALE) != 0);
+		gpiod_set_value(gpiod_nce, !(ctrl & NAND_NCE));
+		gpiod_set_value(gpiod_cle, !!(ctrl & NAND_CLE));
+		gpiod_set_value(gpiod_ale, !!(ctrl & NAND_ALE));
 	}
 
 	if (cmd != NAND_CMD_NONE)

@@ -134,41 +136,9 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
 
 static int ams_delta_nand_ready(struct mtd_info *mtd)
 {
-	return gpio_get_value(AMS_DELTA_GPIO_PIN_NAND_RB);
+	return gpiod_get_value(gpiod_rdy);
 }
 
-static const struct gpio _mandatory_gpio[] = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_NCE,
-		.flags	= GPIOF_OUT_INIT_HIGH,
-		.label	= "nand_nce",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_NRE,
-		.flags	= GPIOF_OUT_INIT_HIGH,
-		.label	= "nand_nre",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_NWP,
-		.flags	= GPIOF_OUT_INIT_HIGH,
-		.label	= "nand_nwp",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_NWE,
-		.flags	= GPIOF_OUT_INIT_HIGH,
-		.label	= "nand_nwe",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_ALE,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "nand_ale",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_NAND_CLE,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "nand_cle",
-	},
-};
 
 /*
  * Main initialization routine

@@ -216,12 +186,15 @@ static int ams_delta_init(struct platform_device *pdev)
 	this->write_buf = ams_delta_write_buf;
 	this->read_buf = ams_delta_read_buf;
 	this->cmd_ctrl = ams_delta_hwcontrol;
-	if (gpio_request(AMS_DELTA_GPIO_PIN_NAND_RB, "nand_rdy") = 0) {
+
+	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
 		this->dev_ready = ams_delta_nand_ready;
 	} else {
 		this->dev_ready = NULL;
 		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
 	}
+
 	/* 25 us command delay time */
 	this->chip_delay = 30;
 	this->ecc.mode = NAND_ECC_SOFT;

@@ -230,7 +203,44 @@ static int ams_delta_init(struct platform_device *pdev)
 	platform_set_drvdata(pdev, io_base);
 
 	/* Set chip enabled, but  */
-	err = gpio_request_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
+	gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod_nwp)) {
+		err = PTR_ERR(gpiod_nwp);
+		dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
+		goto err_gpiod;
+	}
+	gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod_nce)) {
+		err = PTR_ERR(gpiod_nce);
+		dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
+		goto err_gpiod;
+	}
+	gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod_nre)) {
+		err = PTR_ERR(gpiod_nre);
+		dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
+		goto err_gpiod;
+	}
+	gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod_nwe)) {
+		err = PTR_ERR(gpiod_nwe);
+		dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
+		goto err_gpiod;
+	}
+	gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_ale)) {
+		err = PTR_ERR(gpiod_ale);
+		dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
+		goto err_gpiod;
+	}
+	gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_cle)) {
+		err = PTR_ERR(gpiod_cle);
+		dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
+	}
+err_gpiod:
+	if (err = -ENODEV || err = -ENOENT)
+		err = -EPROBE_DEFER;
 	if (err)
 		goto out_gpio;
 

@@ -246,9 +256,7 @@ static int ams_delta_init(struct platform_device *pdev)
 	goto out;
 
  out_mtd:
-	gpio_free_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
 out_gpio:
-	gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
 	iounmap(io_base);
 out_free:
 	kfree(this);

@@ -266,8 +274,6 @@ static int ams_delta_cleanup(struct platform_device *pdev)
 	/* Release resources, unregister device */
 	nand_release(ams_delta_mtd);
 
-	gpio_free_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
-	gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
 	iounmap(io_base);
 
 	/* Free the MTD device structure */

-- 
2.16.1

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents. Move GPIO lookup
to the driver probe function so device initialization can be postponed
instead of aborted if the GPIO pin is not yet available.

Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.

Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/video/fbdev/omap/lcd_ams_delta.c | 59 ++++++++++++++------------------
 1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/drivers/video/fbdev/omap/lcd_ams_delta.c b/drivers/video/fbdev/omap/lcd_ams_delta.c
index a4ee947006c7..19b6425b54be 100644
--- a/drivers/video/fbdev/omap/lcd_ams_delta.c
+++ b/drivers/video/fbdev/omap/lcd_ams_delta.c

@@ -24,11 +24,10 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/lcd.h>
-#include <linux/gpio.h>
 
 #include <mach/hardware.h>
-#include <mach/board-ams-delta.h>
 
 #include "omapfb.h"
 

@@ -41,6 +40,8 @@
 /* LCD class device section */
 
 static int ams_delta_lcd;
+static struct gpio_desc *gpiod_vblen;
+static struct gpio_desc *gpiod_ndisp;
 
 static int ams_delta_lcd_set_power(struct lcd_device *dev, int power)
 {

@@ -99,41 +100,17 @@ static struct lcd_ops ams_delta_lcd_ops = {
 
 /* omapfb panel section */
 
-static const struct gpio _gpios[] = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_LCD_VBLEN,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "lcd_vblen",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_LCD_NDISP,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "lcd_ndisp",
-	},
-};
-
-static int ams_delta_panel_init(struct lcd_panel *panel,
-		struct omapfb_device *fbdev)
-{
-	return gpio_request_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
-static void ams_delta_panel_cleanup(struct lcd_panel *panel)
-{
-	gpio_free_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
 static int ams_delta_panel_enable(struct lcd_panel *panel)
 {
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 1);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 1);
+	gpiod_set_value(gpiod_ndisp, 1);
+	gpiod_set_value(gpiod_vblen, 1);
 	return 0;
 }
 
 static void ams_delta_panel_disable(struct lcd_panel *panel)
 {
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 0);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 0);
+	gpiod_set_value(gpiod_vblen, 0);
+	gpiod_set_value(gpiod_ndisp, 0);
 }
 
 static struct lcd_panel ams_delta_panel = {

@@ -154,8 +131,6 @@ static struct lcd_panel ams_delta_panel = {
 	.pcd		= 0,
 	.acb		= 37,
 
-	.init		= ams_delta_panel_init,
-	.cleanup	= ams_delta_panel_cleanup,
 	.enable		= ams_delta_panel_enable,
 	.disable	= ams_delta_panel_disable,
 };

@@ -166,9 +141,27 @@ static struct lcd_panel ams_delta_panel = {
 static int ams_delta_panel_probe(struct platform_device *pdev)
 {
 	struct lcd_device *lcd_device = NULL;
-#ifdef CONFIG_LCD_CLASS_DEVICE
 	int ret;
 
+	gpiod_vblen = devm_gpiod_get(&pdev->dev, "vblen", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_vblen)) {
+		ret = PTR_ERR(gpiod_vblen);
+		dev_err(&pdev->dev, "VBLEN GPIO request failed (%d)\n", ret);
+		if (ret = -ENODEV || ret = -ENOENT)
+			ret = -EPROBE_DEFER;
+		return ret;
+	}
+
+	gpiod_ndisp = devm_gpiod_get(&pdev->dev, "ndisp", GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod_ndisp)) {
+		ret = PTR_ERR(gpiod_ndisp);
+		dev_err(&pdev->dev, "NDISP GPIO request failed (%d)\n", ret);
+		if (ret = -ENODEV || ret = -ENOENT)
+			ret = -EPROBE_DEFER;
+		return ret;
+	}
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
 	lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
 						&ams_delta_lcd_ops);
 

-- 
2.16.1

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
[off-list ref] wrote:

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

                this->dev_ready = ams_delta_nand_ready;
        } else {
                this->dev_ready = NULL;
                pr_notice("Couldn't request gpio for Delta NAND ready.\n");

dev_notice() ?

        }

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

-- 
With Best Regards,
Andy Shevchenko

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional. 
That's why I decided to use the _optional variant of devm_gpiod_get(). In case 
of ams-delta, the dev_ready() callback depends on availability of the 'rdy' 
GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to 
decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only if 
the purpose of the _optional form is different.

quoted

                this->dev_ready = ams_delta_nand_ready;
        
        } else {
        
                this->dev_ready = NULL;
                pr_notice("Couldn't request gpio for Delta NAND
                ready.\n");

dev_notice() ?

Sure, but maybe in a separate patch? That's not a new code just being added 
but an existing one, not the merit of the change.

quoted

        }

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not 
availble before device init phase, unlike other crucial GPIO drivers which are 
initialized earlier, e.g. during the postcore or at latetst the subsys phase. 
Hence, devices which depend on GPIO pins provided by gpio-mmio must either be 
declared late or fail softly so they get another chance of being probed 
succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions it 
exports but for now I haven't implemented it, not even shared the idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?

Thanks,
Janusz

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref] wrote:

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In case
of ams-delta, the dev_ready() callback depends on availability of the 'rdy'
GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to
decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only if
the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

quoted

quoted

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which are
initialized earlier, e.g. during the postcore or at latetst the subsys phase.
Hence, devices which depend on GPIO pins provided by gpio-mmio must either be
declared late or fail softly so they get another chance of being probed
succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions it
exports but for now I haven't implemented it, not even shared the idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?

I'm only concerned if it would be an infinite defer in the case when
driver will never appear.
But I don't remember the details.

-- 
With Best Regards,
Andy Shevchenko

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref] 

wrote:

quoted

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In
case of ams-delta, the dev_ready() callback depends on availability of
the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
in order to decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only
if the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-		this->dev_ready = ams_delta_nand_ready;
-	} else {
-		this->dev_ready = NULL;
-		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+						  GPIOD_IN);
+	if (IS_ERR(priv->gpiod_rdy)) {
+		err = PTR_ERR(priv->gpiod_nwp);
+		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+		goto err_gpiod;
 	}
 
+	if (priv->gpiod_rdy)
+		this->dev_ready = ams_delta_nand_ready;

quoted

quoted

quoted

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which
are initialized earlier, e.g. during the postcore or at latetst the
subsys phase. Hence, devices which depend on GPIO pins provided by
gpio-mmio must either be declared late or fail softly so they get another
chance of being probed succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions
it exports but for now I haven't implemented it, not even shared the
idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
obtained?

I'm only concerned if it would be an infinite defer in the case when
driver will never appear.
But I don't remember the details.

Deferred probes are handled effectively during late_initcall, no risk of 
infinite defer, see drivers/base/dd.c for details.

Thanks,
Janusz

On Sun, May 20, 2018 at 12:55 AM, Janusz Krzysztofik
[off-list ref] wrote:

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref]

wrote:

quoted

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-       if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-               this->dev_ready = ams_delta_nand_ready;
-       } else {
-               this->dev_ready = NULL;
-               pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+       priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+                                                 GPIOD_IN);
+       if (IS_ERR(priv->gpiod_rdy)) {
+               err = PTR_ERR(priv->gpiod_nwp);
+               dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+               goto err_gpiod;
        }

+       if (priv->gpiod_rdy)
+               this->dev_ready = ams_delta_nand_ready;

This makes sense.

Though, I completely dislike "rdy" name of GPIO. Where is it documented?

quoted

quoted

quoted

quoted

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which
are initialized earlier, e.g. during the postcore or at latetst the
subsys phase. Hence, devices which depend on GPIO pins provided by
gpio-mmio must either be declared late or fail softly so they get another
chance of being probed succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions
it exports but for now I haven't implemented it, not even shared the
idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
obtained?

I'm only concerned if it would be an infinite defer in the case when
driver will never appear.
But I don't remember the details.

Deferred probes are handled effectively during late_initcall, no risk of
infinite defer, see drivers/base/dd.c for details.

Yes, but the code you provided in patch looks somehow suspicious. OK,
I let Linus decide whtat to do with that.

-- 
With Best Regards,
Andy Shevchenko

On Sunday, May 20, 2018 4:44:31 PM CEST Andy Shevchenko wrote:

On Sun, May 20, 2018 at 12:55 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref]

wrote:

quoted

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-       if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-               this->dev_ready = ams_delta_nand_ready;
-       } else {
-               this->dev_ready = NULL;
-               pr_notice("Couldn't request gpio for Delta NAND
ready.\n");
+       priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+                                                 GPIOD_IN);
+       if (IS_ERR(priv->gpiod_rdy)) {
+               err = PTR_ERR(priv->gpiod_nwp);
+               dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n",
err); +               goto err_gpiod;

        }

+       if (priv->gpiod_rdy)
+               this->dev_ready = ams_delta_nand_ready;

This makes sense.

Though, I completely dislike "rdy" name of GPIO. Where is it documented?

No documentation files for Amstrad Delta nor for its NAND driver specifically 
exist under Documentation/. However, there exist some for generic GPIO NAND 
driver where the pin name "rdy" is used explicitly:
Documentation/driver-api/gpio/drivers-on-gpio.rst
Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
You can find that mnemonic used across drivers/mtd/nand/, standalone or as a 
suffix, including the Amstrad Delta NAND driver before the change discussed.

To be honest, I don't like it much either, but I'm just using it instead of 
inventing something new.

Thanks,
Janusz

On Sun, May 20, 2018 at 6:37 PM, Janusz Krzysztofik [off-list ref] wrote:

On Sunday, May 20, 2018 4:44:31 PM CEST Andy Shevchenko wrote:

quoted

Though, I completely dislike "rdy" name of GPIO. Where is it documented?

No documentation files for Amstrad Delta nor for its NAND driver specifically
exist under Documentation/. However, there exist some for generic GPIO NAND
driver where the pin name "rdy" is used explicitly:
Documentation/driver-api/gpio/drivers-on-gpio.rst
Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
You can find that mnemonic used across drivers/mtd/nand/, standalone or as a
suffix, including the Amstrad Delta NAND driver before the change discussed.

To be honest, I don't like it much either, but I'm just using it instead of
inventing something new.

OK, that's what I was looking for. Since it's already in use and
documented, then it's fine for me.

-- 
With Best Regards,
Andy Shevchenko

On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref] 

wrote:

quoted

quoted

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In
case of ams-delta, the dev_ready() callback depends on availability of
the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
in order to decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only
if the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-		this->dev_ready = ams_delta_nand_ready;
-	} else {
-		this->dev_ready = NULL;
-		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+						  GPIOD_IN);
+	if (IS_ERR(priv->gpiod_rdy)) {
+		err = PTR_ERR(priv->gpiod_nwp);

??? --------------------------------^^^^^^^^^

+		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+		goto err_gpiod;

Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.

	ladis

 	}
 
+	if (priv->gpiod_rdy)
+		this->dev_ready = ams_delta_nand_ready;

quoted

quoted

quoted

quoted

+err_gpiod:
+       if (err = -ENODEV || err = -ENOENT)
+               err = -EPROBE_DEFER;

Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which
are initialized earlier, e.g. during the postcore or at latetst the
subsys phase. Hence, devices which depend on GPIO pins provided by
gpio-mmio must either be declared late or fail softly so they get another
chance of being probed succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions
it exports but for now I haven't implemented it, not even shared the
idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
obtained?

I'm only concerned if it would be an infinite defer in the case when
driver will never appear.
But I don't remember the details.

Deferred probes are handled effectively during late_initcall, no risk of 
infinite defer, see drivers/base/dd.c for details.

Thanks,
Janusz



_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

On Sun, May 20, 2018 at 09:27:05PM +0200, Ladislav Michl wrote:

On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:

quoted

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik [off-list ref] 

wrote:

quoted

quoted

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In
case of ams-delta, the dev_ready() callback depends on availability of
the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
in order to decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only
if the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-		this->dev_ready = ams_delta_nand_ready;
-	} else {
-		this->dev_ready = NULL;
-		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+						  GPIOD_IN);
+	if (IS_ERR(priv->gpiod_rdy)) {
+		err = PTR_ERR(priv->gpiod_nwp);

??? --------------------------------^^^^^^^^^

quoted

+		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+		goto err_gpiod;

Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.

If RDY signal is not available then the board should not define it.
Degrading performance and having users wondering because RDY is
sometimes not available is not great. Especially if we get -EPROBE_DEFER
here.

Thanks.

-- 
Dmitry

Hi Janusz,

On Fri, May 18, 2018 at 11:09:50PM +0200, Janusz Krzysztofik wrote:

Since you are doing the conversion: it does not appear that all these
are necessarily GPIOs; for example should not power be gpio-regulator
and data be simply expressed as IRQ resource? And the driver to be
converted into a platform driver?

I think this needs to be done first, because otherwise you are
committing to a certain binding and will have hard time changing it
later.

Thanks.

-- 
Dmitry

On Fri, May 18, 2018 at 11:09:51PM +0200, Janusz Krzysztofik wrote:

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Acked-by: Mark Brown <broonie@kernel.org>

Hi,

* Janusz Krzysztofik [off-list ref] [180518 14:12]:

Scope of the change is limited to GPIO pins used by board specific
device drivers which will be updated by follow-up patches of the
series. Those are some OMAP GPIO (gpio-0-15) and most of Amstrad Delta
latch2 GPIO bank pins. Remaining pins of those banks, as well as
Amstrad Delta latch1 pins, will be addressed later.

Assign a label ("latch2") to the bank, enumerate its pins and put that
information, together with OMAP GPIO bank pins, in GPIO lookup tables.
Assign lookup tables to devices as soon as those devices are registered
and their names can be obtained.

A step froward in:
- removal of hard-coded GPIO numbers from drivers,
- removal of board mach includes from drivers,
- switching to dynamically assigned GPIO numbers.

Is this first patch safe for me to apply separately?

Regards,

Tony

On Monday, May 21, 2018 7:35:19 PM CEST Tony Lindgren wrote:

Hi,

* Janusz Krzysztofik [off-list ref] [180518 14:12]:

quoted

Scope of the change is limited to GPIO pins used by board specific
device drivers which will be updated by follow-up patches of the
series. Those are some OMAP GPIO (gpio-0-15) and most of Amstrad Delta
latch2 GPIO bank pins. Remaining pins of those banks, as well as
Amstrad Delta latch1 pins, will be addressed later.

Assign a label ("latch2") to the bank, enumerate its pins and put that
information, together with OMAP GPIO bank pins, in GPIO lookup tables.
Assign lookup tables to devices as soon as those devices are registered
and their names can be obtained.

A step froward in:
- removal of hard-coded GPIO numbers from drivers,
- removal of board mach includes from drivers,
- switching to dynamically assigned GPIO numbers.

Is this first patch safe for me to apply separately?

Absolutely, it is.

Thanks,
Janusz

On Sunday, May 20, 2018 10:08:22 PM CEST Dmitry Torokhov wrote:

On Sun, May 20, 2018 at 09:27:05PM +0200, Ladislav Michl wrote:

quoted

On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:

quoted

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik
[off-list ref]

wrote:

quoted

quoted

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is
optional.
That's why I decided to use the _optional variant of
devm_gpiod_get(). In
case of ams-delta, the dev_ready() callback depends on availability
of
the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and
ERR
in order to decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for
ERR only
if the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-		this->dev_ready = ams_delta_nand_ready;
-	} else {
-		this->dev_ready = NULL;
-		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+						  GPIOD_IN);
+	if (IS_ERR(priv->gpiod_rdy)) {
+		err = PTR_ERR(priv->gpiod_nwp);

??? --------------------------------^^^^^^^^^

quoted

+		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+		goto err_gpiod;

Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.

If RDY signal is not available then the board should not define it.
Degrading performance and having users wondering because RDY is
sometimes not available is not great. Especially if we get -EPROBE_DEFER
here.

Hi,

I'm a bit lost after your comments.

As far as I can read the code of gpiod_get_optional and underlying functions, 
if a board doesn't define the "rdy" pin in a respective lookup table, the 
function returns NULL and the device gets a chance to work in degraded mode.

NULL may also happen if the driver probes the device before the lookup table 
is added. In that case other non-optional pin requests fail with -ENOENT, the 
probe is deferred and the device gets a chance to probe successfully in 
late_init if the table is added but fails if not.

If the pin is defined but GPIO device providing that pin is not available 
(-ENODEV), the probe is initially deferred and may succeed in late_init if the 
GPIO device appears but fails otherwise. 

Isn't that behavior acceptable, close enough to the expected even if not 
strictly because of that -EPROBE_DEFER?

Thanks,
Janusz

On Mon, May 21, 2018 at 10:21:46PM +0200, Janusz Krzysztofik wrote:

On Sunday, May 20, 2018 10:08:22 PM CEST Dmitry Torokhov wrote:

quoted

On Sun, May 20, 2018 at 09:27:05PM +0200, Ladislav Michl wrote:

quoted

On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:

quoted

On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik
[off-list ref]

wrote:

quoted

quoted

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:

quoted

On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik

[off-list ref] wrote:

quoted

+       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
GPIOD_IN);
+       if (!IS_ERR_OR_NULL(gpiod_rdy)) {

So, is it optional or not at the end?
If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is
optional.
That's why I decided to use the _optional variant of
devm_gpiod_get(). In
case of ams-delta, the dev_ready() callback depends on availability
of
the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and
ERR
in order to decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for
ERR only
if the purpose of the _optional form is different.

NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.

OK, I'm going to use something like the below while submitting v2:

-	gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
-	if (!IS_ERR_OR_NULL(gpiod_rdy)) {
-		this->dev_ready = ams_delta_nand_ready;
-	} else {
-		this->dev_ready = NULL;
-		pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+	priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+						  GPIOD_IN);
+	if (IS_ERR(priv->gpiod_rdy)) {
+		err = PTR_ERR(priv->gpiod_nwp);

??? --------------------------------^^^^^^^^^

quoted

+		dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+		goto err_gpiod;

Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.

If RDY signal is not available then the board should not define it.
Degrading performance and having users wondering because RDY is
sometimes not available is not great. Especially if we get -EPROBE_DEFER
here.

Hi,

I'm a bit lost after your comments.

As far as I can read the code of gpiod_get_optional and underlying functions, 
if a board doesn't define the "rdy" pin in a respective lookup table, the 
function returns NULL and the device gets a chance to work in degraded mode.

NULL may also happen if the driver probes the device before the lookup table 
is added. In that case other non-optional pin requests fail with -ENOENT, the 
probe is deferred and the device gets a chance to probe successfully in 
late_init if the table is added but fails if not.

If the pin is defined but GPIO device providing that pin is not available 
(-ENODEV), the probe is initially deferred and may succeed in late_init if the 
GPIO device appears but fails otherwise. 

Isn't that behavior acceptable, close enough to the expected even if not 
strictly because of that -EPROBE_DEFER?

Yes, this is correct. I was responding to the comment that erroring out
in "if (IS_ERR(priv->gpiod_rdy))" branch is too strict. My assertion
that it is not. If a board defines RDY pin we should use it and not try
to degrade to lower performance mode.

Thanks.

-- 
Dmitry

* Mark Brown [off-list ref] [180521 10:07]:

On Fri, May 18, 2018 at 11:09:51PM +0200, Janusz Krzysztofik wrote:

quoted

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Acked-by: Mark Brown <broonie@kernel.org>

Thanks applying patches 1 and 3 of this series into omap-for-v4.18/soc.
It's kind of getting late for v4.18, but let's see if we can still make
it.

Seems the others can be applied to the driver trees after no more
comments, then once all that is done we can apply the last patch
in this series.

Regards,

Tony

On Wednesday, May 23, 2018 8:52:44 PM CEST Tony Lindgren wrote:

* Mark Brown [off-list ref] [180521 10:07]:

quoted

On Fri, May 18, 2018 at 11:09:51PM +0200, Janusz Krzysztofik wrote:

quoted

Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.

Acked-by: Mark Brown <broonie@kernel.org>

Hi Tony,

Thanks applying patches 1 and 3 of this series into omap-for-v4.18/soc.
It's kind of getting late for v4.18, but let's see if we can still make
it.

Thank you.

Seems the others can be applied to the driver trees after no more
comments, then once all that is done we can apply the last patch
in this series.

I'll be submitting v2 of 5/6 (nand) very soon. 4/6 (lcd) is still waiting for 
Tomi to respond. I hope there will be no issues with it. Howevver, regarding 
2/6 - serio - I have to work more on that to satisfy Dmitry's comments. So 
let's forget about 6/6 for now and I'll resubmit it again when we are ready 
for that. Meanwhile, I'm going to submit a few more patches against the board 
init file to complete migration to GPIO descriptors so dynamic allocation of 
GPIO numbers to ams-delta latches will be possible.

Thanks,
Janusz

On Friday, May 18, 2018 11:09:52 PM CEST Janusz Krzysztofik wrote:

Now as Amstrad Delta board - the only user of this driver - provides
GPIO lookup tables, switch from GPIO numbers to GPIO descriptors and
use the table to locate required GPIO pins.

Declare static variables for storing GPIO descriptors and replace
gpio_ function calls with their gpiod_ equivalents. Move GPIO lookup
to the driver probe function so device initialization can be deferred
instead of aborted if a GPIO pin is not yet available.

Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: Remove problematic error code conversion, no longer needed if used
    on top of commit d08605a64e67 ("ARM: OMAP1: ams-delta: move late
    devices back to init_machine") already in linux-next and commit
    8853daf3b4ac ("gpiolib: Defer on non-DT find_chip_by_name()
    failure") just applied to linux-gpio/devel.

 drivers/video/fbdev/omap/lcd_ams_delta.c | 55 +++++++++++++-------------------
 1 file changed, 22 insertions(+), 33 deletions(-)

diff --git a/drivers/video/fbdev/omap/lcd_ams_delta.c b/drivers/video/fbdev/omap/lcd_ams_delta.c
index e8c748a0dfe2..cddbd00cbf9f 100644
--- a/drivers/video/fbdev/omap/lcd_ams_delta.c
+++ b/drivers/video/fbdev/omap/lcd_ams_delta.c

@@ -24,11 +24,10 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/lcd.h>
-#include <linux/gpio.h>
 
 #include <mach/hardware.h>
-#include <mach/board-ams-delta.h>
 
 #include "omapfb.h"
 

@@ -41,6 +40,8 @@
 /* LCD class device section */
 
 static int ams_delta_lcd;
+static struct gpio_desc *gpiod_vblen;
+static struct gpio_desc *gpiod_ndisp;
 
 static int ams_delta_lcd_set_power(struct lcd_device *dev, int power)
 {

@@ -99,41 +100,17 @@ static struct lcd_ops ams_delta_lcd_ops = {
 
 /* omapfb panel section */
 
-static const struct gpio _gpios[] = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_LCD_VBLEN,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "lcd_vblen",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_LCD_NDISP,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "lcd_ndisp",
-	},
-};
-
-static int ams_delta_panel_init(struct lcd_panel *panel,
-		struct omapfb_device *fbdev)
-{
-	return gpio_request_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
-static void ams_delta_panel_cleanup(struct lcd_panel *panel)
-{
-	gpio_free_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
 static int ams_delta_panel_enable(struct lcd_panel *panel)
 {
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 1);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 1);
+	gpiod_set_value(gpiod_ndisp, 1);
+	gpiod_set_value(gpiod_vblen, 1);
 	return 0;
 }
 
 static void ams_delta_panel_disable(struct lcd_panel *panel)
 {
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 0);
-	gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 0);
+	gpiod_set_value(gpiod_vblen, 0);
+	gpiod_set_value(gpiod_ndisp, 0);
 }
 
 static struct lcd_panel ams_delta_panel = {