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 <[email protected]>
---
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.
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 <[email protected]>
---
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 <[email protected]>
---
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. 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 <[email protected]>
---
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
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 <[email protected]>
---
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.
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 <[email protected]>
---
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
On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
<[email protected]> 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
>
> <[email protected]> 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?
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.
> > 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.
> > }
> >
> > +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 <[email protected]> wrote:
> On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
>> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
>>
>> <[email protected]> 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?
>
> 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.
>> > +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 <[email protected]>
wrote:
> > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> >>
> >> <[email protected]> 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?
> >
> > 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;
>
> >> > +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
<[email protected]> wrote:
> On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
>> On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <[email protected]>
> wrote:
>> 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?
>> >> > +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
>
> <[email protected]> wrote:
> > On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> >> On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <[email protected]>
> >
> > wrote:
> >> 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 <[email protected]> wrote:
> On Sunday, May 20, 2018 4:44:31 PM CEST Andy Shevchenko wrote:
>> 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
Hello,
On Sun, 20 May 2018 19:17:04 +0300, Andy Shevchenko
<[email protected]> wrote:
> >> 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.
Do we actually have the possibility to rename this gpio? I guess no
since it would break DT backward compatibility. Otherwise it would have
been more descriptive to call it something like 'gpio-rb'.
Anyway, if you find the time, documentation for this controller would be
welcome!
Thanks,
Miquèl
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:
> > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <[email protected]>
> wrote:
> > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > >>
> > >> <[email protected]> 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?
> > >
> > > 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;
>
> >
> > >> > +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
> [email protected]
> 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:
> > On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> > > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <[email protected]>
> > wrote:
> > > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > > >>
> > > >> <[email protected]> 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?
> > > >
> > > > 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.
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:
> 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 <[email protected]>
> ---
> 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;
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.
>
> 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
>
--
Dmitry
On Sun, May 20, 2018 at 8:25 PM, Miquel Raynal
<[email protected]> wrote:
> On Sun, 20 May 2018 19:17:04 +0300, Andy Shevchenko
> <[email protected]> wrote:
> Do we actually have the possibility to rename this gpio? I guess no
> since it would break DT backward compatibility.
No we don't.
> Otherwise it would have
> been more descriptive to call it something like 'gpio-rb'.
"gpio" prefix, actually "gpios" suffix is a mandatory part of the
name. For sake of convenience it's not used in API calls.
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
Hi,
* Janusz Krzysztofik <[email protected]> [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 <[email protected]> [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?
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:
> > 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:
> > > > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik
> > > > <[email protected]>
> > >
> > > wrote:
> > > > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > > > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > > > >>
> > > > >> <[email protected]> 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?
> > > > >
> > > > > 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.
>
> 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:
> > 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:
> > > > On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> > > > > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik
> > > > > <[email protected]>
> > > >
> > > > wrote:
> > > > > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > > > > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > > > > >>
> > > > > >> <[email protected]> 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?
> > > > > >
> > > > > > 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.
> >
> > 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 <[email protected]> [180521 10:07]:
> 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 <[email protected]>
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 <[email protected]> [180521 10:07]:
> > 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 <[email protected]>
>
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
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" (already applied to
omap-for-v4.18/soc tree).
Changes since v1:
- fix handling of devm_gpiod_get_optional() return values - thanks to
Andy Shevchenko.
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++++------------------
1 file changed, 64 insertions(+), 56 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..524ceaf12de0 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,17 @@ 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) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto err_gpiod;
}
+
+ if (gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,7 +205,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 +258,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 +276,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
On Friday, May 18, 2018 11:09:52 PM CEST 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.
>
> 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 <[email protected]>
> ---
> 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);
Hi Janusz,
On Sat, 26 May 2018 00:20:45 +0200
Janusz Krzysztofik <[email protected]> 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.
>
> 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" (already applied to
> omap-for-v4.18/soc tree).
>
> Changes since v1:
> - fix handling of devm_gpiod_get_optional() return values - thanks to
> Andy Shevchenko.
Can you put the changelog after the "---" separator so that it does not
appear in the final commit message?
>
> Signed-off-by: Janusz Krzysztofik <[email protected]>
> ---
> drivers/mtd/nand/raw/ams-delta.c | 120 +++++++++++++++++++++------------------
> 1 file changed, 64 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
> index 37a3cc21c7bc..524ceaf12de0 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,17 @@ 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) {
> - this->dev_ready = ams_delta_nand_ready;
> - } else {
> - this->dev_ready = NULL;
> - pr_notice("Couldn't request gpio for Delta NAND ready.\n");
> +
> + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> + if (IS_ERR(gpiod_rdy)) {
> + err = PTR_ERR(gpiod_rdy);
> + dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> + goto err_gpiod;
> }
> +
> + if (gpiod_rdy)
> + this->dev_ready = ams_delta_nand_ready;
> +
> /* 25 us command delay time */
> this->chip_delay = 30;
> this->ecc.mode = NAND_ECC_SOFT;
> @@ -230,7 +205,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;
Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER)
here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
because it's returned when there's no entry matching the requested gpio
in the lookup table, and deferring the probe won't solve this problem.
Regards,
Boris
[1]https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/gpio/gpiolib.c#L3525
On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> Hi Janusz,
Hi Boris,
> On Sat, 26 May 2018 00:20:45 +0200
> Janusz Krzysztofik <[email protected]> wrote:
> > ...
> > Changes since v1:
> > - fix handling of devm_gpiod_get_optional() return values - thanks to
> > Andy Shevchenko.
>
> Can you put the changelog after the "---" separator so that it does not
> appear in the final commit message?
Yes, sure, sorry for that.
> > +err_gpiod:
> > + if (err == -ENODEV || err == -ENOENT)
> > + err = -EPROBE_DEFER;
>
> Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER)
> here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> because it's returned when there's no entry matching the requested gpio
> in the lookup table, and deferring the probe won't solve this problem.
ENOENT is also returned when no matching lookup table is found. That may
happen if consumer dev_name stored in the table differs from dev_name assigned
to the consumer by its bus, the platform bus in this case. For that reason I
think the consumer dev_name should be initialized in the table after the
device is registered, when its actual dev_name can be obtained. If that device
registration happens after the driver is already registered, e.g., at
late_initcall, the device is probed before its lookup table is ready. For that
reason returning EPROBE_DEFER seems better to me even in the ENOENT case.
Thanks,
Janusz
On Wed, 30 May 2018 19:43:09 +0200
Janusz Krzysztofik <[email protected]> wrote:
> On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > Hi Janusz,
>
> Hi Boris,
>
> > On Sat, 26 May 2018 00:20:45 +0200
> > Janusz Krzysztofik <[email protected]> wrote:
> > > ...
> > > Changes since v1:
> > > - fix handling of devm_gpiod_get_optional() return values - thanks to
> > > Andy Shevchenko.
> >
> > Can you put the changelog after the "---" separator so that it does not
> > appear in the final commit message?
>
> Yes, sure, sorry for that.
>
> > > +err_gpiod:
> > > + if (err == -ENODEV || err == -ENOENT)
> > > + err = -EPROBE_DEFER;
> >
> > Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER)
> > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > because it's returned when there's no entry matching the requested gpio
> > in the lookup table, and deferring the probe won't solve this problem.
>
> ENOENT is also returned when no matching lookup table is found. That may
> happen if consumer dev_name stored in the table differs from dev_name assigned
> to the consumer by its bus, the platform bus in this case. For that reason I
> think the consumer dev_name should be initialized in the table after the
> device is registered, when its actual dev_name can be obtained. If that device
> registration happens after the driver is already registered, e.g., at
> late_initcall, the device is probed before its lookup table is ready. For that
> reason returning EPROBE_DEFER seems better to me even in the ENOENT case.
Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
in board files, especially if the GPIO is used by a platform device?
When would you have a lookup table registered later in the init/boot
process?
On Wednesday, May 30, 2018 7:52:20 PM CEST Boris Brezillon wrote:
> On Wed, 30 May 2018 19:43:09 +0200
>
> Janusz Krzysztofik <[email protected]> wrote:
> > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > > Hi Janusz,
> >
> > Hi Boris,
> >
> > > On Sat, 26 May 2018 00:20:45 +0200
> > >
> > > Janusz Krzysztofik <[email protected]> wrote:
> > > > ...
> > > > Changes since v1:
> > > > - fix handling of devm_gpiod_get_optional() return values - thanks to
> > > >
> > > > Andy Shevchenko.
> > >
> > > Can you put the changelog after the "---" separator so that it does not
> > > appear in the final commit message?
> >
> > Yes, sure, sorry for that.
> >
> > > > +err_gpiod:
> > > > + if (err == -ENODEV || err == -ENOENT)
> > > > + err = -EPROBE_DEFER;
> > >
> > > Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER)
> > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > > because it's returned when there's no entry matching the requested gpio
> > > in the lookup table, and deferring the probe won't solve this problem.
> >
> > ENOENT is also returned when no matching lookup table is found. That may
> > happen if consumer dev_name stored in the table differs from dev_name
> > assigned to the consumer by its bus, the platform bus in this case. For
> > that reason I think the consumer dev_name should be initialized in the
> > table after the device is registered, when its actual dev_name can be
> > obtained. If that device registration happens after the driver is already
> > registered, e.g., at late_initcall, the device is probed before its
> > lookup table is ready. For that reason returning EPROBE_DEFER seems
> > better to me even in the ENOENT case.
> Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
> in board files, especially if the GPIO is used by a platform device?
> When would you have a lookup table registered later in the init/boot
> process?
When e.g. I'd like to register my GPIO consumer platform device at
late_initcall for some reason, and I'm not sure what exact dev_name my
consomer will be registered with by the platform bus. In that case I think I
should assign dev_name to the lookup table after the consumer device is
registered and its exact dev_name can be obtained, then register the table,
Am I missing something?
Thanks,
Janusz
On Wed, 30 May 2018 22:39:03 +0200
Janusz Krzysztofik <[email protected]> wrote:
> On Wednesday, May 30, 2018 7:52:20 PM CEST Boris Brezillon wrote:
> > On Wed, 30 May 2018 19:43:09 +0200
> >
> > Janusz Krzysztofik <[email protected]> wrote:
> > > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > > > Hi Janusz,
> > >
> > > Hi Boris,
> > >
> > > > On Sat, 26 May 2018 00:20:45 +0200
> > > >
> > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > ...
> > > > > Changes since v1:
> > > > > - fix handling of devm_gpiod_get_optional() return values - thanks to
> > > > >
> > > > > Andy Shevchenko.
> > > >
> > > > Can you put the changelog after the "---" separator so that it does not
> > > > appear in the final commit message?
> > >
> > > Yes, sure, sorry for that.
> > >
> > > > > +err_gpiod:
> > > > > + if (err == -ENODEV || err == -ENOENT)
> > > > > + err = -EPROBE_DEFER;
> > > >
> > > > Hm, isn't it better to make gpiod_find() return ERR_PTR(-EPROBE_DEFER)
> > > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > > > because it's returned when there's no entry matching the requested gpio
> > > > in the lookup table, and deferring the probe won't solve this problem.
> > >
> > > ENOENT is also returned when no matching lookup table is found. That may
> > > happen if consumer dev_name stored in the table differs from dev_name
> > > assigned to the consumer by its bus, the platform bus in this case. For
> > > that reason I think the consumer dev_name should be initialized in the
> > > table after the device is registered, when its actual dev_name can be
> > > obtained. If that device registration happens after the driver is already
> > > registered, e.g., at late_initcall, the device is probed before its
> > > lookup table is ready. For that reason returning EPROBE_DEFER seems
> > > better to me even in the ENOENT case.
> > Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
> > in board files, especially if the GPIO is used by a platform device?
> > When would you have a lookup table registered later in the init/boot
> > process?
>
> When e.g. I'd like to register my GPIO consumer platform device at
> late_initcall for some reason, and I'm not sure what exact dev_name my
> consomer will be registered with by the platform bus.
You should know the name before the device is registered.
> In that case I think I
> should assign dev_name to the lookup table after the consumer device is
> registered and its exact dev_name can be obtained, then register the table,
I'm pretty sure it's not supposed to work like that. Resources attached
to a device should be defined before the device is registered, not
after, simply because when you call platform_device_register(), the
device might be directly bind to the driver before the
platform_device_register() calls return, and the driver will fail to
probe the device if it doesn't find the GPIO it needs.
On Monday, June 4, 2018 11:55:43 AM CEST Boris Brezillon wrote:
> On Wed, 30 May 2018 22:39:03 +0200
>
> Janusz Krzysztofik <[email protected]> wrote:
> > On Wednesday, May 30, 2018 7:52:20 PM CEST Boris Brezillon wrote:
> > > On Wed, 30 May 2018 19:43:09 +0200
> > >
> > > Janusz Krzysztofik <[email protected]> wrote:
> > > > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > > > > Hi Janusz,
> > > >
> > > > Hi Boris,
> > > >
> > > > > On Sat, 26 May 2018 00:20:45 +0200
> > > > >
> > > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > > ...
> > > > > > Changes since v1:
> > > > > > - fix handling of devm_gpiod_get_optional() return values - thanks
> > > > > > to
> > > > > >
> > > > > > Andy Shevchenko.
> > > > >
> > > > > Can you put the changelog after the "---" separator so that it does
> > > > > not
> > > > > appear in the final commit message?
> > > >
> > > > Yes, sure, sorry for that.
> > > >
> > > > > > +err_gpiod:
> > > > > > + if (err == -ENODEV || err == -ENOENT)
> > > > > > + err = -EPROBE_DEFER;
> > > > >
> > > > > Hm, isn't it better to make gpiod_find() return
> > > > > ERR_PTR(-EPROBE_DEFER)
> > > > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > > > > because it's returned when there's no entry matching the requested
> > > > > gpio
> > > > > in the lookup table, and deferring the probe won't solve this
> > > > > problem.
> > > >
> > > > ENOENT is also returned when no matching lookup table is found. That
> > > > may
> > > > happen if consumer dev_name stored in the table differs from dev_name
> > > > assigned to the consumer by its bus, the platform bus in this case.
> > > > For
> > > > that reason I think the consumer dev_name should be initialized in the
> > > > table after the device is registered, when its actual dev_name can be
> > > > obtained. If that device registration happens after the driver is
> > > > already
> > > > registered, e.g., at late_initcall, the device is probed before its
> > > > lookup table is ready. For that reason returning EPROBE_DEFER seems
> > > > better to me even in the ENOENT case.
> > >
> > > Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
> > > in board files, especially if the GPIO is used by a platform device?
> > > When would you have a lookup table registered later in the init/boot
> > > process?
> >
> > When e.g. I'd like to register my GPIO consumer platform device at
> > late_initcall for some reason, and I'm not sure what exact dev_name my
> > consomer will be registered with by the platform bus.
>
> You should know the name before the device is registered.
What if I use PLATFORM_DEVID_AUTO?
For other cases, if bus specific names of devices were supposed to be known
before registration, bus drivers should export functions returning those names
from initialized bus specific device structures or their components while they
don't. Under such circumstances, we end up hardcoding device names based on
our knowledge of bus internals if we need to specify them in advance, and
those internals are not guaranteed to never change.
> > In that case I think I
> > should assign dev_name to the lookup table after the consumer device is
> > registered and its exact dev_name can be obtained, then register the
> > table,
>
> I'm pretty sure it's not supposed to work like that. Resources attached
> to a device should be defined before the device is registered, not
> after,
What do you mean by resources attached to a device? I don't think we should
consider GPIO lookup tables as consumer device resources. Those tables are
registered separately from consumer device registration and I know of no
requirement for registering them in advance. Maybe I'm missing something.
Let's have a look at regulators. There are no separately registered regulator
lookup tables, instead regulator consumer supply tables are attached to bus
specific device structures of regulator devices, not their consumers, hence
registered together with providers, not consumers. Will you still call those
tables 'resources attached to' consumers?
As far as I can see, regulator_get() never returns -EINVAL, only -ENODEV or -
EPROBE_DEFER. However, gpiod_get() can also return -EINVAL. Maybe it
shouldn't, but it does, and I'm just trying to adopt to that in order to not
break a driver I'm trying to update.
> simply because when you call platform_device_register(), the
> device might be directly bind to the driver before the
> platform_device_register() calls return, and the driver will fail to
> probe the device if it doesn't find the GPIO it needs.
That's exactly the case I'm talking about, but my conclusion is different: the
driver should fail softly so the device is probed again later, as long as I'm
not wrong the no requirement exists for registering GPIO lookup tables before
related consumers are registered.
If I' missing something or you are still not convinced, I'll try to resolve
issues with the device I see in a different way and submit a new patch that
hopefully matches your requirements.
Thanks,
Janusz
On Mon, 04 Jun 2018 18:48:08 +0200
Janusz Krzysztofik <[email protected]> wrote:
> On Monday, June 4, 2018 11:55:43 AM CEST Boris Brezillon wrote:
> > On Wed, 30 May 2018 22:39:03 +0200
> >
> > Janusz Krzysztofik <[email protected]> wrote:
> > > On Wednesday, May 30, 2018 7:52:20 PM CEST Boris Brezillon wrote:
> > > > On Wed, 30 May 2018 19:43:09 +0200
> > > >
> > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > > > > > Hi Janusz,
> > > > >
> > > > > Hi Boris,
> > > > >
> > > > > > On Sat, 26 May 2018 00:20:45 +0200
> > > > > >
> > > > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > > > ...
> > > > > > > Changes since v1:
> > > > > > > - fix handling of devm_gpiod_get_optional() return values - thanks
> > > > > > > to
> > > > > > >
> > > > > > > Andy Shevchenko.
> > > > > >
> > > > > > Can you put the changelog after the "---" separator so that it does
> > > > > > not
> > > > > > appear in the final commit message?
> > > > >
> > > > > Yes, sure, sorry for that.
> > > > >
> > > > > > > +err_gpiod:
> > > > > > > + if (err == -ENODEV || err == -ENOENT)
> > > > > > > + err = -EPROBE_DEFER;
> > > > > >
> > > > > > Hm, isn't it better to make gpiod_find() return
> > > > > > ERR_PTR(-EPROBE_DEFER)
> > > > > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > > > > > because it's returned when there's no entry matching the requested
> > > > > > gpio
> > > > > > in the lookup table, and deferring the probe won't solve this
> > > > > > problem.
> > > > >
> > > > > ENOENT is also returned when no matching lookup table is found. That
> > > > > may
> > > > > happen if consumer dev_name stored in the table differs from dev_name
> > > > > assigned to the consumer by its bus, the platform bus in this case.
> > > > > For
> > > > > that reason I think the consumer dev_name should be initialized in the
> > > > > table after the device is registered, when its actual dev_name can be
> > > > > obtained. If that device registration happens after the driver is
> > > > > already
> > > > > registered, e.g., at late_initcall, the device is probed before its
> > > > > lookup table is ready. For that reason returning EPROBE_DEFER seems
> > > > > better to me even in the ENOENT case.
> > > >
> > > > Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
> > > > in board files, especially if the GPIO is used by a platform device?
> > > > When would you have a lookup table registered later in the init/boot
> > > > process?
> > >
> > > When e.g. I'd like to register my GPIO consumer platform device at
> > > late_initcall for some reason, and I'm not sure what exact dev_name my
> > > consomer will be registered with by the platform bus.
> >
> > You should know the name before the device is registered.
>
> What if I use PLATFORM_DEVID_AUTO?
In this case, the id assigned to the device will be dependent on the
device registration order, which in case of C board files should be
predictable. Am I missing something?
>
> For other cases, if bus specific names of devices were supposed to be known
> before registration, bus drivers should export functions returning those names
> from initialized bus specific device structures or their components while they
> don't. Under such circumstances, we end up hardcoding device names based on
> our knowledge of bus internals if we need to specify them in advance, and
> those internals are not guaranteed to never change.
But, when it comes to C board files, device names are known ahead of
time, right? And the only use case I see for those gpio_lookup_table is
non-DT platforms.
I had a quick look at a few call-sites of gpiod_add_lookup_table() and
couldn't find an example where the gpio lookup table was registered
after the platform device it was meant to be used by. If you have such
an example, can you point it to me?
>
> > > In that case I think I
> > > should assign dev_name to the lookup table after the consumer device is
> > > registered and its exact dev_name can be obtained, then register the
> > > table,
> >
> > I'm pretty sure it's not supposed to work like that. Resources attached
> > to a device should be defined before the device is registered, not
> > after,
>
> What do you mean by resources attached to a device? I don't think we should
> consider GPIO lookup tables as consumer device resources.
Why? If the device requires a GPIO to be controlled by the SoC, why
shouldn't we consider the GPIO as a resource. Sure, it's not part of
the struct resource array attached to the platform_device, but it's
still a resource that needs to be available for the driver to correctly
probe the device.
> Those tables are
> registered separately from consumer device registration and I know of no
> requirement for registering them in advance. Maybe I'm missing something.
But that doesn't make sense. Why would you register a device, and only
then attach it the description of the GPIOs it needs. Think about
the DT case, and imagine you support DT overlays, it's like having an
overlay that defines your device, and then another overlay that adds the
xx-gpios props to this device on top.
>
> Let's have a look at regulators. There are no separately registered regulator
> lookup tables, instead regulator consumer supply tables are attached to bus
> specific device structures of regulator devices, not their consumers, hence
> registered together with providers, not consumers. Will you still call those
> tables 'resources attached to' consumers?
Except the gpio-lookup tables are not here to describe GPIOs, but to
give consumer-oriented friendly names so that the consumer can then do
devm_gpiod_get(&pdev->dev, "foo", GPIOD_OUT_LOW);
Here "foo" is only meaningful to pdev, and not globally exposed, and
another driver might pretty well request the exact same GPIO using a
different name.
>
> As far as I can see, regulator_get() never returns -EINVAL, only -ENODEV or -
> EPROBE_DEFER. However, gpiod_get() can also return -EINVAL. Maybe it
> shouldn't, but it does, and I'm just trying to adopt to that in order to not
> break a driver I'm trying to update.
Well, IMO it should return EINVAL if no entry in the registered lookup
tables is matching the requested "dev_name(dev) + con_id" pair. I'm
bringing back my analogy with the DT case. What you're suggesting
would be similar to waiting for a new xxx-gpios prop to magically
appear in the DT.
Note that the GPIO itself is not necessarily ready to be used when the
consumer calls devm_gpio_get(), because the GPIO chip might not be
probed yet or it might be missing a dependency. And that's all fine, we
have EPROBE_DEFER for that. But that's different than saying "we don't
have a GPIO description attached to the device, let's wait for someone
to create one".
>
> > simply because when you call platform_device_register(), the
> > device might be directly bind to the driver before the
> > platform_device_register() calls return, and the driver will fail to
> > probe the device if it doesn't find the GPIO it needs.
>
> That's exactly the case I'm talking about, but my conclusion is different: the
> driver should fail softly so the device is probed again later, as long as I'm
> not wrong the no requirement exists for registering GPIO lookup tables before
> related consumers are registered.
How long. Why should we wait only after late init calls? What if the
lookup table is created after that?
>
> If I' missing something or you are still not convinced, I'll try to resolve
> issues with the device I see in a different way and submit a new patch that
> hopefully matches your requirements.
Maybe I'm wrong, but I still think that registering a lookup table
after the pdev it will be used by is a bad idea.
Regards,
Boris
On Tue, 5 Jun 2018 01:09:32 +0200
Boris Brezillon <[email protected]> wrote:
> On Mon, 04 Jun 2018 18:48:08 +0200
> Janusz Krzysztofik <[email protected]> wrote:
>
> > On Monday, June 4, 2018 11:55:43 AM CEST Boris Brezillon wrote:
> > > On Wed, 30 May 2018 22:39:03 +0200
> > >
> > > Janusz Krzysztofik <[email protected]> wrote:
> > > > On Wednesday, May 30, 2018 7:52:20 PM CEST Boris Brezillon wrote:
> > > > > On Wed, 30 May 2018 19:43:09 +0200
> > > > >
> > > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > > On Wednesday, May 30, 2018 11:05:00 AM CEST Boris Brezillon wrote:
> > > > > > > Hi Janusz,
> > > > > >
> > > > > > Hi Boris,
> > > > > >
> > > > > > > On Sat, 26 May 2018 00:20:45 +0200
> > > > > > >
> > > > > > > Janusz Krzysztofik <[email protected]> wrote:
> > > > > > > > ...
> > > > > > > > Changes since v1:
> > > > > > > > - fix handling of devm_gpiod_get_optional() return values - thanks
> > > > > > > > to
> > > > > > > >
> > > > > > > > Andy Shevchenko.
> > > > > > >
> > > > > > > Can you put the changelog after the "---" separator so that it does
> > > > > > > not
> > > > > > > appear in the final commit message?
> > > > > >
> > > > > > Yes, sure, sorry for that.
> > > > > >
> > > > > > > > +err_gpiod:
> > > > > > > > + if (err == -ENODEV || err == -ENOENT)
> > > > > > > > + err = -EPROBE_DEFER;
> > > > > > >
> > > > > > > Hm, isn't it better to make gpiod_find() return
> > > > > > > ERR_PTR(-EPROBE_DEFER)
> > > > > > > here [1]? At least, ENOENT should not be turned into EPROBE_DEFER,
> > > > > > > because it's returned when there's no entry matching the requested
> > > > > > > gpio
> > > > > > > in the lookup table, and deferring the probe won't solve this
> > > > > > > problem.
> > > > > >
> > > > > > ENOENT is also returned when no matching lookup table is found. That
> > > > > > may
> > > > > > happen if consumer dev_name stored in the table differs from dev_name
> > > > > > assigned to the consumer by its bus, the platform bus in this case.
> > > > > > For
> > > > > > that reason I think the consumer dev_name should be initialized in the
> > > > > > table after the device is registered, when its actual dev_name can be
> > > > > > obtained. If that device registration happens after the driver is
> > > > > > already
> > > > > > registered, e.g., at late_initcall, the device is probed before its
> > > > > > lookup table is ready. For that reason returning EPROBE_DEFER seems
> > > > > > better to me even in the ENOENT case.
> > > > >
> > > > > Sorry, I don't get it. Aren't GPIO lookup tables supposed to be declared
> > > > > in board files, especially if the GPIO is used by a platform device?
> > > > > When would you have a lookup table registered later in the init/boot
> > > > > process?
> > > >
> > > > When e.g. I'd like to register my GPIO consumer platform device at
> > > > late_initcall for some reason, and I'm not sure what exact dev_name my
> > > > consomer will be registered with by the platform bus.
> > >
> > > You should know the name before the device is registered.
> >
> > What if I use PLATFORM_DEVID_AUTO?
Just had a quick look at board-ams-delta.c and you don't have a single
device setting pdev->id to PLATFORM_DEVID_AUTO. It's either set to
PLATFORM_DEVID_NONE (-1) or assigned a specific id, so the problem does
not exist, really. Just set the name .dev_id to the appropriate value
at declaration time and register the lookup tables before registering
the pdevs.
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 <[email protected]>
---
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 = {
@@ -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,23 @@ 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);
+ 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);
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.4
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.
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 <[email protected]>
---
Changlog:
v1: Fix handling of devm_gpiod_get_optional() return values - thanks to
Andy Shevchenko.
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/mtd/nand/raw/ams-delta.c | 121 ++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..09b2f9fda5b9 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,17 @@ 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) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto out_mtd;
}
+
+ if (gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,9 +205,42 @@ 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));
- if (err)
- goto out_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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(ams_delta_mtd, 1);
@@ -246,9 +254,6 @@ 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 +271,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.4
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 <[email protected]>
---
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") and commit 8853daf3b4ac ("gpiolib:
Defer on non-DT find_chip_by_name() failure") already in linux-next.
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 = {
@@ -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,23 @@ 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);
+ 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);
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.4
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.
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 <[email protected]>
---
Changlog:
v1: Fix handling of devm_gpiod_get_optional() return values - thanks to
Andy Shevchenko.
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") and commit 8853daf3b4ac ("gpiolib:
Defer on non-DT find_chip_by_name() failure") already in linux-next.
drivers/mtd/nand/raw/ams-delta.c | 121 ++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..09b2f9fda5b9 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,17 @@ 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) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto out_mtd;
}
+
+ if (gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,9 +205,42 @@ 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));
- if (err)
- goto out_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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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 out_mtd;
+ }
+ 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);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(ams_delta_mtd, 1);
@@ -246,9 +254,6 @@ 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 +271,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.4
Hi Janusz,
On Mon, 9 Jul 2018 21:38:50 +0200
Janusz Krzysztofik <[email protected]> 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.
>
> 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 <[email protected]>
> ---
> Changlog:
> v1: Fix handling of devm_gpiod_get_optional() return values - thanks to
> Andy Shevchenko.
> 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.
Sorry, but we can't apply this patch now because of the dependency on
those 2 commits. I guess it's not a big issue if we defer it to 4.20.
Alternatively, we could consider queuing it to mtd/fixes after 4.19-rc1
is out, but we'll need a good reason to do that (like a regression
that this patch is supposed to fix). Note for your future contributions:
for this kind of cross-subsystem changes, it's better to let everything
go through a single tree (usually done by sending all patches in a
single series and explaining the dependencies between the patches in
the cover letter), but it's already too late here (I guess d08605a64e67
is in the omap tree and 8853daf3b4ac in the gpio one).
Regards,
Boris
Hi Boris,
On Tuesday, July 17, 2018 9:37:36 PM CEST Boris Brezillon wrote:
> ...
> Sorry, but we can't apply this patch now because of the dependency on
> those 2 commits. I guess it's not a big issue if we defer it to 4.20.
> Alternatively, we could consider queuing it to mtd/fixes after 4.19-rc1
> is out, but we'll need a good reason to do that (like a regression
> that this patch is supposed to fix). Note for your future contributions:
> for this kind of cross-subsystem changes, it's better to let everything
> go through a single tree (usually done by sending all patches in a
> single series and explaining the dependencies between the patches in
> the cover letter),
Good advice, Sine I have in my queue a patch for OMAP that depends on this
one, and still another one with the same dependency issues, I'll take the path
you suggest and resend those three as a series to be merged via OMAP.
Thanks,
Janusz
On Tue, 17 Jul 2018 22:20:00 +0200
Janusz Krzysztofik <[email protected]> wrote:
> Hi Boris,
>
> On Tuesday, July 17, 2018 9:37:36 PM CEST Boris Brezillon wrote:
> > ...
> > Sorry, but we can't apply this patch now because of the dependency on
> > those 2 commits. I guess it's not a big issue if we defer it to 4.20.
> > Alternatively, we could consider queuing it to mtd/fixes after 4.19-rc1
> > is out, but we'll need a good reason to do that (like a regression
> > that this patch is supposed to fix). Note for your future contributions:
> > for this kind of cross-subsystem changes, it's better to let everything
> > go through a single tree (usually done by sending all patches in a
> > single series and explaining the dependencies between the patches in
> > the cover letter),
>
> Good advice, Sine I have in my queue a patch for OMAP that depends on this
> one, and still another one with the same dependency issues, I'll take the path
> you suggest and resend those three as a series to be merged via OMAP.
Sounds good.
+Tony
On Tue, 17 Jul 2018 19:05:52 +0200
Janusz Krzysztofik <[email protected]> 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.
>
> 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 <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Just a minor comment below (nothing important, just a coding style
preference).
Also, if it goes through the omap tree (I guess it will target 4.20),
I'll need an immutable tag, because I have changes touching this driver
in the pipe (that's more a request for Tony).
> @@ -230,9 +205,42 @@ 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));
> - if (err)
> - goto out_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 out_mtd;
> + }
Can you add a blank line after each if (IS_ERR(gpiod_nwp)) { } block.
> + 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 out_mtd;
> + }
> + 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 out_mtd;
> + }
> + 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 out_mtd;
> + }
> + 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 out_mtd;
> + }
> + 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);
> + goto out_mtd;
> + }
>
> /* Scan to find existence of the device */
> err = nand_scan(ams_delta_mtd, 1);
Hi,
Please ignore this submission, I'm going to send this patch with two others in
a single series.
Thanks,
Janusz
On Tuesday, July 17, 2018 6:54:00 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 <[email protected]>
> ---
> 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") and commit 8853daf3b4ac ("gpiolib:
> Defer on non-DT find_chip_by_name() failure") already in linux-next.
>
> 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 = {
> @@ -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,23 @@ 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);
> + 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);
> + return ret;
> + }
> +
> +#ifdef CONFIG_LCD_CLASS_DEVICE
> lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
> &ams_delta_lcd_ops);
>
>
This is a follow up of initial submission of a series consisted of
6 changes, 3 of which have been already applied or reworkeed.
V2 changelog:
[PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables
- already in mainline, commit 68e62a15a914
[PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
- reworked and submitted as a series, already in linux-omap,
commit 68e62a15a914 ("ARM: OMAP1: ams-delta: drop GPIO lookup
table for serio device") followed by 9 more
[PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
- already in mainline, commit d65777d1a2cd
[PATCH 4/6] fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
- resubmitting as [PATCH v2 1/3 v2]
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")
and commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") already in linux-next
[PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
- resubmitting as [PATCH v2 2/3 v4]
v2: Fix handling of devm_gpiod_get_optional() return values -
thanks to Andy Shevchenko.
v3: 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") and
commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") already in linux-next - thanks
to Boris Brezillon
v4: fix style issue - thanks to Boris Brezillon
[PATCH 6/6] ARM: OMAP1: ams-delta: make board header file local to
mach-omap1
- resending as [PATCH v2 3/3]
Dependency on commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") is not critical - it is not needed for
clean build or run, it only prevents from potential future changes to
driver initializaton order during device_initcall.
I'm submitting the three patches in series because the last one depends
on the other two.
Thanks,
Janusz
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.
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
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-omap and commit
8853daf3b4ac ("gpiolib: Defer on non-DT find_chip_by_name()
failure") already in linux-next
Dependency on commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") is not critical for clean build or run, it
only prevents from unexpected future changes to driver initialization
order durin device_initall.
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 = {
@@ -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,23 @@ 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);
+ 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);
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.4
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.
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 <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
---
Changelog:
v2: Fix handling of devm_gpiod_get_optional() return values - thanks to
Andy Shevchenko
v3: 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-omap and commit
8853daf3b4ac ("gpiolib: Defer on non-DT find_chip_by_name()
failure") already in linux-next - thanks to Boris Brezillon
v4: Fix style issue - thanks to Boris Brezillon
Dependency on commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") is not critical for clean build or run, it
only prevents from unexpected future changes to driver initialization
order during device_initall.
drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
1 file changed, 67 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..2a8872ebd14a 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,17 @@ 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) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto out_mtd;
}
+
+ if (gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,9 +205,47 @@ 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));
- if (err)
- goto out_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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(ams_delta_mtd, 1);
@@ -246,9 +259,6 @@ 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 +276,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.4
Now as board header file is no longer included by drivers, move it to
the root directory of mach-omap1.
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
Depends on patches 1/3 and 2/3 of the series.
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 | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
rename arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h (98%)
diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
index ddc27638ba2a..2e2a17364dc9 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
+++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
@@ -17,9 +17,9 @@
#include <linux/platform_data/ams-delta-fiq.h>
#include <asm/assembler.h>
-#include <mach/board-ams-delta.h>
#include "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 b0dc7ddf5877..14c3d3f5255e 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -22,11 +22,10 @@
#include <linux/platform_data/ams-delta-fiq.h>
#include <linux/platform_device.h>
-#include <mach/board-ams-delta.h>
-
#include <asm/fiq.h>
#include "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 dd28d2614d7f..34cb63ff45b3 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 <mach/usb.h>
#include "ams-delta-fiq.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 98%
rename from arch/arm/mach-omap1/include/mach/board-ams-delta.h
rename to arch/arm/mach-omap1/board-ams-delta.h
index ad6f865d1f16..1fbada29431a 100644
--- a/arch/arm/mach-omap1/include/mach/board-ams-delta.h
+++ b/arch/arm/mach-omap1/board-ams-delta.h
@@ -1,5 +1,5 @@
/*
- * arch/arm/plat-omap/include/mach/board-ams-delta.h
+ * arch/arm/mach-omap1/board-ams-delta.h
*
* Copyright (C) 2006 Jonathan McDowell <[email protected]>
*
--
2.16.4
Hi Janusz, Tony
Janusz Krzysztofik <[email protected]> wrote on Wed, 18 Jul 2018
01:14:47 +0200:
> 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.
>
> 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 <[email protected]>
> Acked-by: Boris Brezillon <[email protected]>
> ---
Acked-by: Miquel Raynal <[email protected]>
As suggested by Boris, we expect this series to go through the OMAP
tree so we'll need an immutable tag, there are more changes on this
driver coming in.
Thanks!
Miquèl
Hi Janusz,
On mer., juil. 18 2018, Janusz Krzysztofik <[email protected]> wrote:
> This is a follow up of initial submission of a series consisted of
> 6 changes, 3 of which have been already applied or reworkeed.
>
> V2 changelog:
> [PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables
> - already in mainline, commit 68e62a15a914
> [PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
> - reworked and submitted as a series, already in linux-omap,
> commit 68e62a15a914 ("ARM: OMAP1: ams-delta: drop GPIO lookup
> table for serio device") followed by 9 more
> [PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
> - already in mainline, commit d65777d1a2cd
> [PATCH 4/6] fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
> - resubmitting as [PATCH v2 1/3 v2]
> 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")
> and commit 8853daf3b4ac ("gpiolib: Defer on non-DT
> find_chip_by_name() failure") already in linux-next
> [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
> - resubmitting as [PATCH v2 2/3 v4]
> v2: Fix handling of devm_gpiod_get_optional() return values -
> thanks to Andy Shevchenko.
> v3: 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") and
> commit 8853daf3b4ac ("gpiolib: Defer on non-DT
> find_chip_by_name() failure") already in linux-next - thanks
> to Boris Brezillon
> v4: fix style issue - thanks to Boris Brezillon
> [PATCH 6/6] ARM: OMAP1: ams-delta: make board header file local to
> mach-omap1
> - resending as [PATCH v2 3/3]
>
> Dependency on commit 8853daf3b4ac ("gpiolib: Defer on non-DT
> find_chip_by_name() failure") is not critical - it is not needed for
> clean build or run, it only prevents from potential future changes to
> driver initializaton order during device_initcall.
>
> I'm submitting the three patches in series because the last one depends
> on the other two.
I think that being in CC in this series is a mistake as I don't see
anything related what I have done in this series.
Gregory
>
> Thanks,
> Janusz
>
--
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
* Miquel Raynal <[email protected]> [180718 07:24]:
> Hi Janusz, Tony
>
> Janusz Krzysztofik <[email protected]> wrote on Wed, 18 Jul 2018
> 01:14:47 +0200:
>
> > 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.
> >
> > 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 <[email protected]>
> > Acked-by: Boris Brezillon <[email protected]>
> > ---
>
> Acked-by: Miquel Raynal <[email protected]>
>
> As suggested by Boris, we expect this series to go through the OMAP
> tree so we'll need an immutable tag, there are more changes on this
> driver coming in.
Let's wait for v4.19-rc1 to clear the dependencies. I'm not applying
new patches at this point only dealing with the pending pull requests
I have.
Regards,
Tony
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.
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
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 = {
@@ -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,23 @@ 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);
+ 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);
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.4
Now as board header file is no longer included by drivers, move it to
the root directory of mach-omap1.
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
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 | 2 +-
4 files changed, 4 insertions(+), 5 deletions(-)
rename arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h (98%)
diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
index ddc27638ba2a..2e2a17364dc9 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
+++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
@@ -17,9 +17,9 @@
#include <linux/platform_data/ams-delta-fiq.h>
#include <asm/assembler.h>
-#include <mach/board-ams-delta.h>
#include "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 b0dc7ddf5877..14c3d3f5255e 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -22,11 +22,10 @@
#include <linux/platform_data/ams-delta-fiq.h>
#include <linux/platform_device.h>
-#include <mach/board-ams-delta.h>
-
#include <asm/fiq.h>
#include "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 dd28d2614d7f..34cb63ff45b3 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 <mach/usb.h>
#include "ams-delta-fiq.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 98%
rename from arch/arm/mach-omap1/include/mach/board-ams-delta.h
rename to arch/arm/mach-omap1/board-ams-delta.h
index ad6f865d1f16..1fbada29431a 100644
--- a/arch/arm/mach-omap1/include/mach/board-ams-delta.h
+++ b/arch/arm/mach-omap1/board-ams-delta.h
@@ -1,5 +1,5 @@
/*
- * arch/arm/plat-omap/include/mach/board-ams-delta.h
+ * arch/arm/mach-omap1/board-ams-delta.h
*
* Copyright (C) 2006 Jonathan McDowell <[email protected]>
*
--
2.16.4
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.
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 <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Miquel Raynal <[email protected]>
---
drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
1 file changed, 67 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..2a8872ebd14a 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,17 @@ 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) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto out_mtd;
}
+
+ if (gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,9 +205,47 @@ 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));
- if (err)
- goto out_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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(ams_delta_mtd, 1);
@@ -246,9 +259,6 @@ 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 +276,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.4
This is a follow up of initial submission of a series consisted of
6 changes, 3 of which have been already applied or reworkeed.
Janusz Krzysztofik (3):
video: fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
mtd: rawnand: ams-delta: use GPIO lookup table
ARM: OMAP1: ams-delta: make board header file local to mach-omap1
I'm submitting the three patches in series because the last one depends
on the other two.
Tony, please set up an immutable tag for this series to be used by MTD
as there are more changes on Amstrad Delta NAND driver coming in.
Thanks,
Janusz
Changelog:
v3:
- rebased on top of v4.19-rc1
- added Acked-by: received from Miquel
v2:
[PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables
- already in mainline, commit 68e62a15a914
[PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
- reworked and submitted as a series, already in linux-omap,
commit 68e62a15a914 ("ARM: OMAP1: ams-delta: drop GPIO lookup
table for serio device") followed by 9 more
[PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
- already in mainline, commit d65777d1a2cd
[PATCH 4/6] fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
- resubmitting as [PATCH v2 1/3 v2]
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")
and commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") already in linux-next
[PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
- resubmitting as [PATCH v2 2/3 v4]
v2: Fix handling of devm_gpiod_get_optional() return values -
thanks to Andy Shevchenko.
v3: 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") and
commit 8853daf3b4ac ("gpiolib: Defer on non-DT
find_chip_by_name() failure") already in linux-next - thanks
to Boris Brezillon
v4: fix style issue - thanks to Boris Brezillon
[PATCH 6/6] ARM: OMAP1: ams-delta: make board header file local to
mach-omap1
- resending as [PATCH v2 3/3]
All dependencies mentioned in v2 changelog are satisfied in v4.19-rc1.
diffstat:
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/board-ams-delta.h | 2
drivers/mtd/nand/raw/ams-delta.c | 126 ++++++++++++++--------------
drivers/video/fbdev/omap/lcd_ams_delta.c | 55 ++++--------
6 files changed, 93 insertions(+), 97 deletions(-)
On Mon, Sep 10, 2018 at 12:55 AM Janusz Krzysztofik <[email protected]> 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.
>
> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Janusz Krzysztofik <[email protected]>
Good work as usual:
Reviewed-by: Linus Walleij <[email protected]>
FWIW I think the entire drivers/video/fbdev/omap/*
could be pretty easy to migrate to DRM if you compare
the simple drivers/gpu/drm/pl111 or drivers/gpu/drm/tve200
drivers. Just inspiration! :)
Yours,
Linus Walleij
On Monday, September 10, 2018 12:56:02 AM CEST Janusz Krzysztofik wrote:
>
> This is a follow up of initial submission of a series consisted of
> 6 changes, 3 of which have been already applied or reworkeed.
>
>
> Janusz Krzysztofik (3):
> video: fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
> mtd: rawnand: ams-delta: use GPIO lookup table
Hi Tony,
Please ignore this patch. It may no longer be possible to merged it cleanly
with nand/next tree. I'll exclude it from the series, rebase on top of nand/
next and submit via linux-mtd.
That shouldn't affect the two remaining patches of the series which should
still apply and merge cleanly, but I can resend them renumbered if you wish.
Thanks,
Janusz
> ARM: OMAP1: ams-delta: make board header file local to mach-omap1
>
>
> I'm submitting the three patches in series because the last one depends
> on the other two.
>
> Tony, please set up an immutable tag for this series to be used by MTD
> as there are more changes on Amstrad Delta NAND driver coming in.
>
> Thanks,
> Janusz
>
>
> Changelog:
> v3:
> - rebased on top of v4.19-rc1
> - added Acked-by: received from Miquel
>
> v2:
> [PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables
> - already in mainline, commit 68e62a15a914
> [PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
> - reworked and submitted as a series, already in linux-omap,
> commit 68e62a15a914 ("ARM: OMAP1: ams-delta: drop GPIO lookup
> table for serio device") followed by 9 more
> [PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
> - already in mainline, commit d65777d1a2cd
> [PATCH 4/6] fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
> - resubmitting as [PATCH v2 1/3 v2]
> 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")
> and commit 8853daf3b4ac ("gpiolib: Defer on non-DT
> find_chip_by_name() failure") already in linux-next
> [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
> - resubmitting as [PATCH v2 2/3 v4]
> v2: Fix handling of devm_gpiod_get_optional() return values -
> thanks to Andy Shevchenko.
> v3: 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") and
> commit 8853daf3b4ac ("gpiolib: Defer on non-DT
> find_chip_by_name() failure") already in linux-next - thanks
> to Boris Brezillon
> v4: fix style issue - thanks to Boris Brezillon
> [PATCH 6/6] ARM: OMAP1: ams-delta: make board header file local to
> mach-omap1
> - resending as [PATCH v2 3/3]
>
> All dependencies mentioned in v2 changelog are satisfied in v4.19-rc1.
>
>
> diffstat:
> 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/board-ams-delta.h | 2
> drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++
+--------------
> drivers/video/fbdev/omap/lcd_ams_delta.c | 55 ++++--------
> 6 files changed, 93 insertions(+), 97 deletions(-)
>
>
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.
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 <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
Acked-by: Miquel Raynal <[email protected]>
---
Changelog:
v5:
- rebased on nand/next
- resubmitted to linux-mtd as a stand-alone patch
v3 2/3:
- series rebased on top of v4.19-rc1
- added Acked-by: received from Miquel
v2 2/3 v4:
- fix style issue - thanks to Boris Brezillon
- resubmitted to linux-omap in a series
v3:
- 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") and commit 8853daf3b4ac ("gpiolib: Defer on
non-DT find_chip_by_name() failure") already in linux-next - thanks
to Boris Brezillon
v2:
- fix handling of devm_gpiod_get_optional() return values - thanks to
Andy Shevchenko
- resubmitted to linux-mtd as a stand-alone patch
Originally submitted to linux-omap as "[PATCH 5/6] mtd: rawnand:
ams-delta: use GPIO lookup table"
drivers/mtd/nand/raw/ams-delta.c | 126 +++++++++++++++++++++------------------
1 file changed, 67 insertions(+), 59 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 3d3786dcc5d1..a2d2dfc55984 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
@@ -69,9 +74,9 @@ static void ams_delta_write_byte(struct nand_chip *this, u_char byte)
writew(0, io_base + OMAP_MPUIO_IO_CNTL);
writew(byte, this->legacy.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 nand_chip *this)
@@ -79,11 +84,11 @@ static u_char ams_delta_read_byte(struct nand_chip *this)
u_char res;
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->legacy.IO_ADDR_R);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NRE, 1);
+ gpiod_set_value(gpiod_nre, 1);
return res;
}
@@ -118,12 +123,9 @@ static void ams_delta_hwcontrol(struct nand_chip *this, 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)
@@ -132,41 +134,9 @@ static void ams_delta_hwcontrol(struct nand_chip *this, int cmd,
static int ams_delta_nand_ready(struct nand_chip *this)
{
- 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
@@ -214,12 +184,17 @@ static int ams_delta_init(struct platform_device *pdev)
this->legacy.write_buf = ams_delta_write_buf;
this->legacy.read_buf = ams_delta_read_buf;
this->legacy.cmd_ctrl = ams_delta_hwcontrol;
- if (gpio_request(AMS_DELTA_GPIO_PIN_NAND_RB, "nand_rdy") == 0) {
- this->legacy.dev_ready = ams_delta_nand_ready;
- } else {
- this->legacy.dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (IS_ERR(gpiod_rdy)) {
+ err = PTR_ERR(gpiod_rdy);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto out_mtd;
}
+
+ if (gpiod_rdy)
+ this->legacy.dev_ready = ams_delta_nand_ready;
+
/* 25 us command delay time */
this->legacy.chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -228,9 +203,47 @@ 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));
- if (err)
- goto out_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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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 out_mtd;
+ }
+
+ 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);
+ goto out_mtd;
+ }
/* Scan to find existence of the device */
err = nand_scan(this, 1);
@@ -244,9 +257,6 @@ 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);
@@ -264,8 +274,6 @@ static int ams_delta_cleanup(struct platform_device *pdev)
/* Release resources, unregister device */
nand_release(mtd_to_nand(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.4
On Wed, Sep 19, 2018 at 3:17 PM Janusz Krzysztofik <[email protected]> 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.
>
> 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 <[email protected]>
> Acked-by: Boris Brezillon <[email protected]>
> Acked-by: Miquel Raynal <[email protected]>
> ---
> Changelog:
> v5:
> - rebased on nand/next
> - resubmitted to linux-mtd as a stand-alone patch
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
* Janusz Krzysztofik <[email protected]> [180919 18:13]:
> On Monday, September 10, 2018 12:56:02 AM CEST Janusz Krzysztofik wrote:
> >
> > This is a follow up of initial submission of a series consisted of
> > 6 changes, 3 of which have been already applied or reworkeed.
> >
> >
> > Janusz Krzysztofik (3):
> > video: fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
> > mtd: rawnand: ams-delta: use GPIO lookup table
>
> Hi Tony,
>
> Please ignore this patch. It may no longer be possible to merged it cleanly
> with nand/next tree. I'll exclude it from the series, rebase on top of nand/
> next and submit via linux-mtd.
OK sounds good to me.
> That shouldn't affect the two remaining patches of the series which should
> still apply and merge cleanly, but I can resend them renumbered if you wish.
Up to the mtd and fb folks as far as I'm concerned :)
Regards,
Tony
Hi Janusz,
Janusz Krzysztofik <[email protected]> wrote on Thu, 20 Sep 2018
00:17:29 +0200:
> 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.
>
> 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 <[email protected]>
> Acked-by: Boris Brezillon <[email protected]>
> Acked-by: Miquel Raynal <[email protected]>
Applied on nand/next.
Thanks,
Miquèl
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 <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
---
Changelog:
v4:
- added Reviewed-by: Linus Walleij
- resubmitted as a standalone patch for inclusion via fbdev tree again
v3:
- rebased on top of v4.19-rc1 with all dependencies mentioned in v2
changelog satisfied
v2/v2:
- resubmitted in a series as [PATCH v2 1/3 v2] for inclusion via omap
tree
v2:
- removed 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") and commit 8853daf3b4ac ("gpiolib: Defer on
non-DT find_chip_by_name() failure") already in linux-next
- resubmitted as a standalone patch for inclusion via fbdev tree
Ininally submitted in a series as [PATCH 4/6]
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 = {
@@ -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,23 @@ 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);
+ 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);
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.4
On 10/03/2018 03:03 PM, 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 <[email protected]>
> Reviewed-by: Linus Walleij <[email protected]>
Patch queued for 4.20, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics