[PATCH 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[PATCH 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Janusz Krzysztofik]

Convert modem related GPIO setup from integer space to GPIO descriptors.
Also, restore original initialization order of the MODEM device and its
related GPIO pins.

Cleanup of MODEM relaated regulator setup is postponed while waiting for
upcoming conversion of fixed regulator API to GPIO descriptors.


Janusz Krzysztofik (3):
      ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
      ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
      ARM: OMAP1: ams-delta: register MODEM device earlier


diffstat:
 board-ams-delta.c |  120 +++++++++++++++++++++++++++++++++++++++---------------
 board-ams-delta.h |    7 ---
 2 files changed, 88 insertions(+), 39 deletions(-)

[PATCH 1/3] ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor

[Janusz Krzysztofik]

Don't request MODEM IRQ GPIO by its global number in
ams_delta_modem_init().  Instead, obtain its GPIO descriptor
and assign related IRQ to the MODEM.  Do that from
omap_gpio_deps_init(), where the chip is already looked up.  Then, in
ams_delta_modem_init(), just check for the IRQ number having been
already assigned.

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

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 34cb63ff45b3..b3fe515b6f49 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -630,6 +630,28 @@ static struct gpiod_hog ams_delta_gpio_hogs[] = {
 	{},
 };
 
+static struct plat_serial8250_port ams_delta_modem_ports[];
+
+/*
+ * Obtain MODEM IRQ GPIO descriptor using its hardware pin
+ * number and assign related IRQ number to the MODEM port.
+ * Keep the GPIO descriptor open so nobody steps in.
+ */
+static void __init modem_assign_irq(struct gpio_chip *chip)
+{
+	struct gpio_desc *gpiod;
+
+	gpiod = gpiochip_request_own_desc(chip, AMS_DELTA_GPIO_PIN_MODEM_IRQ,
+					  "modem_irq");
+	if (IS_ERR(gpiod)) {
+		pr_err("%s: modem IRQ GPIO request failed (%ld)\n", __func__,
+		       PTR_ERR(gpiod));
+	} else {
+		gpiod_direction_input(gpiod);
+		ams_delta_modem_ports[0].irq = gpiod_to_irq(gpiod);
+	}
+}
+
 /*
  * The purpose of this function is to take care of proper initialization of
  * devices and data structures which depend on GPIO lines provided by OMAP GPIO
@@ -649,7 +671,13 @@ static void __init omap_gpio_deps_init(void)
 		return;
 	}
 
+	/*
+	 * Start with FIQ initialization as it may have to request
+	 * and release successfully each OMAP GPIO pin in turn.
+	 */
 	ams_delta_init_fiq(chip, &ams_delta_serio_device);
+
+	modem_assign_irq(chip);
 }
 
 static void __init ams_delta_init(void)
@@ -844,20 +872,18 @@ static int __init modem_nreset_init(void)
 }
 
 
+/*
+ * This function expects MODEM IRQ number already assigned to the port
+ * and fails if it's not.
+ */
 static int __init ams_delta_modem_init(void)
 {
 	int err;
 
-	omap_cfg_reg(M14_1510_GPIO2);
-	ams_delta_modem_ports[0].irq =
-			gpio_to_irq(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
+	if (ams_delta_modem_ports[0].irq < 0)
+		return ams_delta_modem_ports[0].irq;
 
-	err = gpio_request(AMS_DELTA_GPIO_PIN_MODEM_IRQ, "modem");
-	if (err) {
-		pr_err("Couldn't request gpio pin for modem\n");
-		return err;
-	}
-	gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
+	omap_cfg_reg(M14_1510_GPIO2);
 
 	/* Initialize the modem_nreset regulator consumer before use */
 	modem_priv.regulator = ERR_PTR(-ENODEV);
@@ -866,8 +892,6 @@ static int __init ams_delta_modem_init(void)
 			AMS_DELTA_LATCH2_MODEM_CODEC);
 
 	err = platform_device_register(&ams_delta_modem_device);
-	if (err)
-		gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
 
 	return err;
 }
-- 
2.16.4

Re: [PATCH 1/3] ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor

[Linus Walleij]

On Mon, Aug 20, 2018 at 8:12 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:

> Don't request MODEM IRQ GPIO by its global number in
> ams_delta_modem_init().  Instead, obtain its GPIO descriptor
> and assign related IRQ to the MODEM.  Do that from
> omap_gpio_deps_init(), where the chip is already looked up.  Then, in
> ams_delta_modem_init(), just check for the IRQ number having been
> already assigned.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

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

