Skip to contentSkip to conversation

Start getting rid of pdata callbacks with gpio_find_by_chip_name()

PATCH In review Tony Lindgren 6 participants 30 messages Mar 1, 2012 – Mar 9, 2012

Subsystems (9)
lore
Share & tools
lore

Tony Lindgren

author maintainer
#0 lore raw

cover Start getting rid of pdata callbacks with gpio_find_by_chip_name()

Hi all,

This series adds gpio_find_by_name() that allows finding GPIOs on specific gpio_chips. As the GPIO numbers can be dynamic, it's hard to find the GPIO numbers from drivers using them directly.

So far we've dealt with this using platform specific callbacks, but that is messy. This series removes the needs for these callbacks for omap hsmmc driver. Further callbacks can be removed people are OK with adding gpio_find_by_name().

This series is based on the omap fixes-non-critical that's needed for the arch/arm/mach-omap2 parts of this series.

Regards,

Tony

---

Tony Lindgren (4):
gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()
mmc: omap_hsmmc: Use GPIO offset for external GPIO chips
mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

 arch/arm/mach-omap2/board-3430sdp.c          |   13 +-
 arch/arm/mach-omap2/board-4430sdp.c          |   45 --------
 arch/arm/mach-omap2/board-cm-t35.c           |    8 -
 arch/arm/mach-omap2/board-devkit8000.c       |    7 -
 arch/arm/mach-omap2/board-igep0020.c         |    8 -
 arch/arm/mach-omap2/board-omap3beagle.c      |    9 +-
 arch/arm/mach-omap2/board-omap3evm.c         |    8 -
 arch/arm/mach-omap2/board-omap3pandora.c     |   13 +-
 arch/arm/mach-omap2/board-omap3stalker.c     |    8 -
 arch/arm/mach-omap2/board-omap3touchbook.c   |    7 -
 arch/arm/mach-omap2/board-omap4panda.c       |   52 ----------
 arch/arm/mach-omap2/board-zoom-peripherals.c |    7 -
 arch/arm/mach-omap2/hsmmc.c                  |    3 +
 arch/arm/mach-omap2/hsmmc.h                  |    5 +
 arch/arm/plat-omap/include/plat/mmc.h        |    3 +
 drivers/gpio/gpio-twl4030.c                  |    2 
 drivers/gpio/gpiolib.c                       |   47 +++++++++
 drivers/mfd/twl6030-irq.c                    |   33 +++---
 drivers/mmc/host/omap_hsmmc.c                |  140 +++++++++++++++++++-------
 include/asm-generic/gpio.h                   |    3 -
 20 files changed, 204 insertions(+), 217 deletions(-)
Signature · 1 line
Signature
author maintainer
#1 lore raw

PATCH 1/4 gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Currently there is no way for drivers to request a GPIO on a particular gpio chip. This makes it hard to support multiple GPIO controllers with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.

Make it easier for device drivers to find GPIOs on a specific gpio_chip by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().

Note that as gpiochip_find() is already exported, we may as well export gpiochip_find_by_name() too.

Cc Grant Likely <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Rajendra Nayak <[email protected]>
---
 drivers/gpio/gpiolib.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    3 ++-
 2 files changed, 49 insertions(+), 1 deletions(-)
drivers/gpio/gpiolib.c +47 -0
index 17fdf4b..0e5bf55 100644
@@ -1173,6 +1173,53 @@ struct gpio_chip *gpiochip_find(void *data,
 }
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
+static int match_name(struct gpio_chip *chip, void *data)
+{
+	const char *name = data;
+
+	return sysfs_streq(name, chip->label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);
+}
+EXPORT_SYMBOL_GPL(gpiochip_find_by_name);
+
+/**
+ * gpio_find_by_chip_name() - find a gpio on a specific gpio_chip
+ * @chip_name: name of the gpio_chip
+ * @gpio_offset: offset of the gpio on the gpio_chip
+ *
+ * Note that gpiochip_find_by_name returns the first matching
+ * gpio_chip name. For more complex matching, see gpio_find.
+ */
+int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset)
+{
+	struct gpio_chip *chip;
+	int gpio, res;
+
+	chip = gpiochip_find_by_name(chip_name);
+	if (!chip)
+		return -ENODEV;
+
+	gpio = chip->base + gpio_offset;
+
+	return gpio;
+}
+EXPORT_SYMBOL_GPL(gpio_find_by_chip_name);
+
 /* These "optional" allocation calls help prevent drivers from stomping
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
include/asm-generic/gpio.h +2 -1
index 1ff4e22..d7a2100 100644
@@ -145,7 +145,8 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
 extern struct gpio_chip *gpiochip_find(void *data,
 					int (*match)(struct gpio_chip *chip,
 						     void *data));
-
+extern struct gpio_chip *gpiochip_find_by_name(const char *name);
+extern int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset);
 
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.
#2 lore raw

PATCH 1/4 gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 9 lines · Tony Lindgren

Currently there is no way for drivers to request a GPIO on a particular gpio chip. This makes it hard to support multiple GPIO controllers with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.

Make it easier for device drivers to find GPIOs on a specific gpio_chip by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().

Note that as gpiochip_find() is already exported, we may as well export gpiochip_find_by_name() too.

How is the device going to know the name of the gpio controller? I don't particularly like interfaces that find devices by-names since I don't think device names can be relied to be stable when devices are instantiated from device tree data.

Quoted 16 lines
Cc Grant Likely <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Rajendra Nayak <[email protected]>
---
 drivers/gpio/gpiolib.c     |   47 ++++++++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    3 ++-
 2 files changed, 49 insertions(+), 1 deletions(-)
drivers/gpio/gpiolib.c +0 -0
index 17fdf4b..0e5bf55 100644

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1176

Quoted 4 lines
drivers/gpio/gpiolib.c +1 -0
@@ -1173,6 +1173,53 @@ struct gpio_chip *gpiochip_find(void *data,
 }
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
+static int match_name(struct gpio_chip *chip, void *data)

Even though this is a static, please keep the prefix to avoid namespace conflicts. gpiochip_match_name()

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1178

Quoted 2 lines
drivers/gpio/gpiolib.c +2 -0
+{
+	const char *name = data;

This is unnecessary; the void* can be passed directly to sysfs_streq... but why is sysfs_streq being used here instead of strcmp? This is not sysfs related code.

Quoted 8 lines
drivers/gpio/gpiolib.c +8 -0
+
+	return sysfs_streq(name, chip->label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1196

Quoted 10 lines
drivers/gpio/gpiolib.c +10 -0
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

Quoted 44 lines
drivers/gpio/gpiolib.c +26 -0
+}
+EXPORT_SYMBOL_GPL(gpiochip_find_by_name);
+
+/**
+ * gpio_find_by_chip_name() - find a gpio on a specific gpio_chip
+ * @chip_name: name of the gpio_chip
+ * @gpio_offset: offset of the gpio on the gpio_chip
+ *
+ * Note that gpiochip_find_by_name returns the first matching
+ * gpio_chip name. For more complex matching, see gpio_find.
+ */
+int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset)
+{
+	struct gpio_chip *chip;
+	int gpio, res;
+
+	chip = gpiochip_find_by_name(chip_name);
+	if (!chip)
+		return -ENODEV;
+
+	gpio = chip->base + gpio_offset;
+
+	return gpio;
+}
+EXPORT_SYMBOL_GPL(gpio_find_by_chip_name);
+
 /* These "optional" allocation calls help prevent drivers from stomping
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
include/asm-generic/gpio.h +2 -1
index 1ff4e22..d7a2100 100644
@@ -145,7 +145,8 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
 extern struct gpio_chip *gpiochip_find(void *data,
 					int (*match)(struct gpio_chip *chip,
 						     void *data));
-
+extern struct gpio_chip *gpiochip_find_by_name(const char *name);
+extern int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset);
 
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.
Signature · 1 line
email sent from notmuch.vim plugin
author maintainer
#3 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Hi,

Correcting a typo in the LKML address..

Quoted 8 lines · Grant Likely
Quoted 7 lines · Tony Lindgren

Currently there is no way for drivers to request a GPIO on a particular gpio chip. This makes it hard to support multiple GPIO controllers with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.

Make it easier for device drivers to find GPIOs on a specific gpio_chip by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().

Quoted 7 lines
Quoted 2 lines

Note that as gpiochip_find() is already exported, we may as well export gpiochip_find_by_name() too.

How is the device going to know the name of the gpio controller? I don't particularly like interfaces that find devices by-names since I don't think device names can be relied to be stable when devices are instantiated from device tree data.

The gpio_chip name + gpio offset is coming from pdata until things are converted over to use DT.

For DT, this should not be needed, this is needed for removing callbacks in pdata so we can clean up some drivers and keep them working with both pdata and DT until things are converted over to DT.

Quoted 11 lines
Quoted 1 line

+static int match_name(struct gpio_chip *chip, void *data)

Even though this is a static, please keep the prefix to avoid namespace conflicts. gpiochip_match_name()

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1178

Quoted 2 lines
drivers/gpio/gpiolib.c +2 -0
+{
+	const char *name = data;

This is unnecessary; the void* can be passed directly to sysfs_streq... but why is sysfs_streq being used here instead of strcmp? This is not sysfs related code.

That comes from the bus code, will take a look.

Quoted 13 lines
Quoted 13 lines
drivers/gpio/gpiolib.c +13 -0
+
+	return sysfs_streq(name, chip->label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
Quoted 8 lines

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1196

Quoted 5 lines
drivers/gpio/gpiolib.c +5 -0
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

I think so, this too comes from the bus code.

Regards,

Tony

author maintainer
#4 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 21 lines · Tony Lindgren
Quoted 11 lines · Grant Likely
Quoted 10 lines · Tony Lindgren
drivers/gpio/gpiolib.c +10 -0
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
Quoted 8 lines

drivers/gpio/gpiolib.c struct gpio_chip *gpiochip_find(void *data, · line 1196

Quoted 5 lines
drivers/gpio/gpiolib.c +5 -0
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

Quoted 1 line

I think so, this too comes from the bus code.

Looks like it can't be const as of_node_to_gpiochip uses it with a struct device_node * that for of_get_named_gpio_flags comes from of_parse_phandle_with_args.

Regards,

Tony

#5 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 22 lines · Tony Lindgren
Quoted 21 lines · Tony Lindgren
Quoted 19 lines · Grant Likely
Quoted 5 lines · Tony Lindgren
(quoted diff) +5 -0
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *

Review

Quoted 10 lines
(quoted diff) +10 -0
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

Quoted 5 lines
Quoted 1 line

I think so, this too comes from the bus code.

Looks like it can't be const as of_node_to_gpiochip uses it with a struct device_node * that for of_get_named_gpio_flags comes from of_parse_phandle_with_args.

Really? It sees to work fine here:

---

drivers/gpio/gpiolib.c +3 -2
index d773540..e633a2a 100644
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
  * non-zero, this function will return to the caller and not iterate over any
  * more gpio_chips.
  */
-struct gpio_chip *gpiochip_find(void *data,
-				int (*match)(struct gpio_chip *chip, void *data))
+struct gpio_chip *gpiochip_find(const void *data,
+				int (*match)(struct gpio_chip *chip,
+					     const void *data))
 {
 	struct gpio_chip *chip = NULL;
 	unsigned long flags;
drivers/of/gpio.c +1 -1
index e034b38..bba8121 100644
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
 }
 
 /* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
 {
 	return chip->of_node == data;
 }
include/asm-generic/gpio.h +2 -2
index 1ff4e22..5f52690 100644
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
 /* add/remove chips */
 extern int gpiochip_add(struct gpio_chip *chip);
 extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
