2018-05-27 13:56:17

by Janusz Krzysztofik

[permalink] [raw]
Subject: [PATCH 1/2] ARM: OMAP1: ams-delta: refactor late_init()

Before the board level GPIO handling is converted from GPIO numbers to
GPIO descriptors, split late_init() into functional blocks and move
them to separate functions.

While being at it, drop machine type check from late_init() - the
function is now called from the board init_late callback so there is
no need for yet another applicability check.

Signed-off-by: Janusz Krzysztofik <[email protected]>
---
arch/arm/mach-omap1/board-ams-delta.c | 55 +++++++++++++++++++++++++++--------
1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276..cdba8decc532 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -643,19 +643,19 @@ static struct platform_device ams_delta_modem_device = {
},
};

-static int __init late_init(void)
+static int __init ams_delta_gpio_init(void)
{
int err;

- if (!machine_is_ams_delta())
- return -ENODEV;
-
err = gpio_request_array(latch_gpios, ARRAY_SIZE(latch_gpios));
- if (err) {
+ if (err)
pr_err("Couldn't take over latch1/latch2 GPIO pins\n");
- return err;
- }

+ return err;
+}
+
+static void __init ams_delta_late_devices(void)
+{
platform_add_devices(late_devices, ARRAY_SIZE(late_devices));

/*
@@ -666,12 +666,23 @@ static int __init late_init(void)
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));
+}
+
+static int __init modem_nreset_init(void)
+{
+ int err;

err = platform_device_register(&modem_nreset_device);
- if (err) {
+ if (err)
pr_err("Couldn't register the modem regulator device\n");
- return err;
- }
+
+ return err;
+}
+
+
+static int __init ams_delta_modem_init(void)
+{
+ int err;

omap_cfg_reg(M14_1510_GPIO2);
ams_delta_modem_ports[0].irq =
@@ -692,7 +703,28 @@ static int __init late_init(void)

err = platform_device_register(&ams_delta_modem_device);
if (err)
- goto gpio_free;
+ gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
+
+ return err;
+}
+
+static int __init late_init(void)
+{
+ int err;
+
+ err = ams_delta_gpio_init();
+ if (err)
+ return err;
+
+ ams_delta_late_devices();
+
+ err = modem_nreset_init();
+ if (err)
+ return err;
+
+ err = ams_delta_modem_init();
+ if (err)
+ return err;

/*
* Once the modem device is registered, the modem_nreset
@@ -708,7 +740,6 @@ static int __init late_init(void)

unregister:
platform_device_unregister(&ams_delta_modem_device);
-gpio_free:
gpio_free(AMS_DELTA_GPIO_PIN_MODEM_IRQ);
return err;
}
--
2.16.1



2018-05-27 13:56:36

by Janusz Krzysztofik

[permalink] [raw]
Subject: [PATCH 2/2] ARM: OMAP1: ams-delta: assign LED GPIO numbers from descriptors

Assign a label to latch1 GPIO device the LEDs hang off, enumerate its
pins for the purpose of indexing gpio_led table, remove hardcoded GPIO
numbers from that table replacing them with invalid GPIO numbers and
remove initialization of incompletely described LED device from
machine_init.

As soon as the latch1 GPIO device is registered, use its label to find
respective GPIO chip, identify each LED's GPIO descriptor by its pin
number and assign its gobal GPIO number to the gpio_led table. Once
completed, register the LED device.

Created and tested against linux-v4.17-rc3.

Signed-off-by: Janusz Krzysztofik <[email protected]>
---
arch/arm/mach-omap1/board-ams-delta.c | 116 ++++++++++++++++++++++++++++------
1 file changed, 98 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index cdba8decc532..d4b26352ddde 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -179,7 +179,10 @@ static struct resource latch1_resources[] = {
},
};

+#define LATCH1_LABEL "latch1"
+
static struct bgpio_pdata latch1_pdata = {
+ .label = LATCH1_LABEL,
.base = LATCH1_GPIO_BASE,
.ngpio = LATCH1_NGPIO,
};
@@ -194,6 +197,15 @@ static struct platform_device latch1_gpio_device = {
},
};

+#define LATCH1_PIN_LED_CAMERA 0
+#define LATCH1_PIN_LED_ADVERT 1
+#define LATCH1_PIN_LED_MAIL 2
+#define LATCH1_PIN_LED_HANDSFREE 3
+#define LATCH1_PIN_LED_VOICEMAIL 4
+#define LATCH1_PIN_LED_VOICE 5
+#define LATCH1_PIN_DOCKIT1 6
+#define LATCH1_PIN_DOCKIT2 7
+
static struct resource latch2_resources[] = {
[0] = {
.name = "dat",
@@ -398,38 +410,43 @@ static struct gpiod_lookup_table ams_delta_lcd_gpio_table = {
},
};

-static const struct gpio_led gpio_leds[] __initconst = {
- {
+/*
+ * Dynamically allocated GPIO numbers must be obtained fromm GPIO device
+ * before they can be put in the gpio_led table. Before that happens,
+ * initialize the table with invalid GPIO numbers, not 0.
+ */
+static struct gpio_led gpio_leds[] __initdata = {
+ [LATCH1_PIN_LED_CAMERA] = {
.name = "camera",
- .gpio = LATCH1_GPIO_BASE + 0,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
#ifdef CONFIG_LEDS_TRIGGERS
.default_trigger = "ams_delta_camera",
#endif
},
- {
+ [LATCH1_PIN_LED_ADVERT] = {
.name = "advert",
- .gpio = LATCH1_GPIO_BASE + 1,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
},
- {
+ [LATCH1_PIN_LED_MAIL] = {
.name = "email",
- .gpio = LATCH1_GPIO_BASE + 2,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
},
- {
+ [LATCH1_PIN_LED_HANDSFREE] = {
.name = "handsfree",
- .gpio = LATCH1_GPIO_BASE + 3,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
},
- {
+ [LATCH1_PIN_LED_VOICEMAIL] = {
.name = "voicemail",
- .gpio = LATCH1_GPIO_BASE + 4,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
},
- {
+ [LATCH1_PIN_LED_VOICE] = {
.name = "voice",
- .gpio = LATCH1_GPIO_BASE + 5,
+ .gpio = -EINVAL,
.default_state = LEDS_GPIO_DEFSTATE_OFF,
},
};
@@ -542,6 +559,22 @@ static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
&ams_delta_nand_gpio_table,
};

+/*
+ * Some drivers may not use GPIO lookup tables but need to be provided
+ * with GPIO numbers. The same applies to GPIO based IRQ lines - some
+ * drivers may even not use GPIO layer but expect just IRQ numbers.
+ * We could either define GPIO lookup tables then use them on behalf
+ * of those devices, or we can use GPIO driver level methods for
+ * identification of GPIO and IRQ numbers. For the purpose of the latter,
+ * defina a helper function which identifies GPIO chips by their labels.
+ */
+static int gpiochip_match_by_label(struct gpio_chip *chip, void *data)
+{
+ char *label = data;
+
+ return !strcmp(label, chip->label);
+}
+
static void __init ams_delta_init(void)
{
/* mux pins for uarts */
@@ -571,7 +604,6 @@ static void __init ams_delta_init(void)
led_trigger_register_simple("ams_delta_camera",
&ams_delta_camera_led_trigger);
#endif
- gpio_led_register_device(-1, &leds_pdata);
platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));

/*
@@ -643,16 +675,68 @@ static struct platform_device ams_delta_modem_device = {
},
};

+/*
+ * leds-gpio driver doesn't make use of GPIO lookup tables,
+ * it has to be provided with GPIO numbers over platform data
+ * if GPIO descriptor info can't be obtained from device tree.
+ * We could either define GPIO lookup tables and use them on behalf
+ * of the leds-gpio device, or we can use GPIO driver level methods
+ * for identification of GPIO numbers as long as we don't support
+ * device tree. Let's do the latter.
+ */
+static void __init ams_delta_led_init(struct gpio_chip *chip)
+{
+ struct gpio_desc *gpiod;
+ int i;
+
+ for (i = LATCH1_PIN_LED_CAMERA; i < LATCH1_PIN_DOCKIT1; i++) {
+ gpiod = gpiochip_request_own_desc(chip, i, NULL);
+ if (IS_ERR(gpiod)) {
+ pr_warn("%s: %s GPIO %d request failed (%ld)\n",
+ __func__, LATCH1_LABEL, i, PTR_ERR(gpiod));
+ continue;
+ }
+
+ /* Assign GPIO numbers to LED device. */
+ gpio_leds[i].gpio = desc_to_gpio(gpiod);
+
+ gpiochip_free_own_desc(gpiod);
+ }
+
+ gpio_led_register_device(PLATFORM_DEVID_NONE, &leds_pdata);
+}
+
+/*
+ * The purpose of this function is to take care of assignment of GPIO numbers
+ * to platform devices which depend on GPIO lines provided by Amstrad Delta
+ * latch1 and/or latch2 GPIO devices but don't use GPIO lookup tables.
+ * The function may be called as soon as latch1/latch2 GPIO devices are
+ * initilized. Since basic-mmio-gpio driver is not registered before
+ * device_initcall, this may happen at erliest during device_initcall_sync.
+ * Dependent devices shouldn't be registered before that, their
+ * registration may be performed from within this function or later.
+ */
static int __init ams_delta_gpio_init(void)
{
+ struct gpio_chip *chip;
int err;

+ if (!machine_is_ams_delta())
+ return -ENODEV;
+
+ chip = gpiochip_find(LATCH1_LABEL, gpiochip_match_by_label);
+ if (!chip)
+ pr_err("%s: latch1 GPIO chip not found\n", __func__);
+ else
+ ams_delta_led_init(chip);
+
err = gpio_request_array(latch_gpios, ARRAY_SIZE(latch_gpios));
if (err)
pr_err("Couldn't take over latch1/latch2 GPIO pins\n");

return err;
}
+device_initcall_sync(ams_delta_gpio_init);

static void __init ams_delta_late_devices(void)
{
@@ -712,10 +796,6 @@ static int __init late_init(void)
{
int err;

- err = ams_delta_gpio_init();
- if (err)
- return err;
-
ams_delta_late_devices();

err = modem_nreset_init();
--
2.16.1


2018-07-02 12:57:37

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: OMAP1: ams-delta: refactor late_init()

* Janusz Krzysztofik <[email protected]> [180527 06:58]:
> Before the board level GPIO handling is converted from GPIO numbers to
> GPIO descriptors, split late_init() into functional blocks and move
> them to separate functions.
>
> While being at it, drop machine type check from late_init() - the
> function is now called from the board init_late callback so there is
> no need for yet another applicability check.

Applying both into omap-for-v4.19/omap1 thanks.

Tony