It's a bit unorthodox to have the board files intercept and poke
around inside the gpio_chip like the code in this file does,
I prefer to keep it encapsulated inside the GPIO driver.
However this is the lesser evil... if OMAP1 is ever phased
over to device tree it will go away, until then it is just nicely
contained.

Yours,
Linus Walleij

[PATCH 2/3] ARM: OMAP1: ams-delta: initialize latch2 pins to safe values

[Janusz Krzysztofik]

Latch2 pins control a number of on-board devices, namely LCD, NAND,
MODEM and CODEC.  Those pins used to be initialized with safe values
from init_machine before that operation was:
1) moved to late_initcall in preparation for conversion of latch2 to
GPIO device - see commit f7519d8c8290 ("ARM: OMAP1: ams-delta: register
latch dependent devices later"),
2) replaced with non-atomic initialization performed by means of
gpio_request_array() - see commit 937eb4bb0058 ("ARM: OMAP1: ams-delta:
convert latches to basic_mmio_gpio"),
3) made completely asynchronous by delegation of GPIO request
operations performed on subsets of pins to respective device drivers in
subsequent commits.

One visible negative result of that disintegration was corrupt keyboard
data reported by serio driver, recently fixed by commit 41f8fee385a0
("ARM: OMAP1: ams-delta: Hog "keybrd_dataout" GPIO pin").

Moreover, initialization of LATCH2_PIN_MODEM_CODEC still performed with
ams_delta_latch2_write() wrapper from late_init() is now done on not
requested GPIO pin.

Reintroduce atomic initialization of latch2 pins at machine_init to
prevent from random values potentially corrupting NAND data or maybe
even destroing other hardware.  Also take care of MODEM/CODEC related
pins so MODEM device probe succeeds even if latch2 GPIO device or
dependent regulator is not ready and CODEC can be reached over the
MODEM even if audio driver doesn't take control over
LATCH2_PIN_MODEM_CODEC.

Once done, remove the no longer needed GPIO based implementation of
ams_delta_latch_write() and its frontend macro.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 arch/arm/mach-omap1/board-ams-delta.c | 52 +++++++++++++++++++++++------------
 arch/arm/mach-omap1/board-ams-delta.h |  7 -----
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index b3fe515b6f49..c865c6921af8 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -321,20 +321,6 @@ struct modem_private_data {
 
 static struct modem_private_data modem_priv;
 
-void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value)
-{
-	int bit = 0;
-	u16 bitpos = 1 << bit;
-
-	for (; bit < ngpio; bit++, bitpos = bitpos << 1) {
-		if (!(mask & bitpos))
-			continue;
-		else
-			gpio_set_value(base + bit, (value & bitpos) != 0);
-	}
-}
-EXPORT_SYMBOL(ams_delta_latch_write);
-
 static struct resource ams_delta_nand_resources[] = {
 	[0] = {
 		.start	= OMAP1_MPUIO_BASE,
@@ -680,6 +666,40 @@ static void __init omap_gpio_deps_init(void)
 	modem_assign_irq(chip);
 }
 
+/*
+ * Initialize latch2 pins with values which are safe for dependent on-board
+ * devices or useful for their successull initialization even before GPIO
+ * driver takes control over the latch pins:
+ * - LATCH2_PIN_LCD_VBLEN	= 0
+ * - LATCH2_PIN_LCD_NDISP	= 0	Keep LCD device powered off before its
+ *					driver takes control over it.
+ * - LATCH2_PIN_NAND_NCE	= 0
+ * - LATCH2_PIN_NAND_NWP	= 0	Keep NAND device down and write-
+ *					protected before its driver takes
+ *					control over it.
+ * - LATCH2_PIN_KEYBRD_PWR	= 0	Keep keyboard powered off before serio
+ *					driver takes control over it.
+ * - LATCH2_PIN_KEYBRD_DATAOUT	= 0	Keep low to avoid corruption of first
+ *					byte of data received from attached
+ *					keyboard when serio device is probed;
+ *					the pin is also hogged low by the latch2
+ *					GPIO driver as soon as it is ready.
+ * - LATCH2_PIN_MODEM_NRESET	= 1	Enable voice MODEM device, allowing for
+ *					its successful probe even before a
+ *					regulator it depends on, which in turn
+ *					takes control over the pin, is set up.
+ * - LATCH2_PIN_MODEM_CODEC	= 1	Attach voice MODEM CODEC data port
+ *					to the MODEM so the CODEC is under
+ *					control even if audio driver doesn't
+ *					take it over.
+ */
+static void __init ams_delta_latch2_init(void)
+{
+	u16 latch2 = 1 << LATCH2_PIN_MODEM_NRESET | 1 << LATCH2_PIN_MODEM_CODEC;
+
+	__raw_writew(latch2, LATCH2_VIRT);
+}
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -701,6 +721,7 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J18_1610_CAM_D7);
 
 	omap_gpio_deps_init();