-						     void *data));
+						     const void *data));
 
 
 /* Always use the library code for GPIO management calls,
author maintainer
#6 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 23 lines · Grant Likely
Quoted 22 lines · Tony Lindgren
Quoted 21 lines · Tony Lindgren
Quoted 19 lines · Grant Likely
Quoted 5 lines · Tony Lindgren
(quoted diff) +5 -0
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *

Review

Quoted 10 lines
(quoted diff) +10 -0
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

I think so, this too comes from the bus code.

Looks like it can't be const as of_node_to_gpiochip uses it with a struct device_node * that for of_get_named_gpio_flags comes from of_parse_phandle_with_args.

Really? It sees to work fine here:

Hmm right you are. I must have missed adding the const to of_gpiochip_is_match, that's good news :)

Want to do that as a separate patch or want me to fold it in?

Tony

Quoted 48 lines

---

drivers/gpio/gpiolib.c +3 -2
index d773540..e633a2a 100644
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
  * non-zero, this function will return to the caller and not iterate over any
  * more gpio_chips.
  */
-struct gpio_chip *gpiochip_find(void *data,
-				int (*match)(struct gpio_chip *chip, void *data))
+struct gpio_chip *gpiochip_find(const void *data,
+				int (*match)(struct gpio_chip *chip,
+					     const void *data))
 {
 	struct gpio_chip *chip = NULL;
 	unsigned long flags;
drivers/of/gpio.c +1 -1
index e034b38..bba8121 100644
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
 }
 
 /* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
 {
 	return chip->of_node == data;
 }
include/asm-generic/gpio.h +2 -2
index 1ff4e22..5f52690 100644
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
 /* add/remove chips */
 extern int gpiochip_add(struct gpio_chip *chip);
 extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
-						     void *data));
+						     const void *data));
 
 
 /* Always use the library code for GPIO management calls,
#7 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 30 lines · Tony Lindgren
Quoted 29 lines · Grant Likely
Quoted 27 lines · Tony Lindgren
Quoted 21 lines · Tony Lindgren
Quoted 11 lines · Grant Likely
Quoted 10 lines · Tony Lindgren
(quoted diff) +10 -0
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find((void *)name, match_name);

Nasty cast. Can the signature for gpiochip_find be changed to accept a (const void *)?

Quoted 1 line

I think so, this too comes from the bus code.

Looks like it can't be const as of_node_to_gpiochip uses it with a struct device_node * that for of_get_named_gpio_flags comes from of_parse_phandle_with_args.

Quoted 6 lines
Quoted 1 line

Really? It sees to work fine here:

Hmm right you are. I must have missed adding the const to of_gpiochip_is_match, that's good news :)

Want to do that as a separate patch or want me to fold it in?

I've got it as a separate commit in my gpio/next branch.

g.

Quoted 48 lines
Quoted 48 lines

---

drivers/gpio/gpiolib.c +3 -2
index d773540..e633a2a 100644
@@ -1152,8 +1152,9 @@ EXPORT_SYMBOL_GPL(gpiochip_remove);
  * non-zero, this function will return to the caller and not iterate over any
  * more gpio_chips.
  */
-struct gpio_chip *gpiochip_find(void *data,
-				int (*match)(struct gpio_chip *chip, void *data))
+struct gpio_chip *gpiochip_find(const void *data,
+				int (*match)(struct gpio_chip *chip,
+					     const void *data))
 {
 	struct gpio_chip *chip = NULL;
 	unsigned long flags;
drivers/of/gpio.c +1 -1
index e034b38..bba8121 100644
@@ -229,7 +229,7 @@ void of_gpiochip_remove(struct gpio_chip *chip)
 }
 
 /* Private function for resolving node pointer to gpio_chip */
-static int of_gpiochip_is_match(struct gpio_chip *chip, void *data)
+static int of_gpiochip_is_match(struct gpio_chip *chip, const void *data)
 {
 	return chip->of_node == data;
 }
include/asm-generic/gpio.h +2 -2
index 1ff4e22..5f52690 100644
@@ -142,9 +142,9 @@ extern int __must_check gpiochip_reserve(int start, int ngpio);
 /* add/remove chips */
 extern int gpiochip_add(struct gpio_chip *chip);
 extern int __must_check gpiochip_remove(struct gpio_chip *chip);
-extern struct gpio_chip *gpiochip_find(void *data,
+extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
-						     void *data));
+						     const void *data));
 
 
 /* Always use the library code for GPIO management calls,
Signature · 1 line
email sent from notmuch.vim plugin
author maintainer
#8 lore raw

Re: [PATCH 1/4] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Quoted 5 lines · Grant Likely
Quoted 2 lines · Tony Lindgren

Want to do that as a separate patch or want me to fold it in?

I've got it as a separate commit in my gpio/next branch.

OK great, below is my patch updated against gpio/next if you want to pick it up.

Regards,

Tony

From: Tony Lindgren <[email protected]>
Date: Fri, 2 Mar 2012 11:30:23 -0800
Subject: [PATCH] gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()

Currently there is no way for drivers to request a GPIO on a particular gpio chip using platform data. This makes it hard to support multiple GPIO controllers with platform driver with dynamic GPIO and interrupt numbering, such as with CONFIG_SPARSE_IRQ.

Let's make it easier for platform device drivers to find GPIOs on a specific gpio_chip by adding two functions: gpiochip_find_by_name() and gpio_find_by_chip_name().

Note that these functions should not be used with device tree as pointed out by Grant Likely <[email protected]> as the names are instantiated from device tree.

As gpiochip_find() is already exported, we may as well export gpiochip_find_by_name() too.

Cc Grant Likely <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Rajendra Nayak <[email protected]>
drivers/gpio/gpiolib.c +45 -0
@@ -1178,6 +1178,51 @@ struct gpio_chip *gpiochip_find(const void *data,
 }
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
+static int gpiochip_match_name(struct gpio_chip *chip, const void *name)
+{
+	return !strcmp(name, chip->label);
+}
+
+/**
+ * gpiochip_find_by_name() - iterator for locating a gpio_chip by name
+ * @name: name of the gpio_chip
+ *
+ * Similar to bus_find_device_by_name. It returns a reference to the
+ * first gpio_chip with matching name. It ignores NULL and empty names.
+ * If you need to do something more complex, then use gpiochip_find.
+ */
+struct gpio_chip *gpiochip_find_by_name(const char *name)
+{
+	if (!name || !strcmp(name, ""))
+		return NULL;
+
+	return gpiochip_find(name, gpiochip_match_name);
+}
+EXPORT_SYMBOL_GPL(gpiochip_find_by_name);
+
+/**
+ * gpio_find_by_chip_name() - find a gpio on a specific gpio_chip
+ * @chip_name: name of the gpio_chip
+ * @gpio_offset: offset of the gpio on the gpio_chip
+ *
+ * Note that gpiochip_find_by_name returns the first matching
+ * gpio_chip name. For more complex matching, see gpio_find.
+ */
+int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset)
+{
+	struct gpio_chip *chip;
+	int gpio;
+
+	chip = gpiochip_find_by_name(chip_name);
+	if (!chip)
+		return -ENODEV;
+
+	gpio = chip->base + gpio_offset;
+
+	return gpio;
+}
+EXPORT_SYMBOL_GPL(gpio_find_by_chip_name);
+
 /* These "optional" allocation calls help prevent drivers from stomping
  * on each other, and help provide better diagnostics in debugfs.
  * They're called even less than the "set direction" calls.
include/asm-generic/gpio.h +2 -1
@@ -145,7 +145,8 @@ extern int __must_check gpiochip_remove(struct gpio_chip *chip);
 extern struct gpio_chip *gpiochip_find(const void *data,
 					int (*match)(struct gpio_chip *chip,
 						     const void *data));
-
+extern struct gpio_chip *gpiochip_find_by_name(const char *name);
+extern int gpio_find_by_chip_name(const char *chip_name, unsigned gpio_offset);
 
 /* Always use the library code for GPIO management calls,
  * or when sleeping may be involved.
author maintainer
#9 lore raw

PATCH 2/4 mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

In reply to Tony Lindgren

Use gpio_find_by_chip_name() to find the GPIO pins as they can be dynamically allocated on various gpio_chips.

Note that we don't want to touch the platform data as it can now specify the GPIO offset on a named gpio_chip.

This removes the need to use callbacks to set the GPIO pins in platform data.

Cc Chris Ball <[email protected]>
Cc Grant Likely <[email protected]>
Cc Rajendra Nayak <[email protected]>
---
 arch/arm/mach-omap2/hsmmc.c           |    3 +
 arch/arm/mach-omap2/hsmmc.h           |    5 ++
 arch/arm/plat-omap/include/plat/mmc.h |    3 +
 drivers/gpio/gpio-twl4030.c           |    2 +
 drivers/mmc/host/omap_hsmmc.c         |  109 +++++++++++++++++++++------------
 5 files changed, 82 insertions(+), 40 deletions(-)
arch/arm/mach-omap2/hsmmc.c +3 -0
index a97876d..dda88f7 100644
@@ -323,7 +323,10 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 
 	mmc->get_context_loss_count = hsmmc_get_context_loss;
 
+	mmc->slots[0].gpiochip_cd = c->gpiochip_cd;
 	mmc->slots[0].switch_pin = c->gpio_cd;
+
+	mmc->slots[0].gpiochip_wp = c->gpiochip_wp;
 	mmc->slots[0].gpio_wp = c->gpio_wp;
 
 	mmc->slots[0].remux = c->remux;
arch/arm/mach-omap2/hsmmc.h +5 -0
index 07831cc..ffbb78d 100644
@@ -22,8 +22,13 @@ struct omap2_hsmmc_info {
 	bool	no_off_init;	/* no power off when not in MMC sleep state */
 	bool	vcc_aux_disable_is_sleep; /* Regulator off remapped to sleep */
 	bool	deferred;	/* mmc needs a deferred probe */
+
+	char	*gpiochip_cd;	/* Optional gpiochip for gpio_cd */
 	int	gpio_cd;	/* or -EINVAL */
+
+	char	*gpiochip_wp;	/* Optional gpiochip for gpio_wp */
 	int	gpio_wp;	/* or -EINVAL */
+
 	char	*name;		/* or NULL for default */
 	struct platform_device *pdev;	/* mmc controller instance */
 	int	ocr_mask;	/* temporary HACK */
arch/arm/plat-omap/include/plat/mmc.h +3 -0
index f75946c..cbfbdc3 100644
@@ -130,7 +130,10 @@ struct omap_mmc_platform_data {
 #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
 		unsigned features;
 
+		char *gpiochip_cd;		/* optional gpiochip for card detect */
 		int switch_pin;			/* gpio (card detect) */
+
+		char *gpiochip_wp;		/* optional gpiochip for write protect */
 		int gpio_wp;			/* gpio (write protect) */
 
 		int (*set_bus_mode)(struct device *dev, int slot, int bus_mode);
drivers/gpio/gpio-twl4030.c +2 -0
index b8b4f22..d0f266c 100644
@@ -391,6 +391,7 @@ static int __devinit gpio_twl4030_debounce(u32 debounce, u8 mmc_cd)
 }
 
 static int gpio_twl4030_remove(struct platform_device *pdev);
