arm: davinci: DA850: wl12xx expansion card

Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip conversion") introduced a bug, causing low level interrupt handlers to get a bogus irq number as an argument. The gpio irq handler falsely assumes that the handler data is the irq base number and that is no longer true.

Fix this by converting gpio_irq_handler's bank_irq argument to the corresponding irq base number.

Signed-off-by Ido Yariv <[email protected]>

CC Thomas Gleixner <[email protected]>

---
 arch/arm/mach-davinci/gpio.c |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

arch/arm/mach-davinci/gpio.c +26 -2

index e722139..ff43e2a 100644
@@ -249,16 +249,40 @@ static struct irq_chip gpio_irqchip = {
 	.flags		= IRQCHIP_SET_TYPE_MASKED,
 };
 
+static inline int bankirq_to_irqbase(unsigned int bank_irq)
+{
+	int gpio;
+	int index;
+
+	/* Each irq bank consists of up to 16 irqs */
+	gpio = 16 * (bank_irq - davinci_soc_info.gpio_irq);
+
+	/* Each controller controls 32 GPIOs */
+	index = gpio / 32;
+
+	if (unlikely(!davinci_soc_info.gpio_ctlrs))
+		return -EINVAL;
+
+	if (unlikely(index >= davinci_soc_info.gpio_ctlrs_num))
+		return -EINVAL;
+
+	return davinci_soc_info.gpio_ctlrs[index].irq_base;
+}
+
 static void
-gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+gpio_irq_handler(unsigned bank_irq, struct irq_desc *desc)
 {
 	struct davinci_gpio_regs __iomem *g;
 	u32 mask = 0xffff;
+	int irqbase = bankirq_to_irqbase(bank_irq);
+
+	if (unlikely(irqbase < 0))
+		return;
 
 	g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc);
 
 	/* we only care about one bank */
-	if (irq & 1)
+	if (bank_irq & 1)
 		mask <<= 16;
 
 	/* temporarily mask (level sensitive) parent IRQ */

@@ -274,11 +298,11 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 		if (!status)
 			break;
 		__raw_writel(status, &g->intstat);

Thanks for the bug fix.

How about setting the handler data for bank IRQ in davinci_gpio_irq_setup() to &chips[bank]?

chips[bank].regs should give you the register base address
(g) and chips[bank].irq_base should give you 'n'.

Also please drop the rename of irq to bank_irq as it is not central to the bug fix.

If you spin in soon enough, we may make it to v3.0

Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip conversion") introduced a bug, causing low level interrupt handlers to get a bogus irq number as an argument. The gpio irq handler falsely assumes that the handler data is the irq base number and that is no longer true.

Set the irq handler data to be a pointer to the corresponding gpio controller. The chained irq handler can then use it to extract both the irq base number and the gpio registers structure.

Signed-off-by Ido Yariv <[email protected]>

CC Thomas Gleixner <[email protected]>

---
 arch/arm/mach-davinci/gpio.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

arch/arm/mach-davinci/gpio.c +0 -0

index e722139..7d64a07 100644

arch/arm/mach-davinci/gpio.c +0 -0

 		while (status) {
 			res = ffs(status);
 			n += res;

@@ -424,7 +426,13 @@ static int __init davinci_gpio_irq_setup(void)
 

Quoted 9 lines

arch/arm/mach-davinci/gpio.c +2 -1

-	g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc);
+	ctl = (struct davinci_gpio_controller *)irq_desc_get_handler_data(desc);
+	g = (struct davinci_gpio_regs __iomem *)ctl->regs;
 
 	/* we only care about one bank */
 	if (irq & 1)

@@ -278,7 +280,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 			status >>= 16;
 

arch/arm/mach-davinci/gpio.c gpio_irq_handler(unsigned irq, struct irq_desc *desc)

Quoted 3 lines

arch/arm/mach-davinci/gpio.c +1 -1

 		/* now demux them to the right lowlevel handler */
-		n = (int)irq_get_handler_data(irq);
+		n = ctl->irq_base;

I realized that this breaks for odd banks as the status is right shifted by 16. The GPIO you are using must have been in even bank?

Quoted 5 lines