+	ams_delta_latch2_init();
 	gpiod_add_hogs(ams_delta_gpio_hogs);
 
 	omap_serial_init();
@@ -888,9 +909,6 @@ static int __init ams_delta_modem_init(void)
 	/* Initialize the modem_nreset regulator consumer before use */
 	modem_priv.regulator = ERR_PTR(-ENODEV);
 
-	ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
-			AMS_DELTA_LATCH2_MODEM_CODEC);
-
 	err = platform_device_register(&ams_delta_modem_device);
 
 	return err;
diff --git a/arch/arm/mach-omap1/board-ams-delta.h b/arch/arm/mach-omap1/board-ams-delta.h
index 1fbada29431a..a74a306d7e77 100644
--- a/arch/arm/mach-omap1/board-ams-delta.h
+++ b/arch/arm/mach-omap1/board-ams-delta.h
@@ -59,13 +59,6 @@
 #define AMS_DELTA_LATCH2_GPIO_BASE	AMS_DELTA_GPIO_PIN_LCD_VBLEN
 #define AMS_DELTA_LATCH2_NGPIO		16
 
-#ifndef __ASSEMBLY__
-void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value);
-#define ams_delta_latch2_write(mask, value) \
-	ams_delta_latch_write(AMS_DELTA_LATCH2_GPIO_BASE, \
-			AMS_DELTA_LATCH2_NGPIO, (mask), (value))
-#endif
-
 #endif /* CONFIG_MACH_AMS_DELTA */
 
 #endif /* __ASM_ARCH_OMAP_AMS_DELTA_H */
-- 
2.16.4

Re: [PATCH 2/3] ARM: OMAP1: ams-delta: initialize latch2 pins to safe values

[Linus Walleij]

On Mon, Aug 20, 2018 at 8:12 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:

> Latch2 pins control a number of on-board devices, namely LCD, NAND,
> MODEM and CODEC.  Those pins used to be initialized with safe values
> from init_machine before that operation was:
> 1) moved to late_initcall in preparation for conversion of latch2 to
> GPIO device - see commit f7519d8c8290 ("ARM: OMAP1: ams-delta: register
> latch dependent devices later"),
> 2) replaced with non-atomic initialization performed by means of
> gpio_request_array() - see commit 937eb4bb0058 ("ARM: OMAP1: ams-delta:
> convert latches to basic_mmio_gpio"),
> 3) made completely asynchronous by delegation of GPIO request
> operations performed on subsets of pins to respective device drivers in
> subsequent commits.
>
> One visible negative result of that disintegration was corrupt keyboard
> data reported by serio driver, recently fixed by commit 41f8fee385a0
> ("ARM: OMAP1: ams-delta: Hog "keybrd_dataout" GPIO pin").
>
> Moreover, initialization of LATCH2_PIN_MODEM_CODEC still performed with
> ams_delta_latch2_write() wrapper from late_init() is now done on not
> requested GPIO pin.
>
> Reintroduce atomic initialization of latch2 pins at machine_init to
> prevent from random values potentially corrupting NAND data or maybe
> even destroing other hardware.  Also take care of MODEM/CODEC related
> pins so MODEM device probe succeeds even if latch2 GPIO device or
> dependent regulator is not ready and CODEC can be reached over the
> MODEM even if audio driver doesn't take control over
> LATCH2_PIN_MODEM_CODEC.
>
> Once done, remove the no longer needed GPIO based implementation of
> ams_delta_latch_write() and its frontend macro.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

This should turn into a nice pin control driver some day.

But until then, this looks way better after than before the patch,
so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 3/3] ARM: OMAP1: ams-delta: register MODEM device earlier

[Janusz Krzysztofik]