+static struct platform_driver gpio_twl4030_driver;
 
 static int __devinit gpio_twl4030_probe(struct platform_device *pdev)
 {
@@ -430,6 +431,7 @@ no_irqs:
 				pdata->debounce, pdata->mmc_cd,
 				ret);
 
+	twl_gpiochip.label = gpio_twl4030_driver.driver.name;
 	twl_gpiochip.base = pdata->gpio_base;
 	twl_gpiochip.ngpio = TWL4030_GPIO_MAX;
 	twl_gpiochip.dev = &pdev->dev;
drivers/mmc/host/omap_hsmmc.c +69 -40

187 lines not on this page · raw message

#10 lore raw

PATCH 2/4 mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

Quoted 8 lines · Tony Lindgren

Use gpio_find_by_chip_name() to find the GPIO pins as they can be dynamically allocated on various gpio_chips.

Note that we don't want to touch the platform data as it can now specify the GPIO offset on a named gpio_chip.

This removes the need to use callbacks to set the GPIO pins in platform data.

While one of the reasons for those callbacks was to set the GPIO pins in platform data, I guess the other and more important one was to make sure the init sequencing between twl4030-gpio and the mmc device depending on it was done rightly. Doesn't this patch now leave the sequencing to work by luck (in the absence of something like deferred probe being in place) like is the case of twl6030 and mmc init sequence on OMAP4 already?

regards,
Rajendra

Quoted 280 lines
Cc Chris Ball<[email protected]>
Cc Grant Likely<[email protected]>
Cc Rajendra Nayak<[email protected]>
---
  arch/arm/mach-omap2/hsmmc.c           |    3 +
  arch/arm/mach-omap2/hsmmc.h           |    5 ++
  arch/arm/plat-omap/include/plat/mmc.h |    3 +
  drivers/gpio/gpio-twl4030.c           |    2 +
  drivers/mmc/host/omap_hsmmc.c         |  109 +++++++++++++++++++++------------
  5 files changed, 82 insertions(+), 40 deletions(-)
arch/arm/mach-omap2/hsmmc.c +3 -0
index a97876d..dda88f7 100644
@@ -323,7 +323,10 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 
  	mmc->get_context_loss_count = hsmmc_get_context_loss;
 
+	mmc->slots[0].gpiochip_cd = c->gpiochip_cd;
  	mmc->slots[0].switch_pin = c->gpio_cd;
+
+	mmc->slots[0].gpiochip_wp = c->gpiochip_wp;
  	mmc->slots[0].gpio_wp = c->gpio_wp;
 
  	mmc->slots[0].remux = c->remux;