arch/arm/mach-davinci/gpio.c +0 -0

 		while (status) {
 			res = ffs(status);
 			n += res;

@@ -424,7 +426,13 @@ static int __init davinci_gpio_irq_setup(void)
 

arch/arm/mach-davinci/gpio.c static int __init davinci_gpio_irq_setup(void) · line 435

Quoted 10 lines

arch/arm/mach-davinci/gpio.c +7 -1

 		/* set up all irqs in this bank */
 		irq_set_chained_handler(bank_irq, gpio_irq_handler);
-		irq_set_handler_data(bank_irq, (__force void *)g);
+
+		/*
+		 * Each chip handles 32 gpios, and each irq bank consists of 16
+		 * gpio irqs. Pass the irq bank's corresponding controller to
+		 * the chained irq handler.
+		 */
+		irq_set_handler_data(bank_irq, &chips[bank * 16 / 32]);

This can simply be:

		irq_set_handler_data(bank_irq, &chips[gpio / 32]);

In the interest of time, I did these fixes and pushed the
patch to "fixes" branch of git://gitorious.org/linux-davinci/linux-davinci.git

Can you please test it out and let me know if it works.

Davinci platforms may define a default queue for each channel controller. If one is not defined, the default queue is set to EVENTQ_1. However, there's no way to distinguish between an unset default queue to one that is set to EVENTQ_0, as EVENTQ_0 = 0.

Explicitly specify the default queue for all channel controllers on all Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe function.

One exception is the DA850 board, for which EVENTQ_1 is not a valid option for its second channel controller. Use EVENTQ_0 instead for that channel controller.

Signed-off-by Ido Yariv <[email protected]>

Quoted 8 lines

Davinci platforms may define a default queue for each channel controller. If one is not defined, the default queue is set to EVENTQ_1. However, there's no way to distinguish between an unset default queue to one that is set to EVENTQ_0, as EVENTQ_0 = 0.

Explicitly specify the default queue for all channel controllers on all Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe function.

Quoted 5 lines

One exception is the DA850 board, for which EVENTQ_1 is not a valid option for its second channel controller. Use EVENTQ_0 instead for that channel controller.

Signed-off-by Ido Yariv <[email protected]>

Looks good to me. Will queue for v3.2/fixes

Hello.

Quoted 12 lines · Sekhar

Quoted 12 lines

Davinci platforms may define a default queue for each channel controller. If one is not defined, the default queue is set to EVENTQ_1. However, there's no way to distinguish between an unset default queue to one that is set to EVENTQ_0, as EVENTQ_0 = 0.

Explicitly specify the default queue for all channel controllers on all Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe function.

One exception is the DA850 board, for which EVENTQ_1 is not a valid option for its second channel controller. Use EVENTQ_0 instead for that channel controller.

Quoted 3 lines

Quoted 1 line

Signed-off-by Ido Yariv <[email protected]>

Looks good to me. Will queue for v3.2/fixes

Why wait for 3.2? If this is considered a fix, it should be applied to 3.1, no?

Hi Sergei,

Quoted 17 lines · Sergei Shtylyov

Hello.

Quoted 12 lines · Sekhar

Quoted 12 lines

Davinci platforms may define a default queue for each channel controller. If one is not defined, the default queue is set to EVENTQ_1. However, there's no way to distinguish between an unset default queue to one that is set to EVENTQ_0, as EVENTQ_0 = 0.

Explicitly specify the default queue for all channel controllers on all Davinci platforms to EVENTQ_1, and don't overwrite it in the EDMA probe function.

One exception is the DA850 board, for which EVENTQ_1 is not a valid option for its second channel controller. Use EVENTQ_0 instead for that channel controller.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Ido Yariv <[email protected]>

Looks good to me. Will queue for v3.2/fixes

? ? Why wait for 3.2? If this is considered a fix, it should be applied to 3.1, no?

3.2/fixes just indicates it will be queued as a fix/cleanup for 3.2 so it will have higher priority for merge when compared to a new feature.

This patch doesn't really fix any existing broken functionality. It corrects event queue configuration for EDMA CC1 on DA850 for which there are no current users in mainline.

So, not sending for v3.1.

Some devices connected to the MMC bus are power controlled by external means. For instance, an SDIO device may be powered down/up by an external gpio line.

In order to avoid toggling power from within the MMC host driver, add a set_power callback function, which will be called by set_ios upon powering down/up.

Signed-off-by Ido Yariv <[email protected]>

---
 arch/arm/mach-davinci/include/mach/mmc.h |    3 +++
 drivers/mmc/host/davinci_mmc.c           |   13 +++++++++++++

 2 files changed, 16 insertions(+), 0 deletions(-)

Some devices connected to the MMC bus are power controlled by external means. For instance, an SDIO device may be powered down/up by an external gpio line.

In order to avoid toggling power from within the MMC host driver, add a set_power callback function, which will be called by set_ios upon powering down/up.

Signed-off-by Ido Yariv <[email protected]>

CC Chris Ball <[email protected]>

---
 arch/arm/mach-davinci/include/mach/mmc.h |    3 +++
 drivers/mmc/host/davinci_mmc.c           |   13 +++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

arch/arm/mach-davinci/include/mach/mmc.h +3 -0

index d4f1e96..5ba6b22 100644
@@ -12,6 +12,9 @@ struct davinci_mmc_config {
 	/* get_cd()/get_wp() may sleep */
 	int	(*get_cd)(int module);
 	int	(*get_ro)(int module);
+
+	void	(*set_power)(int module, bool on);
+
 	/* wires == 0 is equivalent to wires == 4 (4-bit parallel) */
 	u8	wires;
 

drivers/mmc/host/davinci_mmc.c +13 -0

index 0076c74..64a8325 100644
@@ -807,12 +807,25 @@ static void calculate_clk_divider(struct mmc_host *mmc, struct mmc_ios *ios)
 static void mmc_davinci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct mmc_davinci_host *host = mmc_priv(mmc);
+	struct platform_device *pdev = to_platform_device(mmc->parent);
+	struct davinci_mmc_config *config = pdev->dev.platform_data;
 
 	dev_dbg(mmc_dev(host->mmc),
 		"clock %dHz busmode %d powermode %d Vdd %04x\n",
 		ios->clock, ios->bus_mode, ios->power_mode,
 		ios->vdd);
 
+	switch (ios->power_mode) {
+	case MMC_POWER_OFF:
+		if (config && config->set_power)
+			config->set_power(pdev->id, false);
+		break;
+	case MMC_POWER_UP:
+		if (config && config->set_power)
+			config->set_power(pdev->id, true);
+		break;
+	}
+
 	switch (ios->bus_width) {
 	case MMC_BUS_WIDTH_8:
 		dev_dbg(mmc_dev(host->mmc), "Enabling 8 bit mode\n");

The wl1271 daughter board makes use of a few GPIOs:
GPIO6_9 is used for powering down/up the WLAN functionality. GPIO6_10 is used as an input IRQ line from the WLAN chip.

Add the required pinmux configuration for these GPIOs.

Signed-off-by Ido Yariv <[email protected]>

The wl1271 daughter board makes use of a few GPIOs:
GPIO6_9 is used for powering down/up the WLAN functionality. GPIO6_10 is used as an input IRQ line from the WLAN chip.

Add the required pinmux configuration for these GPIOs.

Signed-off-by Ido Yariv <[email protected]>

The wl1271 daughter card for AM18x EVMs is a combo wireless connectivity add-on card, based on the LS Research TiWi module with Texas Instruments' wl1271 solution.
It is a 4-wire, 1.8V, embedded SDIO WLAN device with an external IRQ line and is power-controlled by a GPIO-based fixed regulator.

This patch adds support for the WLAN capabilities of this expansion board.

Signed-off-by Ido Yariv <[email protected]>

---

arch/arm/mach-davinci/board-da850-evm.c +14 -0

index a7b41bf..2dae1a1 100644
@@ -31,6 +31,8 @@
 
+#ifdef CONFIG_DA850_WL12XX
+
+static int da850_wl12xx_fref = WL12XX_REFCLOCK_38;
+
+static int __init setup_da850_wl12xx_fref(char *fref)
+{
+	if (!strcmp(fref, "19.2"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_19;
+	else if (!strcmp(fref, "26"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_26;
+	else if (!strcmp(fref, "38.4"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_38;
+	else if (!strcmp(fref, "52"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_52;

arch/arm/mach-davinci/board-da850-evm.c +36 -0

+	.set_power	= wl12xx_set_power,
+	.wires		= 4,
+	.max_freq	= 25000000,
+	.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_NONREMOVABLE |
+			  MMC_CAP_POWER_OFF_CARD,
+	.version	= MMC_CTLR_VERSION_2,
+};
 
+static void da850_wl12xx_init(void)
+{
+	int ret;
+
+	ret = davinci_cfg_reg_list(da850_evm_mmc_wl12xx_pins);
+	if (ret) {
+		pr_warning("da850_evm_init: wl12xx/mmc mux setup failed:"
+			   " %d\n", ret);
+		return;
+	}
+
+	ret = da850_register_mmcsd1(&da850_mmc_wl12xx_config);
+	if (ret) {
+		pr_warning("da850_evm_init: wl12xx/mmc registration failed:"
+			   " %d\n", ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_WLAN_EN, GPIOF_OUT_INIT_LOW, "wl12xx_en");
+	if (ret) {
+		pr_err("Error initializing the wl12xx enable gpio: %d\n", ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_WLAN_IRQ, GPIOF_IN, "wl12xx_irq");
+	if (ret) {
+		pr_err("Error initializing the wl12xx irq gpio: %d\n", ret);
+		gpio_free(DA850_WLAN_EN);
+		return;

Adding a new kernel parameter requires update to Documentation/kernel-parameters.txt as well.

I am Ccing a couple of folks in case they have ideas on whether there is a better way to pass this information to the kernel. I assume there is no way to detect this from hardware.

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1175

Quoted 3 lines

arch/arm/mach-davinci/board-da850-evm.c +3 -0

+static struct davinci_mmc_config da850_mmc_wl12xx_config = {
+	.get_ro		= NULL,
+	.get_cd		= NULL,

You can get rid of these NULL initializers.

Quoted 7 lines

arch/arm/mach-davinci/board-da850-evm.c +7 -0

+	.set_power	= wl12xx_set_power,
+	.wires		= 4,
+	.max_freq	= 25000000,
+	.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_NONREMOVABLE |
+			  MMC_CAP_POWER_OFF_CARD,
+	.version	= MMC_CTLR_VERSION_2,
+};

Quoted 16 lines

arch/arm/mach-davinci/board-da850-evm.c +16 -0

+	ret = gpio_request_one(DA850_WLAN_EN, GPIOF_OUT_INIT_LOW, "wl12xx_en");
+	if (ret) {
+		pr_err("Error initializing the wl12xx enable gpio: %d\n", ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_WLAN_IRQ, GPIOF_IN, "wl12xx_irq");
+	if (ret) {
+		pr_err("Error initializing the wl12xx irq gpio: %d\n", ret);
+		gpio_free(DA850_WLAN_EN);
+		return;
+	}
+
+	da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
+	da850_wl12xx_wlan_data.board_ref_clock = da850_wl12xx_fref;
+

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1235

Quoted 5 lines

arch/arm/mach-davinci/board-da850-evm.c +5 -0

+	ret = wl12xx_set_platform_data(&da850_wl12xx_wlan_data);
+	if (ret) {
+		pr_err("Error setting wl12xx data: %d\n", ret);
+		gpio_free(DA850_WLAN_IRQ);
+		gpio_free(DA850_WLAN_EN);

Why not just use the traditional goto based bail out mechanism? You will avoid the multiple gpio_free() calls.

From where to pull this repository.

Hi Sekhar,

Quoted 7 lines · Sekhar

Adding a new kernel parameter requires update to Documentation/kernel-parameters.txt as well.

I am Ccing a couple of folks in case they have ideas on whether there is a better way to pass this information to the kernel. I assume there is no way to detect this from hardware.

Unfortunately, auto-detection of the reference clock is not currently possible. However, it might be a better idea to have the ability to override this value with a wl12xx module parameter instead of a kernel parameter. I'll drop this kernel parameter.

Quoted 5 lines

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1175

Quoted 3 lines

arch/arm/mach-davinci/board-da850-evm.c +3 -0

+static struct davinci_mmc_config da850_mmc_wl12xx_config = {
+   .get_ro         = NULL,
+   .get_cd         = NULL,

You can get rid of these NULL initializers.

Sure.

Quoted 7 lines

Quoted 7 lines

arch/arm/mach-davinci/board-da850-evm.c +7 -0

+   .set_power      = wl12xx_set_power,
+   .wires          = 4,
+   .max_freq       = 25000000,
+   .caps           = MMC_CAP_4_BIT_DATA | MMC_CAP_NONREMOVABLE |
+                     MMC_CAP_POWER_OFF_CARD,
+   .version        = MMC_CTLR_VERSION_2,
+};

[...]

Quoted 1 line

Quoted 1 line

+ ret = gpio_request_one(DA850_WLAN_EN, GPIOF_OUT_INIT_LOW,

"wl12xx_en");

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+   if (ret) {
+           pr_err("Error initializing the wl12xx enable gpio: %d\n",

ret);

Quoted 6 lines

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void)

Quoted 6 lines

arch/arm/mach-davinci/board-da850-evm.c +6 -0

+           return;
+   }
+
+   ret = gpio_request_one(DA850_WLAN_IRQ, GPIOF_IN, "wl12xx_irq");
+   if (ret) {
+           pr_err("Error initializing the wl12xx irq gpio: %d\n",

ret);

Quoted 7 lines

Quoted 7 lines

arch/arm/mach-davinci/board-da850-evm.c +7 -0

+           gpio_free(DA850_WLAN_EN);
+           return;
+   }
+
+   da850_wl12xx_wlan_data.irq = gpio_to_irq(DA850_WLAN_IRQ);
+   da850_wl12xx_wlan_data.board_ref_clock = da850_wl12xx_fref;
+

Quoted 8 lines

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1235

Quoted 5 lines

arch/arm/mach-davinci/board-da850-evm.c +5 -0

+   ret = wl12xx_set_platform_data(&da850_wl12xx_wlan_data);
+   if (ret) {
+           pr_err("Error setting wl12xx data: %d\n", ret);
+           gpio_free(DA850_WLAN_IRQ);
+           gpio_free(DA850_WLAN_EN);

Why not just use the traditional goto based bail out mechanism? You will avoid the multiple gpio_free() calls.

Sure.

Thanks for your review,
Ido.

_______________________________________________
linux-arm-kernel mailing list linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

The wl1271 daughter card for AM18x EVMs is a combo wireless connectivity add-on card, based on the LS Research TiWi module with Texas Instruments' wl1271 solution.
It is a 4-wire, 1.8V, embedded SDIO WLAN device with an external IRQ line and is power-controlled by a GPIO-based fixed regulator.

Add support for the WLAN capabilities of this expansion board.

Signed-off-by Ido Yariv <[email protected]>

---
 arch/arm/mach-davinci/Kconfig           |   10 +++
 arch/arm/mach-davinci/board-da850-evm.c |  114 +++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 0 deletions(-)

arch/arm/mach-davinci/Kconfig +10 -0

index c0deaca..32d837d 100644
@@ -192,6 +192,16 @@ config DA850_UI_RMII
 
 endchoice
 
+config DA850_WL12XX
+	bool "AM18x wl1271 daughter board"
+	depends on MACH_DAVINCI_DA850_EVM
+	help
+	  The wl1271 daughter card for AM18x EVMs is a combo wireless
+	  connectivity add-on card, based on the LS Research TiWi module with
+	  Texas Instruments' wl1271 solution.
+	  Say Y if you want to use a wl1271 expansion card connected to the
+	  AM18x EVM.
+
 config GPIO_PCA953X
 	default MACH_DAVINCI_DA850_EVM
 

arch/arm/mach-davinci/board-da850-evm.c +15 -0

index bd53945..d817626 100644
@@ -31,6 +31,8 @@
 #include <linux/input/tps6507x-ts.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
+#include <linux/delay.h>
+#include <linux/wl12xx.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>

@@ -49,6 +51,9 @@
 #define DA850_MMCSD_CD_PIN		GPIO_TO_PIN(4, 0)
 #define DA850_MMCSD_WP_PIN		GPIO_TO_PIN(4, 1)
 
+#define DA850_WLAN_EN			GPIO_TO_PIN(6, 9)
+#define DA850_WLAN_IRQ			GPIO_TO_PIN(6, 10)
+
 #define DA850_MII_MDIO_CLKEN_PIN	GPIO_TO_PIN(2, 6)
 
 static struct mtd_partition da850evm_spiflash_part[] = {

@@ -1117,6 +1122,110 @@ static __init int da850_evm_init_cpufreq(void)
 static __init int da850_evm_init_cpufreq(void) { return 0; }
 #endif
 
+#ifdef CONFIG_DA850_WL12XX
+
+static void wl12xx_set_power(int index, bool power_on)
+{
+	static bool power_state;
+
+	pr_debug("Powering %s wl12xx", power_on ? "on" : "off");
+
+	if (power_on == power_state)
+		return;

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1144

Quoted 8 lines

arch/arm/mach-davinci/board-da850-evm.c +8 -0

+	if (power_on) {
+		/* Power up sequence required for wl127x devices */
+		gpio_set_value(DA850_WLAN_EN, 1);
+		usleep_range(15000, 15000);
+		gpio_set_value(DA850_WLAN_EN, 0);
+		usleep_range(1000, 1000);
+		gpio_set_value(DA850_WLAN_EN, 1);
+		msleep(70);

Why turn on, then off, and then back on?
Isn't off, then back on sufficient?

Also, why not use regulator API like panda board does?

Hi Troy,

Quoted 11 lines · Troy Kisky

arch/arm/mach-davinci/board-da850-evm.c static __init int da850_evm_init_cpufreq(void) · line 1144

Quoted 8 lines

arch/arm/mach-davinci/board-da850-evm.c +8 -0

+	if (power_on) {
+		/* Power up sequence required for wl127x devices */
+		gpio_set_value(DA850_WLAN_EN, 1);
+		usleep_range(15000, 15000);
+		gpio_set_value(DA850_WLAN_EN, 0);
+		usleep_range(1000, 1000);
+		gpio_set_value(DA850_WLAN_EN, 1);
+		msleep(70);

Why turn on, then off, and then back on?
Isn't off, then back on sufficient?

Unfortunately, no. This is a required power up sequence for some hardware revisions of the 1271 chip.

Quoted 2 lines

Also, why not use regulator API like panda board does?

Unlike omap_hsmmc, davinci's MMC host driver does not toggle any regulators. To keep things simple, a set_power callback function was added to the mmc host as part of this patch series.
I've considered adding regulator support instead, but found it to be a bit over-complicated for this task. In addition, it would require either modifying the fixed regulator or adding a new one in order to support the above power sequence.

Thanks for your review,
Ido.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

The wl1271 daughter card for AM18x EVMs is a combo wireless connectivity add-on card, based on the LS Research TiWi module with Texas Instruments' wl1271 solution.
It is a 4-wire, 1.8V, embedded SDIO WLAN device with an external IRQ line and is power-controlled by a GPIO-based fixed regulator.

Add support for the WLAN capabilities of this expansion board.

Signed-off-by Ido Yariv <[email protected]>

(quoted diff) +27 -0

+static __init int da850_wl12xx_init(void)
+{
+ ? ? ? int ret;
+
+ ? ? ? ret = davinci_cfg_reg_list(da850_wl12xx_pins);
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_err("wl12xx/mmc mux setup failed: %d\n", ret);
+ ? ? ? ? ? ? ? goto exit;
+ ? ? ? }
+
+ ? ? ? ret = da850_register_mmcsd1(&da850_wl12xx_mmc_config);
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_err("wl12xx/mmc registration failed: %d\n", ret);
+ ? ? ? ? ? ? ? goto exit;
+ ? ? ? }
+
+ ? ? ? ret = gpio_request_one(DA850_WLAN_EN, GPIOF_OUT_INIT_LOW, "wl12xx_en");
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_err("Could not request wl12xx enable gpio: %d\n", ret);
+ ? ? ? ? ? ? ? goto exit;
+ ? ? ? }
+
+ ? ? ? ret = gpio_request_one(DA850_WLAN_IRQ, GPIOF_IN, "wl12xx_irq");
+ ? ? ? if (ret) {
+ ? ? ? ? ? ? ? pr_err("Could not request wl12xx irq gpio: %d\n", ret);
+ ? ? ? ? ? ? ? goto free_wlan_en;
+ ? ? ? }

Hi Ido,

A quick question for my own understanding. What's the mechanism by which wl12xx is bound with above. I see above wl12xx_set_platform_data but I didn't see something like .name = "wl1271" or dev_name that comes in through platform_device_register.

If I look at other board files, I see:

321 #ifdef CONFIG_WL12XX_PLATFORM_DATA
322 {

323                 .name           = "wl1271",
324                 .mmc            = 2,
325                 .caps           = MMC_CAP_4_BIT_DATA |
MMC_CAP_POWER_OFF_CARD,
326                 .gpio_wp        = -EINVAL,
327                 .gpio_cd        = -EINVAL,
328                 .nonremovable   = true,

329 },
330 #endif

729 #ifdef CONFIG_WL12XX_PLATFORM_DATA
730 /* WL12xx WLAN Init */
731 if (wl12xx_set_platform_data(&omap3evm_wlan_data))
732 pr_err("error setting wl12xx data\n");
733 platform_device_register(&omap3evm_wlan_regulator);
734 #endif

The wl1271 daughter card for AM18x EVMs is a combo wireless connectivity add-on card, based on the LS Research TiWi module with Texas Instruments' wl1271 solution.
It is a 4-wire, 1.8V, embedded SDIO WLAN device with an external IRQ line and is power-controlled by a GPIO-based fixed regulator.

Add support for the WLAN capabilities of this expansion board.

Signed-off-by Ido Yariv <[email protected]>

The wl1271 daughter card for AM18x EVMs is a combo wireless connectivity add-on card, based on the LS Research TiWi module with Texas Instruments' wl1271 solution.
It is a 4-wire, 1.8V, embedded SDIO WLAN device with an external IRQ line and is power-controlled by a GPIO-based fixed regulator.

This patch adds support for the WLAN capabilities of this expansion board.

Signed-off-by Ido Yariv <[email protected]>

---

arch/arm/mach-davinci/board-da850-evm.c +14 -0

index a7b41bf..2dae1a1 100644
@@ -31,6 +31,8 @@
 
+#ifdef CONFIG_DA850_WL12XX
+
+static int da850_wl12xx_fref = WL12XX_REFCLOCK_38;
+
+static int __init setup_da850_wl12xx_fref(char *fref)
+{
+	if (!strcmp(fref, "19.2"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_19;
+	else if (!strcmp(fref, "26"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_26;
+	else if (!strcmp(fref, "38.4"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_38;
+	else if (!strcmp(fref, "52"))
+		da850_wl12xx_fref = WL12XX_REFCLOCK_52;

arch/arm/mach-davinci/board-da850-evm.c +36 -0

+	.set_power	= wl12xx_set_power,
+	.wires		= 4,
+	.max_freq	= 25000000,
+	.caps		= MMC_CAP_4_BIT_DATA | MMC_CAP_NONREMOVABLE |
+			  MMC_CAP_POWER_OFF_CARD,
+	.version	= MMC_CTLR_VERSION_2,
+};
 
+static void da850_wl12xx_init(void)
+{
+	int ret;
+
+	ret = davinci_cfg_reg_list(da850_evm_mmc_wl12xx_pins);
+	if (ret) {
+		pr_warning("da850_evm_init: wl12xx/mmc mux setup failed:"
+			   " %d\n", ret);
+		return;
+	}
+
+	ret = da850_register_mmcsd1(&da850_mmc_wl12xx_config);
+	if (ret) {
+		pr_warning("da850_evm_init: wl12xx/mmc registration failed:"
+			   " %d\n", ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_WLAN_EN, GPIOF_OUT_INIT_LOW, "wl12xx_en");
+	if (ret) {
+		pr_err("Error initializing the wl12xx enable gpio: %d\n", ret);
+		return;
+	}
+
+	ret = gpio_request_one(DA850_WLAN_IRQ, GPIOF_IN, "wl12xx_irq");
+	if (ret) {
+		pr_err("Error initializing the wl12xx irq gpio: %d\n", ret);
+		gpio_free(DA850_WLAN_EN);
+		return;