Amstrad Delta MODEM device used to be initialized at arch_initcall
before it was once moved to late_initcall by commit f7519d8c8290 ("ARM:
OMAP1: ams-delta: register latch dependent devices later"). The purpose
of that change was to postpone initialization of devices which depended
on latch2 pins until latch2 converted to GPIO device was ready.

After recent fixes to GPIO handling, it was possible to moove
registration of most of those device back to where they were before.
The same can be safely done with the MODEM device as initialization
of GPIO pins it depends on was moved to machine_init by preceding
patch.

Move registration of the MODEM device to arch_initcall_sync, not to
arch_initcall, so it is never exposed to potential conflictinh
registration order hazard against OMAP serial ports.

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

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index c865c6921af8..a98783f98f3a 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -896,11 +896,28 @@ static int __init modem_nreset_init(void)
 /*
  * This function expects MODEM IRQ number already assigned to the port
  * and fails if it's not.
+ * The MODEM device requires its RESET# pin kept high during probe.
+ * That requirement can be fulfilled in several ways:
+ * - with a descriptor of already functional modem_nreset regulator
+ *   assigned to the MODEM private data,
+ * - with the regulator not yet controlled by modem_pm function but
+ *   already enabled by default on probe,
+ * - before the modem_nreset regulator is probed, with the pin already
+ *   set high explicitly.
+ * The last one is already guaranteed by ams_delta_latch2_init() called
+ * from machine_init.
+ * In order to avoid taking over ttyS0 device slot, the MODEM device
+ * should be registered after OMAP serial ports.  Since those ports
+ * are registered at arch_initcall, this function can be called safely
+ * at arch_initcall_sync earliest.
  */
 static int __init ams_delta_modem_init(void)
 {
 	int err;
 
+	if (!machine_is_ams_delta())
+		return -ENODEV;
+
 	if (ams_delta_modem_ports[0].irq < 0)
 		return ams_delta_modem_ports[0].irq;
 
@@ -913,6 +930,7 @@ static int __init ams_delta_modem_init(void)
 
 	return err;
 }
+arch_initcall_sync(ams_delta_modem_init);
 
 static int __init late_init(void)
 {
@@ -922,10 +940,6 @@ static int __init late_init(void)
 	if (err)
 		return err;
 
-	err = ams_delta_modem_init();
-	if (err)
-		return err;
-
 	/*
 	 * Once the modem device is registered, the modem_nreset
 	 * regulator can be requested on behalf of that device.
-- 
2.16.4

Re: [PATCH 3/3] ARM: OMAP1: ams-delta: register MODEM device earlier

[Linus Walleij]

On Mon, Aug 20, 2018 at 8:12 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:

> Amstrad Delta MODEM device used to be initialized at arch_initcall
> before it was once moved to late_initcall by commit f7519d8c8290 ("ARM:
> OMAP1: ams-delta: register latch dependent devices later"). The purpose
> of that change was to postpone initialization of devices which depended
> on latch2 pins until latch2 converted to GPIO device was ready.
>
> After recent fixes to GPIO handling, it was possible to moove
> registration of most of those device back to where they were before.
> The same can be safely done with the MODEM device as initialization
> of GPIO pins it depends on was moved to machine_init by preceding
> patch.
>
> Move registration of the MODEM device to arch_initcall_sync, not to
> arch_initcall, so it is never exposed to potential conflictinh
> registration order hazard against OMAP serial ports.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

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

Yours,
Linus Walleij

Re: [PATCH 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Tony Lindgren]

* Janusz Krzysztofik <jmkrzyszt@gmail.com> [180820 11:16]:
> 
> Convert modem related GPIO setup from integer space to GPIO descriptors.
> Also, restore original initialization order of the MODEM device and its
> related GPIO pins.
> 
> Cleanup of MODEM relaated regulator setup is postponed while waiting for
> upcoming conversion of fixed regulator API to GPIO descriptors.
> 
> 
> Janusz Krzysztofik (3):
>       ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
>       ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
>       ARM: OMAP1: ams-delta: register MODEM device earlier

Janusz, can you please repost this series based on v4.19-rc1
with Linus' acks?

At least the header file has moved around now.

And as this also conflicts with your earlier patch
"ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor"
please repost that too in the same series.

If you have other arch/arm/*omap*/* related patches then
please repost those too, these are the only ones I still had
tagged :)

Regards,

Tony

[PATCH v2 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Janusz Krzysztofik]

Convert modem related GPIO setup from integer space to GPIO descriptors.
Also, restore original initialization order of the MODEM device and its
related GPIO pins.

Cleanup of MODEM relaated regulator setup is postponed while waiting for
upcoming conversion of fixed regulator API to GPIO descriptors.


Janusz Krzysztofik (3):
      ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
      ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
      ARM: OMAP1: ams-delta: register MODEM device earlier


Changelog:
v2:
- rebased on v4.19-rc1
- added Reviewed-by: obtained from Linus Walleij

Please ignore the patch "ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO"
sent alone on 2018-07-17.

Thanks,
Janusz


diffstat:
 board-ams-delta.c |  121 +++++++++++++++++++++++++++++++++++++++---------------
 board-ams-delta.h |    7 ---
 2 files changed, 88 insertions(+), 40 deletions(-)

[PATCH v2 1/3] ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor

[Janusz Krzysztofik]

Don't request MODEM IRQ GPIO by its global number in
ams_delta_modem_init().  Instead, obtain its GPIO descriptor
and assign related IRQ to the MODEM.  Do that from
omap_gpio_deps_init(), where the chip is already looked up.  Then, in
ams_delta_modem_init(), just check for the IRQ number having been
already assigned.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-omap1/board-ams-delta.c | 47 ++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 34cb63ff45b3..2b90b543c030 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -630,6 +630,28 @@ static struct gpiod_hog ams_delta_gpio_hogs[] = {
 	{},
 };
 
+static struct plat_serial8250_port ams_delta_modem_ports[];
+
+/*
+ * Obtain MODEM IRQ GPIO descriptor using its hardware pin
+ * number and assign related IRQ number to the MODEM port.
+ * Keep the GPIO descriptor open so nobody steps in.
+ */
+static void __init modem_assign_irq(struct gpio_chip *chip)
+{
+	struct gpio_desc *gpiod;
+
+	gpiod = gpiochip_request_own_desc(chip, AMS_DELTA_GPIO_PIN_MODEM_IRQ,
+					  "modem_irq");
+	if (IS_ERR(gpiod)) {
+		pr_err("%s: modem IRQ GPIO request failed (%ld)\n", __func__,
+		       PTR_ERR(gpiod));
+	} else {
+		gpiod_direction_input(gpiod);
+		ams_delta_modem_ports[0].irq = gpiod_to_irq(gpiod);
+	}
+}
+
 /*
  * The purpose of this function is to take care of proper initialization of
  * devices and data structures which depend on GPIO lines provided by OMAP GPIO
@@ -649,7 +671,13 @@ static void __init omap_gpio_deps_init(void)
 		return;
 	}
 
+	/*
+	 * Start with FIQ initialization as it may have to request
+	 * and release successfully each OMAP GPIO pin in turn.
+	 */
 	ams_delta_init_fiq(chip, &ams_delta_serio_device);
+
+	modem_assign_irq(chip);
 }
 
 static void __init ams_delta_init(void)