arch/arm/mach-omap2/hsmmc.h +5 -0
index 07831cc..ffbb78d 100644
@@ -22,8 +22,13 @@ struct omap2_hsmmc_info {
  	bool	no_off_init;	/* no power off when not in MMC sleep state */
  	bool	vcc_aux_disable_is_sleep; /* Regulator off remapped to sleep */
  	bool	deferred;	/* mmc needs a deferred probe */
+
+	char	*gpiochip_cd;	/* Optional gpiochip for gpio_cd */
  	int	gpio_cd;	/* or -EINVAL */
+
+	char	*gpiochip_wp;	/* Optional gpiochip for gpio_wp */
  	int	gpio_wp;	/* or -EINVAL */
+
  	char	*name;		/* or NULL for default */
  	struct platform_device *pdev;	/* mmc controller instance */
  	int	ocr_mask;	/* temporary HACK */
arch/arm/plat-omap/include/plat/mmc.h +3 -0
index f75946c..cbfbdc3 100644
@@ -130,7 +130,10 @@ struct omap_mmc_platform_data {
  #define HSMMC_HAS_UPDATED_RESET	(1<<  1)
  		unsigned features;
 
+		char *gpiochip_cd;		/* optional gpiochip for card detect */
  		int switch_pin;			/* gpio (card detect) */
+
+		char *gpiochip_wp;		/* optional gpiochip for write protect */
  		int gpio_wp;			/* gpio (write protect) */
 
  		int (*set_bus_mode)(struct device *dev, int slot, int bus_mode);
drivers/gpio/gpio-twl4030.c +2 -0
index b8b4f22..d0f266c 100644
@@ -391,6 +391,7 @@ static int __devinit gpio_twl4030_debounce(u32 debounce, u8 mmc_cd)
  }
 
  static int gpio_twl4030_remove(struct platform_device *pdev);
+static struct platform_driver gpio_twl4030_driver;
 
  static int __devinit gpio_twl4030_probe(struct platform_device *pdev)
  {
@@ -430,6 +431,7 @@ no_irqs:
  				pdata->debounce, pdata->mmc_cd,
  				ret);
 
+	twl_gpiochip.label = gpio_twl4030_driver.driver.name;
  	twl_gpiochip.base = pdata->gpio_base;
  	twl_gpiochip.ngpio = TWL4030_GPIO_MAX;
  	twl_gpiochip.dev =&pdev->dev;
drivers/mmc/host/omap_hsmmc.c +69 -40
index fd0c661..1aa2420 100644
@@ -176,6 +176,8 @@ struct omap_hsmmc_host {
  	int			use_dma, dma_ch;
  	int			dma_line_tx, dma_line_rx;
  	int			slot_id;
+	int			gpio_cd;
+	int			gpio_wp;
  	int			got_dbclk;
  	int			response_busy;
  	int			context_loss;
@@ -192,26 +194,29 @@ struct omap_hsmmc_host {
 
  static int omap_hsmmc_card_detect(struct device *dev, int slot)
  {
-	struct omap_mmc_platform_data *mmc = dev->platform_data;
+	struct omap_hsmmc_host *host =
+		platform_get_drvdata(to_platform_device(dev));
 
  	/* NOTE: assumes card detect signal is active-low */
-	return !gpio_get_value_cansleep(mmc->slots[0].switch_pin);
+	return !gpio_get_value_cansleep(host->gpio_cd);
  }
 
  static int omap_hsmmc_get_wp(struct device *dev, int slot)
  {
-	struct omap_mmc_platform_data *mmc = dev->platform_data;
+	struct omap_hsmmc_host *host =
+		platform_get_drvdata(to_platform_device(dev));
 
  	/* NOTE: assumes write protect signal is active-high */
-	return gpio_get_value_cansleep(mmc->slots[0].gpio_wp);
+	return gpio_get_value_cansleep(host->gpio_wp);
  }
 
  static int omap_hsmmc_get_cover_state(struct device *dev, int slot)
  {
-	struct omap_mmc_platform_data *mmc = dev->platform_data;
+	struct omap_hsmmc_host *host =
+		platform_get_drvdata(to_platform_device(dev));
 
  	/* NOTE: assumes card detect signal is active-low */
-	return !gpio_get_value_cansleep(mmc->slots[0].switch_pin);
+	return !gpio_get_value_cansleep(host->gpio_cd);
  }
 
  #ifdef CONFIG_PM
@@ -497,55 +502,80 @@ static inline int omap_hsmmc_have_reg(void)
 
  #endif
 
-static int omap_hsmmc_gpio_init(struct omap_mmc_platform_data *pdata)
+static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host)
  {
-	int ret;
-
-	if (gpio_is_valid(pdata->slots[0].switch_pin)) {
-		if (pdata->slots[0].cover)
-			pdata->slots[0].get_cover_state =
+	struct omap_mmc_platform_data *pdata = host->pdata;
+	struct omap_mmc_slot_data *slot =&pdata->slots[0];
+	int gpio, ret;
+
+	gpio = slot->switch_pin;
+	if (slot->gpiochip_cd)
+		gpio = gpio_find_by_chip_name(slot->gpiochip_cd, gpio);
+	if (gpio_is_valid(gpio)) {
+		if (slot->cover)
+			slot->get_cover_state =
  					omap_hsmmc_get_cover_state;
  		else
-			pdata->slots[0].card_detect = omap_hsmmc_card_detect;
-		pdata->slots[0].card_detect_irq =
-				gpio_to_irq(pdata->slots[0].switch_pin);
-		ret = gpio_request(pdata->slots[0].switch_pin, "mmc_cd");
+			slot->card_detect = omap_hsmmc_card_detect;
+		slot->card_detect_irq =
+				gpio_to_irq(gpio);
+		ret = gpio_request(gpio, "mmc_cd");
  		if (ret)
  			return ret;
-		ret = gpio_direction_input(pdata->slots[0].switch_pin);
+		ret = gpio_direction_input(gpio);
  		if (ret)
  			goto err_free_sp;
-	} else
-		pdata->slots[0].switch_pin = -EINVAL;
+		host->gpio_cd = gpio;
+	} else {
+		if (slot->gpiochip_cd) {
+			pr_warning("MMC %s card detect GPIO chip %s unavailable\n",
+				slot->name, slot->gpiochip_cd);
+			ret = -ENODEV;
+			goto err_free_sp;
+		}
+		host->gpio_cd = -EINVAL;
+	}
 
-	if (gpio_is_valid(pdata->slots[0].gpio_wp)) {
-		pdata->slots[0].get_ro = omap_hsmmc_get_wp;
-		ret = gpio_request(pdata->slots[0].gpio_wp, "mmc_wp");
+	gpio = slot->gpio_wp;
+	if (slot->gpiochip_wp)
+		gpio = gpio_find_by_chip_name(slot->gpiochip_wp, gpio);
+	if (gpio_is_valid(gpio)) {
+		slot->get_ro = omap_hsmmc_get_wp;
+		ret = gpio_request(gpio, "mmc_wp");
  		if (ret)
  			goto err_free_cd;
-		ret = gpio_direction_input(pdata->slots[0].gpio_wp);
+		ret = gpio_direction_input(gpio);
  		if (ret)
  			goto err_free_wp;
-	} else
-		pdata->slots[0].gpio_wp = -EINVAL;
+		host->gpio_wp = gpio;
+	} else {
+		if (slot->gpiochip_wp) {
+			pr_warning("MMC %s write protect GPIO chip %s unavailable\n",
+				slot->name, slot->gpiochip_wp);
+			ret = -ENODEV;
+			goto err_free_wp;
+		}
+		host->gpio_wp = -EINVAL;
+	}
 
  	return 0;
 
  err_free_wp:
-	gpio_free(pdata->slots[0].gpio_wp);
+	if (gpio_is_valid(host->gpio_wp))
+		gpio_free(host->gpio_wp);
  err_free_cd:
-	if (gpio_is_valid(pdata->slots[0].switch_pin))
+	if (gpio_is_valid(host->gpio_cd))
  err_free_sp:
-		gpio_free(pdata->slots[0].switch_pin);
+		gpio_free(host->gpio_cd);
  	return ret;
  }
 
-static void omap_hsmmc_gpio_free(struct omap_mmc_platform_data *pdata)
+static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
  {
-	if (gpio_is_valid(pdata->slots[0].gpio_wp))
-		gpio_free(pdata->slots[0].gpio_wp);
-	if (gpio_is_valid(pdata->slots[0].switch_pin))
-		gpio_free(pdata->slots[0].switch_pin);
+	if (gpio_is_valid(host->gpio_wp))
+		gpio_free(host->gpio_wp);
+	if (gpio_is_valid(host->gpio_cd))
+		gpio_free(host->gpio_cd);
  }
 
  /*
@@ -1876,10 +1906,6 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
  	if (res == NULL)
  		return -EBUSY;
 
-	ret = omap_hsmmc_gpio_init(pdata);
-	if (ret)
-		goto err;
-
  	mmc = mmc_alloc_host(sizeof(struct omap_hsmmc_host),&pdev->dev);
  	if (!mmc) {
  		ret = -ENOMEM;
@@ -1903,6 +1929,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 
  	platform_set_drvdata(pdev, host);
 
+	ret = omap_hsmmc_gpio_init(host);
+	if (ret)
+		goto err1;
+
  	mmc->ops	=&omap_hsmmc_ops;
 
  	/*
@@ -2093,8 +2123,7 @@ err1:
  	platform_set_drvdata(pdev, NULL);
  	mmc_free_host(mmc);
  err_alloc:
-	omap_hsmmc_gpio_free(pdata);
-err:
+	omap_hsmmc_gpio_free(host);
  	release_mem_region(res->start, resource_size(res));
  	return ret;
  }
@@ -2125,7 +2154,7 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 
  		mmc_free_host(host->mmc);
  		iounmap(host->base);
-		omap_hsmmc_gpio_free(pdev->dev.platform_data);
+		omap_hsmmc_gpio_free(host);
  	}
 
  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
author maintainer
#11 lore raw

Re: [PATCH 2/4] mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

Quoted 10 lines · Rajendra Nayak
Quoted 8 lines · Tony Lindgren

Use gpio_find_by_chip_name() to find the GPIO pins as they can be dynamically allocated on various gpio_chips.

Note that we don't want to touch the platform data as it can now specify the GPIO offset on a named gpio_chip.

This removes the need to use callbacks to set the GPIO pins in platform data.

Quoted 7 lines

While one of the reasons for those callbacks was to set the GPIO pins in platform data, I guess the other and more important one was to make sure the init sequencing between twl4030-gpio and the mmc device depending on it was done rightly. Doesn't this patch now leave the sequencing to work by luck (in the absence of something like deferred probe being in place) like is the case of twl6030 and mmc init sequence on OMAP4 already?

Nope :) The difference is that we can exit and produce a sensible error to the user about the particular gpio_chip not being available.

Regards,

Tony

#12 lore raw

Re: [PATCH 2/4] mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

In reply to Tony Lindgren
Quoted 6 lines · Tony Lindgren

Use gpio_find_by_chip_name() to find the GPIO pins as they can be dynamically allocated on various gpio_chips.

Note that we don't want to touch the platform data as it can now specify the GPIO offset on a named gpio_chip.

Quoted 8 lines

This removes the need to use callbacks to set the GPIO pins in platform data.

Cc Chris Ball<[email protected]>
Cc Grant Likely<[email protected]>
Cc Rajendra Nayak<[email protected]>

---

some more comments based on my testing with twl4030-gpio built as a module..

Quoted 50 lines
  arch/arm/mach-omap2/hsmmc.c           |    3 +
  arch/arm/mach-omap2/hsmmc.h           |    5 ++
  arch/arm/plat-omap/include/plat/mmc.h |    3 +
  drivers/gpio/gpio-twl4030.c           |    2 +
  drivers/mmc/host/omap_hsmmc.c         |  109 +++++++++++++++++++++------------
  5 files changed, 82 insertions(+), 40 deletions(-)
arch/arm/mach-omap2/hsmmc.c +16 -11
index a97876d..dda88f7 100644
@@ -323,7 +323,10 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 
@@ -497,55 +502,80 @@ static inline int omap_hsmmc_have_reg(void)
 
  #endif
 
-static int omap_hsmmc_gpio_init(struct omap_mmc_platform_data *pdata)
+static int omap_hsmmc_gpio_init(struct omap_hsmmc_host *host)
  {
-	int ret;
-
-	if (gpio_is_valid(pdata->slots[0].switch_pin)) {
-		if (pdata->slots[0].cover)
-			pdata->slots[0].get_cover_state =
+	struct omap_mmc_platform_data *pdata = host->pdata;
+	struct omap_mmc_slot_data *slot =&pdata->slots[0];
+	int gpio, ret;
+
+	gpio = slot->switch_pin;
+	if (slot->gpiochip_cd)
+		gpio = gpio_find_by_chip_name(slot->gpiochip_cd, gpio);
+	if (gpio_is_valid(gpio)) {
+		if (slot->cover)
+			slot->get_cover_state =
  					omap_hsmmc_get_cover_state;
  		else
-			pdata->slots[0].card_detect = omap_hsmmc_card_detect;
-		pdata->slots[0].card_detect_irq =
-				gpio_to_irq(pdata->slots[0].switch_pin);
-		ret = gpio_request(pdata->slots[0].switch_pin, "mmc_cd");
+			slot->card_detect = omap_hsmmc_card_detect;
+		slot->card_detect_irq =
+				gpio_to_irq(gpio);
+		ret = gpio_request(gpio, "mmc_cd");
  		if (ret)
  			return ret;
-		ret = gpio_direction_input(pdata->slots[0].switch_pin);
+		ret = gpio_direction_input(gpio);
  		if (ret)

arch/arm/mach-omap2/hsmmc.c static inline int omap_hsmmc_have_reg(void) · line 534

Quoted 10 lines
arch/arm/mach-omap2/hsmmc.c +7 -2
  			goto err_free_sp;
-	} else
-		pdata->slots[0].switch_pin = -EINVAL;
+		host->gpio_cd = gpio;
+	} else {
+		if (slot->gpiochip_cd) {
+			pr_warning("MMC %s card detect GPIO chip %s unavailable\n",
+				slot->name, slot->gpiochip_cd);
+			ret = -ENODEV;
+			goto err_free_sp;

This should just return -ENODEV, nothing really to free here.

Quoted 84 lines
arch/arm/mach-omap2/hsmmc.c +33 -18
+		}
+		host->gpio_cd = -EINVAL;
+	}
 
-	if (gpio_is_valid(pdata->slots[0].gpio_wp)) {
-		pdata->slots[0].get_ro = omap_hsmmc_get_wp;
-		ret = gpio_request(pdata->slots[0].gpio_wp, "mmc_wp");
+	gpio = slot->gpio_wp;
+	if (slot->gpiochip_wp)
+		gpio = gpio_find_by_chip_name(slot->gpiochip_wp, gpio);
+	if (gpio_is_valid(gpio)) {
+		slot->get_ro = omap_hsmmc_get_wp;
+		ret = gpio_request(gpio, "mmc_wp");
  		if (ret)
  			goto err_free_cd;
-		ret = gpio_direction_input(pdata->slots[0].gpio_wp);
+		ret = gpio_direction_input(gpio);
  		if (ret)
  			goto err_free_wp;
-	} else
-		pdata->slots[0].gpio_wp = -EINVAL;
+		host->gpio_wp = gpio;
+	} else {
+		if (slot->gpiochip_wp) {
+			pr_warning("MMC %s write protect GPIO chip %s unavailable\n",
+				slot->name, slot->gpiochip_wp);
+			ret = -ENODEV;
+			goto err_free_wp;
+		}
+		host->gpio_wp = -EINVAL;
+	}
 
  	return 0;
 
  err_free_wp:
-	gpio_free(pdata->slots[0].gpio_wp);
+	if (gpio_is_valid(host->gpio_wp))
+		gpio_free(host->gpio_wp);
  err_free_cd:
-	if (gpio_is_valid(pdata->slots[0].switch_pin))
+	if (gpio_is_valid(host->gpio_cd))
  err_free_sp:
-		gpio_free(pdata->slots[0].switch_pin);
+		gpio_free(host->gpio_cd);
  	return ret;
  }
 
-static void omap_hsmmc_gpio_free(struct omap_mmc_platform_data *pdata)
+static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
  {
-	if (gpio_is_valid(pdata->slots[0].gpio_wp))
-		gpio_free(pdata->slots[0].gpio_wp);
-	if (gpio_is_valid(pdata->slots[0].switch_pin))
-		gpio_free(pdata->slots[0].switch_pin);
+	if (gpio_is_valid(host->gpio_wp))
+		gpio_free(host->gpio_wp);
+	if (gpio_is_valid(host->gpio_cd))
+		gpio_free(host->gpio_cd);
  }
 
  /*
@@ -1876,10 +1906,6 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
  	if (res == NULL)
  		return -EBUSY;
 
-	ret = omap_hsmmc_gpio_init(pdata);
-	if (ret)
-		goto err;
-
  	mmc = mmc_alloc_host(sizeof(struct omap_hsmmc_host),&pdev->dev);
  	if (!mmc) {
  		ret = -ENOMEM;
@@ -1903,6 +1929,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 
  	platform_set_drvdata(pdev, host);
 
+	ret = omap_hsmmc_gpio_init(host);
+	if (ret)
+		goto err1;
+
  	mmc->ops	=&omap_hsmmc_ops;
 
  	/*

arch/arm/mach-omap2/hsmmc.c err1: · line 2126

Quoted 6 lines
arch/arm/mach-omap2/hsmmc.c +1 -2
@@ -2093,8 +2123,7 @@ err1:
  	platform_set_drvdata(pdev, NULL);
  	mmc_free_host(mmc);
  err_alloc:
-	omap_hsmmc_gpio_free(pdata);
-err:
+	omap_hsmmc_gpio_free(host);

This error handling needs to be fixed up. In case omap_hsmmc_gpio_init() fails, which already frees up any requested gpios, omap_hsmmc_gpio_free() again tries freeing gpios.

regards,
Rajendra

Quoted 13 lines
arch/arm/mach-omap2/hsmmc.c +1 -1
  	release_mem_region(res->start, resource_size(res));
  	return ret;
  }
@@ -2125,7 +2154,7 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 
  		mmc_free_host(host->mmc);
  		iounmap(host->base);
-		omap_hsmmc_gpio_free(pdev->dev.platform_data);
+		omap_hsmmc_gpio_free(host);
  	}
 
  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
author maintainer
#13 lore raw

Re: [PATCH 2/4] mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

Quoted 8 lines · Rajendra Nayak

drivers/mmc/host/omap_hsmmc.c static inline int omap_hsmmc_have_reg(void) · line 534

Quoted 5 lines · Tony Lindgren
drivers/mmc/host/omap_hsmmc.c +5 -0
+		if (slot->gpiochip_cd) {
+			pr_warning("MMC %s card detect GPIO chip %s unavailable\n",
+				slot->name, slot->gpiochip_cd);
+			ret = -ENODEV;
+			goto err_free_sp;

This should just return -ENODEV, nothing really to free here.

Thanks, will correct.

Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/omap_hsmmc.c +0 -0
@@ -2093,8 +2123,7 @@ err1:
 	platform_set_drvdata(pdev, NULL);
 	mmc_free_host(mmc);
 err_alloc:
Quoted 8 lines

drivers/mmc/host/omap_hsmmc.c err1: · line 2126

Quoted 3 lines
drivers/mmc/host/omap_hsmmc.c +1 -2
-	omap_hsmmc_gpio_free(pdata);
-err:
+	omap_hsmmc_gpio_free(host);

This error handling needs to be fixed up. In case omap_hsmmc_gpio_init() fails, which already frees up any requested gpios, omap_hsmmc_gpio_free() again tries freeing gpios.

Hmm that sounds like a separate patch that should be a fixed before this series?

Regards,

Tony

author maintainer
#14 lore raw

Re: [PATCH 2/4] mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()

Quoted 15 lines · Tony Lindgren
Quoted 4 lines · Rajendra Nayak
Quoted 4 lines
drivers/mmc/host/omap_hsmmc.c +0 -0
@@ -2093,8 +2123,7 @@ err1:
 	platform_set_drvdata(pdev, NULL);
 	mmc_free_host(mmc);
 err_alloc:
Quoted 8 lines

drivers/mmc/host/omap_hsmmc.c err1: · line 2126

Quoted 3 lines
drivers/mmc/host/omap_hsmmc.c +1 -2
-	omap_hsmmc_gpio_free(pdata);
-err:
+	omap_hsmmc_gpio_free(host);

This error handling needs to be fixed up. In case omap_hsmmc_gpio_init() fails, which already frees up any requested gpios, omap_hsmmc_gpio_free() again tries freeing gpios.

Quoted 2 lines

Hmm that sounds like a separate patch that should be a fixed before this series?

Nope, I was confused. The error handling needs to change as the omap_hsmmc_gpio_init() got moved later so host is initialized. Will fix.

Regards,

Tony

author maintainer
#15 lore raw

PATCH 3/4 mmc: omap_hsmmc: Use GPIO offset for external GPIO chips

In reply to Tony Lindgren

We can now remove the setting of GPIO pins with callbacks as the drivers can access a GPIO offset on a named gpio_chip directly with gpio_find_by_chip_name().

---
 arch/arm/mach-omap2/board-3430sdp.c          |   13 ++++---------
 arch/arm/mach-omap2/board-cm-t35.c           |    8 ++------
 arch/arm/mach-omap2/board-devkit8000.c       |    7 ++-----
 arch/arm/mach-omap2/board-igep0020.c         |    8 ++------
 arch/arm/mach-omap2/board-omap3beagle.c      |    9 +++------
 arch/arm/mach-omap2/board-omap3evm.c         |    8 ++------
 arch/arm/mach-omap2/board-omap3pandora.c     |   13 ++++---------
 arch/arm/mach-omap2/board-omap3stalker.c     |    8 ++------
 arch/arm/mach-omap2/board-omap3touchbook.c   |    7 ++-----
 arch/arm/mach-omap2/board-zoom-peripherals.c |    7 ++-----
 10 files changed, 25 insertions(+), 63 deletions(-)
arch/arm/mach-omap2/board-3430sdp.c +4 -9
index da75f23..238b95a 100644
@@ -231,14 +231,16 @@ static struct omap2_hsmmc_info mmc[] = {
 		 * so the SIM card isn't used; else 4 bits.
 		 */
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 4,
-		.deferred	= true,
 	},
 	{
 		.mmc		= 2,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 1,	/* mmc1_cd offset in twl4030 */
 		.gpio_wp	= 7,
-		.deferred	= true,
 	},
 	{}	/* Terminator */
 };
@@ -246,13 +248,6 @@ static struct omap2_hsmmc_info mmc[] = {
 static int sdp3430_twl_gpio_setup(struct device *dev,
 		unsigned gpio, unsigned ngpio)
 {
-	/* gpio + 0 is "mmc0_cd" (input/IRQ),
-	 * gpio + 1 is "mmc1_cd" (input/IRQ)
-	 */
-	mmc[0].gpio_cd = gpio + 0;
-	mmc[1].gpio_cd = gpio + 1;
-	omap_hsmmc_late_init(mmc);
-
 	/* gpio + 7 is "sub_lcd_en_bkl" (output/PWM1) */
 	gpio_request_one(gpio + 7, GPIOF_OUT_INIT_LOW, "sub_lcd_en_bkl");
 
arch/arm/mach-omap2/board-cm-t35.c +2 -6
index 49e6405..26466f3 100644
@@ -411,9 +411,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= -EINVAL,
-		.deferred	= true,
 	},
 	{
 		.mmc		= 2,
@@ -469,10 +469,6 @@ static int cm_t35_twl_gpio_setup(struct device *dev, unsigned gpio,
 		pr_err("CM-T35: could not obtain gpio for WiFi reset\n");
 	}
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	return 0;
 }
 
arch/arm/mach-omap2/board-devkit8000.c +2 -5
index 11cd2a8..b250999 100644
@@ -99,8 +99,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 29,
-		.deferred	= true,
 	},
 	{}	/* Terminator */
 };