@@ -844,20 +872,18 @@ static int __init modem_nreset_init(void)
 }
 
 
+/*
+ * This function expects MODEM IRQ number already assigned to the port
+ * and fails if it's not.
+ */
 static int __init ams_delta_modem_init(void)
 {
 	int err;
 
-	omap_cfg_reg(M14_1510_GPIO2);
-	ams_delta_modem_ports[0].irq =
-			gpio_to_irq(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
+	if (ams_delta_modem_ports[0].irq < 0)
+		return ams_delta_modem_ports[0].irq;
 
-	err = gpio_request(AMS_DELTA_GPIO_PIN_MODEM_IRQ, "modem");
-	if (err) {
-		pr_err("Couldn't request gpio pin for modem\n");
-		return err;
-	}
-	gpio_direction_input(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
+	omap_cfg_reg(M14_1510_GPIO2);
 
 	/* Initialize the modem_nreset regulator consumer before use */
 	modem_priv.regulator = ERR_PTR(-ENODEV);
@@ -866,8 +892,6 @@ static int __init ams_delta_modem_init(void)
 			AMS_DELTA_LATCH2_MODEM_CODEC);
 
 	err = platform_device_register(&ams_delta_modem_device);
-	if (err)
-		gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
 
 	return err;
 }
@@ -898,7 +922,6 @@ static int __init late_init(void)
 
 unregister:
 	platform_device_unregister(&ams_delta_modem_device);
-	gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
 	return err;
 }
 
-- 
2.16.4

[PATCH v2 2/3] ARM: OMAP1: ams-delta: initialize latch2 pins to safe values

[Janusz Krzysztofik]

Latch2 pins control a number of on-board devices, namely LCD, NAND,
MODEM and CODEC.  Those pins used to be initialized with safe values
from init_machine before that operation was:
1) moved to late_initcall in preparation for conversion of latch2 to
GPIO device - see commit f7519d8c8290 ("ARM: OMAP1: ams-delta: register
latch dependent devices later"),
2) replaced with non-atomic initialization performed by means of
gpio_request_array() - see commit 937eb4bb0058 ("ARM: OMAP1: ams-delta:
convert latches to basic_mmio_gpio"),
3) made completely asynchronous by delegation of GPIO request
operations performed on subsets of pins to respective device drivers in
subsequent commits.

One visible negative result of that disintegration was corrupt keyboard
data reported by serio driver, recently fixed by commit 41f8fee385a0
("ARM: OMAP1: ams-delta: Hog "keybrd_dataout" GPIO pin").

Moreover, initialization of LATCH2_PIN_MODEM_CODEC still performed with
ams_delta_latch2_write() wrapper from late_init() is now done on not
requested GPIO pin.

Reintroduce atomic initialization of latch2 pins at machine_init to
prevent from random values potentially corrupting NAND data or maybe
even destroing other hardware.  Also take care of MODEM/CODEC related
pins so MODEM device probe succeeds even if latch2 GPIO device or
dependent regulator is not ready and CODEC can be reached over the
MODEM even if audio driver doesn't take control over
LATCH2_PIN_MODEM_CODEC.

Once done, remove the no longer needed GPIO based implementation of
ams_delta_latch_write() and its frontend macro.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-omap1/board-ams-delta.c | 52 +++++++++++++++++++++++------------
 arch/arm/mach-omap1/board-ams-delta.h |  7 -----
 2 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 2b90b543c030..1d451142248c 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -321,20 +321,6 @@ struct modem_private_data {
 
 static struct modem_private_data modem_priv;
 
-void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value)
-{
-	int bit = 0;
-	u16 bitpos = 1 << bit;
-
-	for (; bit < ngpio; bit++, bitpos = bitpos << 1) {
-		if (!(mask & bitpos))
-			continue;
-		else
-			gpio_set_value(base + bit, (value & bitpos) != 0);
-	}
-}
-EXPORT_SYMBOL(ams_delta_latch_write);
-
 static struct resource ams_delta_nand_resources[] = {
 	[0] = {
 		.start	= OMAP1_MPUIO_BASE,
@@ -680,6 +666,40 @@ static void __init omap_gpio_deps_init(void)
 	modem_assign_irq(chip);
 }
 
+/*
+ * Initialize latch2 pins with values which are safe for dependent on-board
+ * devices or useful for their successull initialization even before GPIO
+ * driver takes control over the latch pins:
+ * - LATCH2_PIN_LCD_VBLEN	= 0
+ * - LATCH2_PIN_LCD_NDISP	= 0	Keep LCD device powered off before its
+ *					driver takes control over it.
+ * - LATCH2_PIN_NAND_NCE	= 0
+ * - LATCH2_PIN_NAND_NWP	= 0	Keep NAND device down and write-
+ *					protected before its driver takes
+ *					control over it.
+ * - LATCH2_PIN_KEYBRD_PWR	= 0	Keep keyboard powered off before serio
+ *					driver takes control over it.
+ * - LATCH2_PIN_KEYBRD_DATAOUT	= 0	Keep low to avoid corruption of first
+ *					byte of data received from attached
+ *					keyboard when serio device is probed;
+ *					the pin is also hogged low by the latch2
+ *					GPIO driver as soon as it is ready.
+ * - LATCH2_PIN_MODEM_NRESET	= 1	Enable voice MODEM device, allowing for
+ *					its successful probe even before a
+ *					regulator it depends on, which in turn
+ *					takes control over the pin, is set up.
+ * - LATCH2_PIN_MODEM_CODEC	= 1	Attach voice MODEM CODEC data port
+ *					to the MODEM so the CODEC is under
+ *					control even if audio driver doesn't
+ *					take it over.
+ */
+static void __init ams_delta_latch2_init(void)
+{
+	u16 latch2 = 1 << LATCH2_PIN_MODEM_NRESET | 1 << LATCH2_PIN_MODEM_CODEC;
+
+	__raw_writew(latch2, LATCH2_VIRT);
+}
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -701,6 +721,7 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J18_1610_CAM_D7);
 
 	omap_gpio_deps_init();