@@ -227,10 +228,6 @@ static int devkit8000_twl_gpio_setup(struct device *dev,
 {
 	int ret;
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/* TWL4030_GPIO_MAX + 1 == ledB, PMU_STAT (out, active low LED) */
 	gpio_leds[2].gpio = gpio + TWL4030_GPIO_MAX + 1;
 
arch/arm/mach-omap2/board-igep0020.c +2 -6
index e558800..d39f016 100644
@@ -293,9 +293,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= -EINVAL,
-		.deferred	= true,
 	},
 #if defined(CONFIG_LIBERTAS_SDIO) || defined(CONFIG_LIBERTAS_SDIO_MODULE)
 	{
@@ -401,10 +401,6 @@ static int igep_twl_gpio_setup(struct device *dev,
 {
 	int ret;
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/* TWL4030_GPIO_MAX + 1 == ledB (out, active low LED) */
 #if !defined(CONFIG_LEDS_GPIO) && !defined(CONFIG_LEDS_GPIO_MODULE)
 	ret = gpio_request_one(gpio + TWL4030_GPIO_MAX + 1, GPIOF_OUT_INIT_HIGH,
arch/arm/mach-omap2/board-omap3beagle.c +3 -6
index 7be8d65..bbb3234 100644
@@ -252,8 +252,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= -EINVAL,
-		.deferred	= true,
 	},
 	{}	/* Terminator */
 };
@@ -273,11 +274,6 @@ static int beagle_twl_gpio_setup(struct device *dev,
 {
 	int r;
 
-	mmc[0].gpio_wp = beagle_config.mmc1_gpio_wp;
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/*
 	 * TWL4030_GPIO_MAX + 0 == ledA, EHCI nEN_USB_PWR (out, XM active
 	 * high / others active low)
@@ -523,6 +519,7 @@ static void __init omap3_beagle_init(void)
 
 	if (beagle_config.mmc1_gpio_wp != -EINVAL)
 		omap_mux_init_gpio(beagle_config.mmc1_gpio_wp, OMAP_PIN_INPUT);
+	mmc[0].gpio_wp = beagle_config.mmc1_gpio_wp;
 	omap_hsmmc_init(mmc);
 
 	omap3_beagle_i2c_init();
arch/arm/mach-omap2/board-omap3evm.c +2 -6
index 6b77ad9..4bc86d9 100644
@@ -315,9 +315,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 63,
-		.deferred	= true,
 	},
 #ifdef CONFIG_WL12XX_PLATFORM_DATA
 	{
@@ -361,10 +361,6 @@ static int omap3evm_twl_gpio_setup(struct device *dev,
 {
 	int r, lcd_bl_en;
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/*
 	 * Most GPIOs are for USB OTG.  Some are mostly sent to
 	 * the P2 connector; notably LEDA for the LCD backlight.
arch/arm/mach-omap2/board-omap3pandora.c +4 -9
index ace466b..b387264 100644
@@ -270,19 +270,19 @@ static struct omap2_hsmmc_info omap3pandora_mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 126,
 		.ext_clock	= 0,
-		.deferred	= true,
 	},
 	{
 		.mmc		= 2,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 127,
 		.ext_clock	= 1,
 		.transceiver	= true,
-		.deferred	= true,
 	},
 	{
 		.mmc		= 3,
@@ -299,11 +299,6 @@ static int omap3pandora_twl_gpio_setup(struct device *dev,
 {
 	int ret, gpio_32khz;
 
-	/* gpio + {0,1} is "mmc{0,1}_cd" (input/IRQ) */
-	omap3pandora_mmc[0].gpio_cd = gpio + 0;
-	omap3pandora_mmc[1].gpio_cd = gpio + 1;
-	omap_hsmmc_late_init(omap3pandora_mmc);
-
 	/* gpio + 13 drives 32kHz buffer for wifi module */
 	gpio_32khz = gpio + 13;
 	ret = gpio_request_one(gpio_32khz, GPIOF_OUT_INIT_HIGH, "wifi 32kHz");
arch/arm/mach-omap2/board-omap3stalker.c +2 -6
index 6410043..6d0deb9 100644
@@ -211,9 +211,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 23,
-		.deferred	= true,
 	 },
 	{}			/* Terminator */
 };
@@ -282,10 +282,6 @@ static int
 omap3stalker_twl_gpio_setup(struct device *dev,
 			    unsigned gpio, unsigned ngpio)
 {
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/*
 	 * Most GPIOs are for USB OTG.  Some are mostly sent to
 	 * the P2 connector; notably LEDA for the LCD backlight.
arch/arm/mach-omap2/board-omap3touchbook.c +2 -5
index 8842e04..cf270b8 100644
@@ -99,8 +99,9 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 29,
-		.deferred	= true,
 	},
 	{}	/* Terminator */
 };
@@ -118,10 +119,6 @@ static struct gpio_led gpio_leds[];
 static int touchbook_twl_gpio_setup(struct device *dev,
 		unsigned gpio, unsigned ngpio)
 {
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	/* REVISIT: need ehci-omap hooks for external VBUS
 	 * power switch and overcurrent detect
 	 */
arch/arm/mach-omap2/board-zoom-peripherals.c +2 -5
index 3d39cdb..604df86 100644
@@ -203,9 +203,10 @@ static struct omap2_hsmmc_info mmc[] = {
 		.name		= "external",
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= -EINVAL,
 		.power_saving	= true,
-		.deferred	= true,
 	},
 	{
 		.name		= "internal",
@@ -232,10 +233,6 @@ static int zoom_twl_gpio_setup(struct device *dev,
 {
 	int ret;
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
 	ret = gpio_request_one(LCD_PANEL_ENABLE_GPIO, GPIOF_OUT_INIT_LOW,
 			       "lcd enable");
 	if (ret)
#16 lore raw

PATCH 3/4 mmc: omap_hsmmc: Use GPIO offset for external GPIO chips

Quoted 24 lines · Tony Lindgren

We can now remove the setting of GPIO pins with callbacks as the drivers can access a GPIO offset on a named gpio_chip directly with gpio_find_by_chip_name().

---
  arch/arm/mach-omap2/board-3430sdp.c          |   13 ++++---------
  arch/arm/mach-omap2/board-cm-t35.c           |    8 ++------
  arch/arm/mach-omap2/board-devkit8000.c       |    7 ++-----
  arch/arm/mach-omap2/board-igep0020.c         |    8 ++------
  arch/arm/mach-omap2/board-omap3beagle.c      |    9 +++------
  arch/arm/mach-omap2/board-omap3evm.c         |    8 ++------
  arch/arm/mach-omap2/board-omap3pandora.c     |   13 ++++---------
  arch/arm/mach-omap2/board-omap3stalker.c     |    8 ++------
  arch/arm/mach-omap2/board-omap3touchbook.c   |    7 ++-----
  arch/arm/mach-omap2/board-zoom-peripherals.c |    7 ++-----
  10 files changed, 25 insertions(+), 63 deletions(-)
arch/arm/mach-omap2/board-3430sdp.c +0 -0
index da75f23..238b95a 100644

arch/arm/mach-omap2/board-3430sdp.c static struct omap2_hsmmc_info mmc[] = { · line 235

Quoted 7 lines
arch/arm/mach-omap2/board-3430sdp.c +2 -1
@@ -231,14 +231,16 @@ static struct omap2_hsmmc_info mmc[] = {
  		 * so the SIM card isn't used; else 4 bits.
  		 */
  		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
  		.gpio_wp	= 4,
-		.deferred	= true,

Shouldn't this patch completely get rid of all the 'deferred' infrastructure that was put in place, including the omap_hsmmc_late_init() function, since there is no need for it anymore?

Quoted 1 line
arch/arm/mach-omap2/board-3430sdp.c +0 -0
  	},

[..]

Quoted 14 lines

/*
* Most GPIOs are for USB OTG. Some are mostly sent to
* the P2 connector; notably LEDA for the LCD backlight.

arch/arm/mach-omap2/board-omap3pandora.c +2 -1
index ace466b..b387264 100644
@@ -270,19 +270,19 @@ static struct omap2_hsmmc_info omap3pandora_mmc[] = {
  	{
  		.mmc		= 1,
  		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */

arch/arm/mach-omap2/board-omap3pandora.c static struct omap2_hsmmc_info omap3pandora_mmc[] = { · line 282

Quoted 10 lines
arch/arm/mach-omap2/board-omap3pandora.c +2 -2
  		.gpio_wp	= 126,
  		.ext_clock	= 0,
-		.deferred	= true,
  	},
  	{
  		.mmc		= 2,
  		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */

This one should be gpio_cd = 1,

regards,
Rajendra

Quoted 104 lines
arch/arm/mach-omap2/board-omap3pandora.c +0 -6
  		.gpio_wp	= 127,
  		.ext_clock	= 1,
  		.transceiver	= true,
-		.deferred	= true,
  	},
  	{
  		.mmc		= 3,
@@ -299,11 +299,6 @@ static int omap3pandora_twl_gpio_setup(struct device *dev,
  {
  	int ret, gpio_32khz;
 
-	/* gpio + {0,1} is "mmc{0,1}_cd" (input/IRQ) */
-	omap3pandora_mmc[0].gpio_cd = gpio + 0;
-	omap3pandora_mmc[1].gpio_cd = gpio + 1;
-	omap_hsmmc_late_init(omap3pandora_mmc);
-
  	/* gpio + 13 drives 32kHz buffer for wifi module */
  	gpio_32khz = gpio + 13;
  	ret = gpio_request_one(gpio_32khz, GPIOF_OUT_INIT_HIGH, "wifi 32kHz");
arch/arm/mach-omap2/board-omap3stalker.c +2 -6
index 6410043..6d0deb9 100644
@@ -211,9 +211,9 @@ static struct omap2_hsmmc_info mmc[] = {
  	{
  		.mmc		= 1,
  		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
  		.gpio_wp	= 23,
-		.deferred	= true,
  	 },
  	{}			/* Terminator */
  };
@@ -282,10 +282,6 @@ static int
  omap3stalker_twl_gpio_setup(struct device *dev,
  			    unsigned gpio, unsigned ngpio)
  {
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
  	/*
  	 * Most GPIOs are for USB OTG.  Some are mostly sent to
  	 * the P2 connector; notably LEDA for the LCD backlight.
arch/arm/mach-omap2/board-omap3touchbook.c +2 -5
index 8842e04..cf270b8 100644
@@ -99,8 +99,9 @@ static struct omap2_hsmmc_info mmc[] = {
  	{
  		.mmc		= 1,
  		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
  		.gpio_wp	= 29,
-		.deferred	= true,
  	},
  	{}	/* Terminator */
  };
@@ -118,10 +119,6 @@ static struct gpio_led gpio_leds[];
  static int touchbook_twl_gpio_setup(struct device *dev,
  		unsigned gpio, unsigned ngpio)
  {
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
  	/* REVISIT: need ehci-omap hooks for external VBUS
  	 * power switch and overcurrent detect
  	 */
arch/arm/mach-omap2/board-zoom-peripherals.c +2 -6
index 3d39cdb..604df86 100644
@@ -203,9 +203,10 @@ static struct omap2_hsmmc_info mmc[] = {
  		.name		= "external",
  		.mmc		= 1,
  		.caps		= MMC_CAP_4_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
  		.gpio_wp	= -EINVAL,
  		.power_saving	= true,
-		.deferred	= true,
  	},
  	{
  		.name		= "internal",
@@ -232,10 +233,6 @@ static int zoom_twl_gpio_setup(struct device *dev,
  {
  	int ret;
 
-	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
-	mmc[0].gpio_cd = gpio + 0;
-	omap_hsmmc_late_init(mmc);
-
  	ret = gpio_request_one(LCD_PANEL_ENABLE_GPIO, GPIOF_OUT_INIT_LOW,
  			       "lcd enable");
  	if (ret)
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

author maintainer
#17 lore raw

Re: [PATCH 3/4] mmc: omap_hsmmc: Use GPIO offset for external GPIO chips

Quoted 16 lines · Rajendra Nayak

arch/arm/mach-omap2/board-3430sdp.c static struct omap2_hsmmc_info mmc[] = { · line 235

Quoted 10 lines · Tony Lindgren
arch/arm/mach-omap2/board-3430sdp.c +2 -1
@@ -231,14 +231,16 @@ static struct omap2_hsmmc_info mmc[] = {
 		 * so the SIM card isn't used; else 4 bits.
 		 */
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 4,
-		.deferred	= true,

Shouldn't this patch completely get rid of all the 'deferred' infrastructure that was put in place, including the omap_hsmmc_late_init() function, since there is no need for it anymore?

Yes, that was needed as a fix so unfortunately there's a little bit going back and forth. But now we can get rid of other stuff too in addition to deferred omap_hsmmc_late_init(), we can remove init and cleanup callbacks for hsmmc. I'll do another patch for that.

Quoted 13 lines
Quoted 13 lines
arch/arm/mach-omap2/board-omap3pandora.c +2 -2
@@ -270,19 +270,19 @@ static struct omap2_hsmmc_info omap3pandora_mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */
 		.gpio_wp	= 126,
 		.ext_clock	= 0,
-		.deferred	= true,
 	},
Quoted 8 lines

arch/arm/mach-omap2/board-omap3pandora.c static struct omap2_hsmmc_info omap3pandora_mmc[] = { · line 282

Quoted 6 lines
arch/arm/mach-omap2/board-omap3pandora.c +2 -1
 	{
 		.mmc		= 2,
 		.caps		= MMC_CAP_4_BIT_DATA,
-		.gpio_cd	= -EINVAL,
+		.gpiochip_cd	= "twl4030_gpio",
+		.gpio_cd	= 0,	/* mmc0_cd offset in twl4030 */

This one should be gpio_cd = 1,

Thanks, will correct.

Regards,

Tony

author maintainer
#18 lore raw

PATCH 4/4 mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

In reply to Tony Lindgren

There's no need to use callbacks for this, we can
do it directly between MMC driver and twl6030.

Cc Samuel Ortiz <[email protected]>
Cc Chris Ball <[email protected]>
Cc Rajendra Nayak <[email protected]>
---
 arch/arm/mach-omap2/board-4430sdp.c    |   45 +---------------------------
 arch/arm/mach-omap2/board-omap4panda.c |   52 +-------------------------------
 drivers/mfd/twl6030-irq.c              |   33 +++++++++-----------
 drivers/mmc/host/omap_hsmmc.c          |   31 +++++++++++++++++++
 4 files changed, 48 insertions(+), 113 deletions(-)
arch/arm/mach-omap2/board-4430sdp.c +1 -44
index 09ae257..c31efa4 100644
@@ -455,49 +455,6 @@ static struct platform_device omap_vwlan_device = {
 	},
 };
 
-static int omap4_twl6030_hsmmc_late_init(struct device *dev)
-{
-	int ret = 0;
-	struct platform_device *pdev = container_of(dev,
-				struct platform_device, dev);
-	struct omap_mmc_platform_data *pdata = dev->platform_data;
-
-	/* Setting MMC1 Card detect Irq */
-	if (pdev->id == 0) {
-		ret = twl6030_mmc_card_detect_config();
-		if (ret)
-			pr_err("Failed configuring MMC1 card detect\n");
-		pdata->slots[0].card_detect_irq = TWL6030_IRQ_BASE +
-						MMCDETECT_INTR_OFFSET;
-		pdata->slots[0].card_detect = twl6030_mmc_card_detect;
-	}
-	return ret;
-}
-
-static __init void omap4_twl6030_hsmmc_set_late_init(struct device *dev)
-{
-	struct omap_mmc_platform_data *pdata;
-
-	/* dev can be null if CONFIG_MMC_OMAP_HS is not set */
-	if (!dev) {
-		pr_err("Failed %s\n", __func__);
-		return;
-	}
-	pdata = dev->platform_data;
-	pdata->init =	omap4_twl6030_hsmmc_late_init;
-}
-
-static int __init omap4_twl6030_hsmmc_init(struct omap2_hsmmc_info *controllers)
-{
-	struct omap2_hsmmc_info *c;
-
-	omap_hsmmc_init(controllers);
-	for (c = controllers; c->mmc; c++)
-		omap4_twl6030_hsmmc_set_late_init(&c->pdev->dev);
-
-	return 0;
-}
-
 static struct regulator_init_data sdp4430_vaux1 = {
 	.constraints = {
 		.min_uV			= 1000000,
@@ -906,7 +863,7 @@ static void __init omap_4430sdp_init(void)
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 	omap4_sdp4430_wifi_init();
-	omap4_twl6030_hsmmc_init(mmc);
+	omap_hsmmc_init(mmc);
 
 	usb_musb_init(&musb_board_data);
 
arch/arm/mach-omap2/board-omap4panda.c +2 -50

71 lines not on this page · raw message

drivers/mfd/twl6030-irq.c +14 -19
index c6b456a..ce0002b 100644
@@ -283,35 +283,30 @@ int twl6030_mmc_card_detect_config(void)
 	 * Card status on TWL6030 for MMC1
 	 */
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, &reg_val, TWL6030_MMCCTRL);
-	if (ret < 0) {
-		pr_err("twl6030: Failed to read MMCCTRL, error %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		goto err;
 	reg_val &= ~VMMC_AUTO_OFF;
 	reg_val |= SW_FC;
 	ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
-	if (ret < 0) {
-		pr_err("twl6030: Failed to write MMCCTRL, error %d\n", ret);
-		return ret;
-	}
+	if (ret < 0)
+		goto err;
 
 	/* Configuring PullUp-PullDown register */
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID0, &reg_val,
 						TWL6030_CFG_INPUT_PUPD3);
-	if (ret < 0) {
-		pr_err("twl6030: Failed to read CFG_INPUT_PUPD3, error %d\n",
-									ret);
-		return ret;
-	}
+	if (ret < 0)
+		goto err;
 	reg_val &= ~(MMC_PU | MMC_PD);
 	ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val,
 						TWL6030_CFG_INPUT_PUPD3);
-	if (ret < 0) {
-		pr_err("twl6030: Failed to write CFG_INPUT_PUPD3, error %d\n",
-									ret);
-		return ret;
-	}
-	return 0;
+	if (ret < 0)
+		goto err;
+
+	return twl6030_irq_base + MMCDETECT_INTR_OFFSET;
+
+err:
+	pr_err("twl6030: Failed to initialize MMC card detect: %d\n", ret);
+	return -ENODEV;
 }
 EXPORT_SYMBOL(twl6030_mmc_card_detect_config);
 
drivers/mmc/host/omap_hsmmc.c +31 -0
index 1aa2420..7f483b7 100644
@@ -34,6 +34,7 @@
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/i2c/twl.h>
 #include <plat/dma.h>
 #include <mach/hardware.h>
 #include <plat/board.h>
@@ -578,6 +579,32 @@ static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
 		gpio_free(host->gpio_cd);
 }
 
+#ifdef CONFIG_TWL4030_CORE
+static int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
+{
+	struct omap_mmc_platform_data *pdata = host->pdata;
+	struct omap_mmc_slot_data *slot = &pdata->slots[0];
+	int irq;
+
+	if (gpio_is_valid(host->gpio_cd) || host->id)
+		return 0;
+
+	irq = twl6030_mmc_card_detect_config();
+	if (irq <= 0)
+		return irq;
+
+	slot->card_detect_irq = irq;
+	slot->card_detect = twl6030_mmc_card_detect;
+
+	return 0;
+}
+#else
+static inline int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
+{
+	return -ENODEV;
+}
+#endif
+
 /*
  * Start clock to the card
  */
@@ -1933,6 +1960,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err1;
 
+	ret = omap_hsmmc_init_twl6030(host);
+	if (ret)
+		goto err1;
+
 	mmc->ops	= &omap_hsmmc_ops;
 
 	/*
#19 lore raw

PATCH 4/4 mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 8 lines · Tony Lindgren

There's no need to use callbacks for this, we can
do it directly between MMC driver and twl6030.

Cc Samuel Ortiz<[email protected]>
Cc Chris Ball<[email protected]>
Cc Rajendra Nayak<[email protected]>

---

[..]

arch/arm/mach-omap2/board-omap4panda.c static struct omap2_hsmmc_info mmc[] = { · line 157

Quoted 11 lines
arch/arm/mach-omap2/board-omap4panda.c +1 -1
index 7ca7a5c..8cf4e54 100644
@@ -153,8 +153,8 @@ static struct omap2_hsmmc_info mmc[] = {
  	{
  		.mmc		= 1,
  		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
-		.gpio_wp	= -EINVAL,
  		.gpio_cd	= -EINVAL,
+		.gpio_wp	= -EINVAL,

stray change.

Quoted 1 line
arch/arm/mach-omap2/board-omap4panda.c +0 -0
  	},

[..]

Quoted 68 lines
drivers/mfd/twl6030-irq.c +14 -19
index c6b456a..ce0002b 100644
@@ -283,35 +283,30 @@ int twl6030_mmc_card_detect_config(void)
  	 * Card status on TWL6030 for MMC1
  	 */
  	ret = twl_i2c_read_u8(TWL6030_MODULE_ID0,&reg_val, TWL6030_MMCCTRL);
-	if (ret<  0) {
-		pr_err("twl6030: Failed to read MMCCTRL, error %d\n", ret);
-		return ret;
-	}
+	if (ret<  0)
+		goto err;
  	reg_val&= ~VMMC_AUTO_OFF;
  	reg_val |= SW_FC;
  	ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
-	if (ret<  0) {
-		pr_err("twl6030: Failed to write MMCCTRL, error %d\n", ret);
-		return ret;
-	}
+	if (ret<  0)
+		goto err;
 
  	/* Configuring PullUp-PullDown register */
  	ret = twl_i2c_read_u8(TWL6030_MODULE_ID0,&reg_val,
  						TWL6030_CFG_INPUT_PUPD3);
-	if (ret<  0) {
-		pr_err("twl6030: Failed to read CFG_INPUT_PUPD3, error %d\n",
-									ret);
-		return ret;
-	}
+	if (ret<  0)
+		goto err;
  	reg_val&= ~(MMC_PU | MMC_PD);
  	ret = twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val,
  						TWL6030_CFG_INPUT_PUPD3);
-	if (ret<  0) {
-		pr_err("twl6030: Failed to write CFG_INPUT_PUPD3, error %d\n",
-									ret);
-		return ret;
-	}
-	return 0;
+	if (ret<  0)
+		goto err;
+
+	return twl6030_irq_base + MMCDETECT_INTR_OFFSET;
+
+err:
+	pr_err("twl6030: Failed to initialize MMC card detect: %d\n", ret);
+	return -ENODEV;
  }
  EXPORT_SYMBOL(twl6030_mmc_card_detect_config);
 
drivers/mmc/host/omap_hsmmc.c +1 -0
index 1aa2420..7f483b7 100644
@@ -34,6 +34,7 @@
  #include<linux/gpio.h>
  #include<linux/regulator/consumer.h>
  #include<linux/pm_runtime.h>
+#include<linux/i2c/twl.h>
  #include<plat/dma.h>
  #include<mach/hardware.h>
  #include<plat/board.h>
@@ -578,6 +579,32 @@ static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
  		gpio_free(host->gpio_cd);

drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host) · line 589

Quoted 10 lines
drivers/mmc/host/omap_hsmmc.c +8 -0
  }
 
+#ifdef CONFIG_TWL4030_CORE
+static int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
+{
+	struct omap_mmc_platform_data *pdata = host->pdata;
+	struct omap_mmc_slot_data *slot =&pdata->slots[0];
+	int irq;
+
+	if (gpio_is_valid(host->gpio_cd) || host->id)

I have a series, which I am asking Chris to pull, which completely gets rid of all host->id based hard-codings' in the driver. Isn't there a better way to do this than rely on the device instance?

regards,
Rajendra

Quoted 33 lines
drivers/mmc/host/omap_hsmmc.c +22 -0
+		return 0;
+
+	irq = twl6030_mmc_card_detect_config();
+	if (irq<= 0)
+		return irq;
+
+	slot->card_detect_irq = irq;
+	slot->card_detect = twl6030_mmc_card_detect;
+
+	return 0;
+}
+#else
+static inline int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
+{
+	return -ENODEV;
+}
+#endif
+
  /*
   * Start clock to the card
   */
@@ -1933,6 +1960,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
  	if (ret)
  		goto err1;
 
+	ret = omap_hsmmc_init_twl6030(host);
+	if (ret)
+		goto err1;
+
  	mmc->ops	=&omap_hsmmc_ops;
 
  	/*
author maintainer
#20 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 12 lines · Rajendra Nayak

arch/arm/mach-omap2/board-omap4panda.c static struct omap2_hsmmc_info mmc[] = { · line 157

Quoted 9 lines · Tony Lindgren
arch/arm/mach-omap2/board-omap4panda.c +1 -1
@@ -153,8 +153,8 @@ static struct omap2_hsmmc_info mmc[] = {
 	{
 		.mmc		= 1,
 		.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA,
-		.gpio_wp	= -EINVAL,
 		.gpio_cd	= -EINVAL,
+		.gpio_wp	= -EINVAL,

stray change.

Hmm I'd like to keep this change before this ordering gets copied to ten other board files that we may have in testing-board for a while.. I'll add a comment to the patch description about that.

Quoted 14 lines
Quoted 14 lines
drivers/mmc/host/omap_hsmmc.c +1 -0
@@ -34,6 +34,7 @@
 #include<linux/gpio.h>
 #include<linux/regulator/consumer.h>
 #include<linux/pm_runtime.h>
+#include<linux/i2c/twl.h>
 #include<plat/dma.h>
 #include<mach/hardware.h>
 #include<plat/board.h>
@@ -578,6 +579,32 @@ static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host)
 		gpio_free(host->gpio_cd);
 }
 
Quoted 12 lines

drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_gpio_free(struct omap_hsmmc_host *host) · line 589

Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c +8 -0
+#ifdef CONFIG_TWL4030_CORE
+static int omap_hsmmc_init_twl6030(struct omap_hsmmc_host *host)
+{
+	struct omap_mmc_platform_data *pdata = host->pdata;
+	struct omap_mmc_slot_data *slot =&pdata->slots[0];
+	int irq;
+
+	if (gpio_is_valid(host->gpio_cd) || host->id)

I have a series, which I am asking Chris to pull, which completely gets rid of all host->id based hard-codings' in the driver. Isn't there a better way to do this than rely on the device instance?

Yes I was thinking about that too. I guess we need to pass some flag in pdata for twl6030 card detect. I was thinking about using the gpiochip_cd, but twl6030 is not gpio based card detect, so a separate flag is better.

BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails to detect any cards, and then fails to unload. This works on omap3 just fine. Any ideas why that would be?

Regards,

Tony

#21 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 3 lines · Tony Lindgren

BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails to detect any cards, and then fails to unload. This works on omap3 just fine. Any ideas why that would be?

Yeah, looks like thats broken. I am not sure whats going wrong though. I just enabled CONFIG_MMC_DEBUG and saw these logs below.

Venkat/Balaji, care to look at this one?

# insmod omap_hsmmc.ko
[   43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
[   43.364105] omap_hsmmc omap_hsmmc.0: enabled
[   44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0 
timing 0
[   44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[   44.474365] omap_hsmmc omap_hsmmc.0: disabled
[   44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
[   44.482208] omap_hsmmc omap_hsmmc.4: enabled
[   44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
[   44.552276] omap_hsmmc omap_hsmmc.0: enabled
[   44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[   44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0 
timing 0
[   44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[   44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 
width 0 timing 0
[   44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
[   44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
[   44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
[   45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0 
timing 0
[   45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
[   45.591491] omap_hsmmc omap_hsmmc.4: disabled
#
#
# rmmod omap_hsmmc
[  607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
[  607.308044] omap_hsmmc omap_hsmmc.4: enabled
[  607.312591] omap_hsmmc omap_hsmmc.4: disabled
[  607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
[  607.322875] omap_hsmmc omap_hsmmc.4: enabled
#22 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 7 lines · Rajendra Nayak
Quoted 4 lines · Tony Lindgren

BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails to detect any cards, and then fails to unload. This works on omap3 just fine. Any ideas why that would be?

Quoted 5 lines

Yeah, looks like thats broken. I am not sure whats going wrong though. I just enabled CONFIG_MMC_DEBUG and saw these logs below.

Venkat/Balaji, care to look at this one?

Let me check this

Quoted 33 lines
# insmod omap_hsmmc.ko
[ ? 43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
[ ? 43.364105] omap_hsmmc omap_hsmmc.0: enabled
[ ? 44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
timing 0
[ ? 44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[ ? 44.474365] omap_hsmmc omap_hsmmc.0: disabled
[ ? 44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
[ ? 44.482208] omap_hsmmc omap_hsmmc.4: enabled
[ ? 44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
[ ? 44.552276] omap_hsmmc omap_hsmmc.0: enabled
[ ? 44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[ ? 44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0
timing 0
[ ? 44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[ ? 44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 width
0 timing 0
[ ? 44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
[ ? 44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
[ ? 44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
[ ? 45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
timing 0
[ ? 45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
[ ? 45.591491] omap_hsmmc omap_hsmmc.4: disabled
#
#
# rmmod omap_hsmmc
[ ?607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
[ ?607.308044] omap_hsmmc omap_hsmmc.4: enabled
[ ?607.312591] omap_hsmmc omap_hsmmc.4: disabled
[ ?607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
[ ?607.322875] omap_hsmmc omap_hsmmc.4: enabled
#23 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 8 lines · Balaji
Quoted 7 lines · Rajendra Nayak
Quoted 4 lines · Tony Lindgren

BTW, with -rc5, looks like re-inserting omap_hsmmc on omap4 fails to detect any cards, and then fails to unload. This works on omap3 just fine. Any ideas why that would be?

Quoted 7 lines
Quoted 5 lines

Yeah, looks like thats broken. I am not sure whats going wrong though. I just enabled CONFIG_MMC_DEBUG and saw these logs below.

Venkat/Balaji, care to look at this one?

Let me check this

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset. Addng the offset to platform_device resource structure increments the start address for every insmod operation.
MMC command fails on re-insertion as module due incorrect register base. Fix this by updating the ioremap base address only.

---
Note: eMMC detection is still failing on resertion due to card vcc power off on rmmod

 drivers/mmc/host/omap_hsmmc.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/mmc/host/omap_hsmmc.c +0 -0
index fd0c661..4e1f8f6 100644
@@ -1870,8 +1870,6 @@ static int __init omap_hsmmc_probe(struct

platform_device *pdev)
if (res == NULL || irq < 0)
return -ENXIO;

- res->start += pdata->reg_offset;
- res->end += pdata->reg_offset;
res = request_mem_region(res->start, resource_size(res), pdev->name); if (res == NULL)
return -EBUSY;
@@ -1896,7 +1894,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
host->irq = irq;
host->id = pdev->id;
host->slot_id = 0;
- host->mapbase = res->start;
+ host->mapbase = res->start + pdata->reg_offset;
host->base = ioremap(host->mapbase, SZ_4K);
host->power_mode = MMC_POWER_OFF;
host->next_data.cookie = 1;

Signature · 1 line
1.7.0.4
Quoted 34 lines
Quoted 34 lines
# insmod omap_hsmmc.ko
[ ? 43.358398] omap_hsmmc omap_hsmmc.0: context was not lost
[ ? 43.364105] omap_hsmmc omap_hsmmc.0: enabled
[ ? 44.434661] mmc0: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
timing 0
[ ? 44.442352] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[ ? 44.474365] omap_hsmmc omap_hsmmc.0: disabled
[ ? 44.482208] omap_hsmmc omap_hsmmc.4: context was not lost
[ ? 44.482208] omap_hsmmc omap_hsmmc.4: enabled
[ ? 44.546600] omap_hsmmc omap_hsmmc.0: context was not lost
[ ? 44.552276] omap_hsmmc omap_hsmmc.0: enabled
[ ? 44.552276] mmc0: mmc_rescan_try_freq: trying to init card at 400000 Hz
[ ? 44.563720] mmc0: clock 0Hz busmode 2 powermode 1 cs 0 Vdd 18 width 0
timing 0
[ ? 44.572174] omap_hsmmc omap_hsmmc.0: Set clock to 0Hz
[ ? 44.613800] mmc0: clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 18 width
0 timing 0
[ ? 44.621887] omap_hsmmc omap_hsmmc.0: Set clock to 400000Hz
[ ? 44.735290] mmc0: starting CMD52 arg 00000c00 flags 00000195
[ ? 44.741271] omap_hsmmc omap_hsmmc.0: mmc0: CMD52, argument 0x00000c00
[ ? 45.560241] mmc1: clock 0Hz busmode 1 powermode 0 cs 0 Vdd 0 width 0
timing 0
[ ? 45.567871] omap_hsmmc omap_hsmmc.4: Set clock to 0Hz
[ ? 45.591491] omap_hsmmc omap_hsmmc.4: disabled
#
#
# rmmod omap_hsmmc
[ ?607.302307] omap_hsmmc omap_hsmmc.4: context was not lost
[ ?607.308044] omap_hsmmc omap_hsmmc.4: enabled
[ ?607.312591] omap_hsmmc omap_hsmmc.4: disabled
[ ?607.317199] omap_hsmmc omap_hsmmc.4: context was not lost
[ ?607.322875] omap_hsmmc omap_hsmmc.4: enabled
Signature · 2 lines
Thanks and Regards,
Balaji T K
#24 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Hi Balaji,

Quoted 7 lines · Balaji

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset. Addng the offset to platform_device resource structure increments the start address for every insmod operation.
MMC command fails on re-insertion as module due incorrect register base. Fix this by updating the ioremap base address only.

Is this a regression, or has it never worked in mainline?

Quoted 2 lines

Note: eMMC detection is still failing on resertion due to card vcc power off on rmmod

And this?

Thanks,

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
author maintainer
#25 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

Quoted 9 lines · Chris Ball

Hi Balaji,

Quoted 6 lines · Balaji

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset. Addng the offset to platform_device resource structure increments the start address for every insmod operation.
MMC command fails on re-insertion as module due incorrect register base. Fix this by updating the ioremap base address only.

Quoted 3 lines
Quoted 1 line

Is this a regression, or has it never worked in mainline?

"Features that never worked originally" so this can wait for the merge window. It happens on re-inserting of the card.

Quoted 4 lines
Quoted 2 lines

Note: eMMC detection is still failing on resertion due to card vcc power off on rmmod

And this?

That sounds like a separate issue that needs to be fixed.

Regards,

Tony

#26 lore raw

Re: [PATCH 4/4] mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

In reply to Chris Ball
Quoted 9 lines · Chris Ball

Hi Balaji,

Quoted 6 lines · Balaji

OMAP4 and OMAP3 HSMMC IP registers differ by 0x100 offset. Addng the offset to platform_device resource structure increments the start address for every insmod operation.
MMC command fails on re-insertion as module due incorrect register base. Fix this by updating the ioremap base address only.

Quoted 3 lines
Quoted 1 line

Is this a regression, or has it never worked in mainline?

Not a regression introduced in current merge window. It happens on re-insertion of module.
will post a patch with $SUBJECT

Quoted 5 lines
Quoted 3 lines

Note: ?eMMC detection is still failing on resertion due to card vcc power off on rmmod

And this?

This issue was hidden and uncovered after this fix. This problem is Vcc being powered off without sleep command.

Quoted 7 lines

Thanks,

- Chris.
--
Chris Ball ? <[email protected]> ? <http://printf.net/>
One Laptop Per Child

#28 lore raw
In reply to Tony Lindgren

Hi Tony,

Quoted 7 lines · Tony Lindgren

Hi all,

This series adds gpio_find_by_name() that allows finding GPIOs on specific gpio_chips. As the GPIO numbers can be dynamic, it's hard to find the GPIO numbers from drivers using them directly.

Quoted 7 lines

So far we've dealt with this using platform specific callbacks, but that is messy. This series removes the needs for these callbacks for omap hsmmc driver. Further callbacks can be removed people are OK with adding gpio_find_by_name().

This series is based on the omap fixes-non-critical that's needed for the arch/arm/mach-omap2 parts of this series.

I tested these on my beagle/panda/omap4sdp and they seem to work fine, also fixing the broken panda card detect (due to missing card_detect_irq in the board file). There are still issues however when I build twl4030-gpio as a module, which I already commented on, and the fact that the init sequence now works by luck :)
The other issue also is that the multiple insmod/rmmod test suggested by Russell still fails, since the second time around the gpio_requests in the board callback fail because they are not freed when you do a module unload/unbind.
That would need this patch from me to add the .teardown hooks
http://marc.info/?l=linux-omap&m=133007767831297&w=2

regards,
Rajendra

Quoted 36 lines

Regards,

Tony

---

Tony Lindgren (4):
gpiolib: Add gpiochip_find_by_name() and gpio_find_by_chip_name()
mmc: omap_hsmmc: Use gpio_find_by_chip_name() for omap_hsmmc_gpio_init()
mmc: omap_hsmmc: Use GPIO offset for external GPIO chips
mmc: omap_hsmmc: Simplify init for twl6030 MMC card detect

  arch/arm/mach-omap2/board-3430sdp.c          |   13 +-
  arch/arm/mach-omap2/board-4430sdp.c          |   45 --------
  arch/arm/mach-omap2/board-cm-t35.c           |    8 -
  arch/arm/mach-omap2/board-devkit8000.c       |    7 -
  arch/arm/mach-omap2/board-igep0020.c         |    8 -
  arch/arm/mach-omap2/board-omap3beagle.c      |    9 +-
  arch/arm/mach-omap2/board-omap3evm.c         |    8 -
  arch/arm/mach-omap2/board-omap3pandora.c     |   13 +-
  arch/arm/mach-omap2/board-omap3stalker.c     |    8 -
  arch/arm/mach-omap2/board-omap3touchbook.c   |    7 -
  arch/arm/mach-omap2/board-omap4panda.c       |   52 ----------
  arch/arm/mach-omap2/board-zoom-peripherals.c |    7 -
  arch/arm/mach-omap2/hsmmc.c                  |    3 +
  arch/arm/mach-omap2/hsmmc.h                  |    5 +
  arch/arm/plat-omap/include/plat/mmc.h        |    3 +
  drivers/gpio/gpio-twl4030.c                  |    2
  drivers/gpio/gpiolib.c                       |   47 +++++++++
  drivers/mfd/twl6030-irq.c                    |   33 +++---
  drivers/mmc/host/omap_hsmmc.c                |  140 +++++++++++++++++++-------
  include/asm-generic/gpio.h                   |    3 -
  20 files changed, 204 insertions(+), 217 deletions(-)
author maintainer
#29 lore raw
Quoted 18 lines · Rajendra Nayak

Hi Tony,

Quoted 7 lines · Tony Lindgren

Hi all,

This series adds gpio_find_by_name() that allows finding GPIOs on specific gpio_chips. As the GPIO numbers can be dynamic, it's hard to find the GPIO numbers from drivers using them directly.

Quoted 7 lines

So far we've dealt with this using platform specific callbacks, but that is messy. This series removes the needs for these callbacks for omap hsmmc driver. Further callbacks can be removed people are OK with adding gpio_find_by_name().

This series is based on the omap fixes-non-critical that's needed for the arch/arm/mach-omap2 parts of this series.

Quoted 6 lines

I tested these on my beagle/panda/omap4sdp and they seem to work fine, also fixing the broken panda card detect (due to missing card_detect_irq in the board file). There are still issues however when I build twl4030-gpio as a module, which I already commented on, and the fact that the init sequence now works by luck :)

Hmm it should not be luck based, loading omap_hsmmc module should fail with a sensible error if the configured card detect or write protect is not available.

I guess this is with twl6030 non-gpio based card detect?

If so, I'll add something to pass the twl6030 card detect from pdata so we can fail with a sensible error in that case too.

Also, sounds like twl as module and mmc built in case won't work without deferred probe. But at least there is a sensible error for that. And maybe we can prevent that in Kconfig.

Quoted 7 lines

The other issue also is that the multiple insmod/rmmod test suggested by Russell still fails, since the second time around the gpio_requests in the board callback fail because they are not freed when you do a module unload/unbind.
That would need this patch from me to add the .teardown hooks
http://marc.info/?l=linux-omap&m=133007767831297&w=2

Yes let's add the teardown patch as a fix for now, but let's plan on getting rid of the twl_setup callback function completely.

With these patches LCD and WLAN too can request the twl gpios directly from the driver based on gpio_chip + gpio offset.

Regards,

Tony