+	ams_delta_latch2_init();
 	gpiod_add_hogs(ams_delta_gpio_hogs);
 
 	omap_serial_init();
@@ -888,9 +909,6 @@ static int __init ams_delta_modem_init(void)
 	/* Initialize the modem_nreset regulator consumer before use */
 	modem_priv.regulator = ERR_PTR(-ENODEV);
 
-	ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
-			AMS_DELTA_LATCH2_MODEM_CODEC);
-
 	err = platform_device_register(&ams_delta_modem_device);
 
 	return err;
diff --git a/arch/arm/mach-omap1/board-ams-delta.h b/arch/arm/mach-omap1/board-ams-delta.h
index 1fbada29431a..a74a306d7e77 100644
--- a/arch/arm/mach-omap1/board-ams-delta.h
+++ b/arch/arm/mach-omap1/board-ams-delta.h
@@ -59,13 +59,6 @@
 #define AMS_DELTA_LATCH2_GPIO_BASE	AMS_DELTA_GPIO_PIN_LCD_VBLEN
 #define AMS_DELTA_LATCH2_NGPIO		16
 
-#ifndef __ASSEMBLY__
-void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value);
-#define ams_delta_latch2_write(mask, value) \
-	ams_delta_latch_write(AMS_DELTA_LATCH2_GPIO_BASE, \
-			AMS_DELTA_LATCH2_NGPIO, (mask), (value))
-#endif
-
 #endif /* CONFIG_MACH_AMS_DELTA */
 
 #endif /* __ASM_ARCH_OMAP_AMS_DELTA_H */
-- 
2.16.4

[PATCH v2 3/3] ARM: OMAP1: ams-delta: register MODEM device earlier

[Janusz Krzysztofik]

Amstrad Delta MODEM device used to be initialized at arch_initcall
before it was once moved to late_initcall by commit f7519d8c8290 ("ARM:
OMAP1: ams-delta: register latch dependent devices later"). The purpose
of that change was to postpone initialization of devices which depended
on latch2 pins until latch2 converted to GPIO device was ready.

After recent fixes to GPIO handling, it was possible to moove
registration of most of those device back to where they were before.
The same can be safely done with the MODEM device as initialization
of GPIO pins it depends on was moved to machine_init by preceding
patch.

Move registration of the MODEM device to arch_initcall_sync, not to
arch_initcall, so it is never exposed to potential conflict in
registration order hazard against OMAP serial ports.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-omap1/board-ams-delta.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 1d451142248c..667c3c1f05f5 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -896,11 +896,28 @@ static int __init modem_nreset_init(void)
 /*
  * This function expects MODEM IRQ number already assigned to the port
  * and fails if it's not.
+ * The MODEM device requires its RESET# pin kept high during probe.
+ * That requirement can be fulfilled in several ways:
+ * - with a descriptor of already functional modem_nreset regulator
+ *   assigned to the MODEM private data,
+ * - with the regulator not yet controlled by modem_pm function but
+ *   already enabled by default on probe,
+ * - before the modem_nreset regulator is probed, with the pin already
+ *   set high explicitly.
+ * The last one is already guaranteed by ams_delta_latch2_init() called
+ * from machine_init.
+ * In order to avoid taking over ttyS0 device slot, the MODEM device
+ * should be registered after OMAP serial ports.  Since those ports
+ * are registered at arch_initcall, this function can be called safely
+ * at arch_initcall_sync earliest.
  */
 static int __init ams_delta_modem_init(void)
 {
 	int err;
 
+	if (!machine_is_ams_delta())
+		return -ENODEV;
+
 	if (ams_delta_modem_ports[0].irq < 0)
 		return ams_delta_modem_ports[0].irq;
 
@@ -913,6 +930,7 @@ static int __init ams_delta_modem_init(void)
 
 	return err;
 }
+arch_initcall_sync(ams_delta_modem_init);
 
 static int __init late_init(void)
 {
@@ -922,10 +940,6 @@ static int __init late_init(void)
 	if (err)
 		return err;
 
-	err = ams_delta_modem_init();
-	if (err)
-		return err;
-
 	/*
 	 * Once the modem device is registered, the modem_nreset
 	 * regulator can be requested on behalf of that device.
-- 
2.16.4

Re: [PATCH v2 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Janusz Krzysztofik]

On Monday, September 10, 2018 1:44:16 AM CEST Janusz Krzysztofik wrote:
> 
> Convert modem related GPIO setup from integer space to GPIO descriptors.
> Also, restore original initialization order of the MODEM device and its
> related GPIO pins.
> 
> Cleanup of MODEM relaated regulator setup is postponed while waiting for
> upcoming conversion of fixed regulator API to GPIO descriptors.
> 
> 
> Janusz Krzysztofik (3):
>       ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
>       ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
>       ARM: OMAP1: ams-delta: register MODEM device earlier
> 
> 
> Changelog:
> v2:
> - rebased on v4.19-rc1

Hi Tony,

I forgot to mention: please apply on top of "[PATCH v3 0/3] ARM: OMAP1: ams-
delta: Complete driver gpiod migration" series. Sorry.

Thanks,
Janusz

> - added Reviewed-by: obtained from Linus Walleij
> 
> Please ignore the patch "ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO"
> sent alone on 2018-07-17.
> 
> Thanks,
> Janusz
> 
> 
> diffstat:
>  board-ams-delta.c |  121 ++++++++++++++++++++++++++++++++++++++
+---------------
>  board-ams-delta.h |    7 ---
>  2 files changed, 88 insertions(+), 40 deletions(-)
> 
> 

Re: [PATCH v2 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Tony Lindgren]

* Janusz Krzysztofik <jmkrzyszt@gmail.com> [180910 21:52]:
> On Monday, September 10, 2018 1:44:16 AM CEST Janusz Krzysztofik wrote:
> > 
> > Convert modem related GPIO setup from integer space to GPIO descriptors.
> > Also, restore original initialization order of the MODEM device and its
> > related GPIO pins.
> > 
> > Cleanup of MODEM relaated regulator setup is postponed while waiting for
> > upcoming conversion of fixed regulator API to GPIO descriptors.
> > 
> > 
> > Janusz Krzysztofik (3):
> >       ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
> >       ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
> >       ARM: OMAP1: ams-delta: register MODEM device earlier
> > 
> > 
> > Changelog:
> > v2:
> > - rebased on v4.19-rc1
> 
> Hi Tony,
> 
> I forgot to mention: please apply on top of "[PATCH v3 0/3] ARM: OMAP1: ams-
> delta: Complete driver gpiod migration" series. Sorry.

Hmm but then in that thread you said that it won't apply.. So I'm applying
this series into omap-for-v4.20/omap1 branch. I just modified the second
patch in this series for the unchanged header location.

I suggest you send the mtd patch separately to mtd maintainers, the fb patch
separately to fb maintainers and then let's do the header move after all
those are merged as a clean-up. That avoids some dependencies between these
branches.

Regards,

Tony

Re: [PATCH v2 0/3] ARM: OMAP1: ams-delta: Clean up GPIO setup for MODEM

[Janusz Krzysztofik]

Hi Tony,

On Thursday, September 20, 2018 11:23:26 PM CEST Tony Lindgren wrote:
> * Janusz Krzysztofik <jmkrzyszt@gmail.com> [180910 21:52]:
> > On Monday, September 10, 2018 1:44:16 AM CEST Janusz Krzysztofik wrote:
> > > 
> > > Convert modem related GPIO setup from integer space to GPIO descriptors.
> > > Also, restore original initialization order of the MODEM device and its
> > > related GPIO pins.
> > > 
> > > Cleanup of MODEM relaated regulator setup is postponed while waiting for
> > > upcoming conversion of fixed regulator API to GPIO descriptors.
> > > 
> > > 
> > > Janusz Krzysztofik (3):
> > >       ARM: OMAP1: ams-delta: assign MODEM IRQ from GPIO descriptor
> > >       ARM: OMAP1: ams-delta: initialize latch2 pins to safe values
> > >       ARM: OMAP1: ams-delta: register MODEM device earlier
> > > 
> > > 
> > > Changelog:
> > > v2:
> > > - rebased on v4.19-rc1
> > 
> > Hi Tony,
> > 
> > I forgot to mention: please apply on top of "[PATCH v3 0/3] ARM: OMAP1: ams-
> > delta: Complete driver gpiod migration" series. Sorry.
> 
> Hmm but then in that thread you said that it won't apply.. So I'm applying
> this series into omap-for-v4.20/omap1 branch. I just modified the second
> patch in this series for the unchanged header location.

OK, thank you.
Janusz

> I suggest you send the mtd patch separately to mtd maintainers, the fb patch
> separately to fb maintainers and then let's do the header move after all
> those are merged as a clean-up. That avoids some dependencies between these
> branches.
> 
> Regards,
> 
> Tony
>