2010-07-06 00:39:11

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 00/15] wlan+omap+mmc: out-of-the-box WLAN support for ZOOM2/3

From: Ohad Ben-Cohen <[email protected]>

The ZOOM2/3 boards include TI's wl1271 wlan sdio device,
hardwired to the 3rd mmc controller.

These patches add support for WLAN on the ZOOM2/3 boards
using only mainline components (most notably mac80211 and wl1271).

Patches were tested on both ZOOM2 and ZOOM3.

In short, these patches add software control for emulating
card detect events, add board configurations to support the
wl1271 device, and update the wl1271 driver to make use of
these new mechanisms.

Software card detect emulation is based on Android's
EMBEDDED_SDIO patch by San Mehat <[email protected]> (thanks, San!).

These patches span over several differnt subsystems, but since
they are highly dependent on each other, it is preferrable
to pull them all together into a single tree (once approved).

Patches are available at:

git://wizery.com/pub/linux-2.6.git wl1271

And will also be sent as a follow-on to this message to the
omap, mmc, arm and wireless mailing lists.

Patches are based on mainline 2.6.35-rc4, but can easily be applied
on wireless-testing (with two minor conflicts). If desired, I can
rebase to wireless-testing and resend.

Note: last missing part for full mainline community support
of the wl1271 on ZOOM is the firmware, and for that there is already
on-going TI work to provide it in linux-firmware. Hopefully
that would be resolved soon.

Thanks,

Ohad Ben-Cohen (15):
sdio: add TI + wl1271 ids
wireless: wl1271: remove SDIO IDs from driver
omap: mmc: prepare for software card detect support
mmc: support embedded data field in mmc_host
omap: hsmmc: add virtual card detect support
omap zoom2: wlan board muxing
omap zoom3: wlan board muxing
wireless: wl1271: make wl12xx.h common to both spi and sdio
wireless: wl12xx: support pdata SDIO handlers
wireless: wl1271: support return value for the set power func
wireless: wl1271: introduce platform device support
wireless: wl1271: take irq info from platform data
wireless: wl1271: make ref_clock configurable by board
omap: zoom: add WLAN device
omap: zoom: enable WLAN device

arch/arm/mach-omap2/Kconfig | 5 +
arch/arm/mach-omap2/Makefile | 1 +
arch/arm/mach-omap2/board-zoom-peripherals.c | 15 ++
arch/arm/mach-omap2/board-zoom-wlan.c | 129 ++++++++++++++++
arch/arm/mach-omap2/board-zoom2.c | 15 ++
arch/arm/mach-omap2/board-zoom3.c | 15 ++
arch/arm/mach-omap2/hsmmc.c | 4 +
arch/arm/mach-omap2/hsmmc.h | 5 +
arch/arm/mach-omap2/include/mach/board-zoom.h | 5 +
arch/arm/plat-omap/include/plat/mmc.h | 5 +
drivers/mmc/core/Kconfig | 8 +
drivers/mmc/host/omap_hsmmc.c | 37 +++++-
drivers/net/wireless/wl12xx/Kconfig | 1 +
drivers/net/wireless/wl12xx/wl1251_sdio.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_spi.c | 2 +-
drivers/net/wireless/wl12xx/wl1271.h | 8 +-
drivers/net/wireless/wl12xx/wl1271_boot.c | 13 +-
drivers/net/wireless/wl12xx/wl1271_boot.h | 1 -
drivers/net/wireless/wl12xx/wl1271_io.h | 8 +-
drivers/net/wireless/wl12xx/wl1271_main.c | 4 +-
drivers/net/wireless/wl12xx/wl1271_sdio.c | 204 +++++++++++++++++++------
drivers/net/wireless/wl12xx/wl1271_spi.c | 8 +-
include/linux/mmc/host.h | 16 ++
include/linux/mmc/sdio_ids.h | 3 +
include/linux/spi/wl12xx.h | 34 ----
include/linux/wl12xx.h | 37 +++++
26 files changed, 486 insertions(+), 99 deletions(-)
create mode 100644 arch/arm/mach-omap2/board-zoom-wlan.c
delete mode 100644 include/linux/spi/wl12xx.h
create mode 100644 include/linux/wl12xx.h



2010-07-06 00:41:11

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 15/15] omap: zoom: enable WLAN device

From: Ohad Ben-Cohen <[email protected]>

Make it possible to build and use TI's wl1271
device on the ZOOM boards.

The device is an embedded SDIO WLAN chip
that is hardwired to the 3rd mmc controller
of the ZOOM2/3 boards.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-omap2/Kconfig | 5 +++++
arch/arm/mach-omap2/Makefile | 1 +
arch/arm/mach-omap2/board-zoom-peripherals.c | 15 +++++++++++++++
3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index b31b6f1..7fee11b 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -131,6 +131,11 @@ config MACH_OMAP_ZOOM3
depends on ARCH_OMAP3
select OMAP_PACKAGE_CBP

+config OMAP_ZOOM_WLAN
+ bool "OMAP Zoom board WLAN support"
+ depends on MACH_OMAP_ZOOM2 || MACH_OMAP_ZOOM3
+ select MMC_EMBEDDED_SDIO
+
config MACH_CM_T35
bool "CompuLab CM-T35 module"
depends on ARCH_OMAP3
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index ea52b03..ac1bad9 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -129,6 +129,7 @@ obj-$(CONFIG_MACH_OMAP_ZOOM3) += board-zoom3.o \
board-zoom-peripherals.o \
hsmmc.o \
board-zoom-debugboard.o
+obj-y += board-zoom-wlan.o
obj-$(CONFIG_MACH_OMAP_3630SDP) += board-3630sdp.o \
board-zoom-peripherals.o \
hsmmc.o
diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-omap2/board-zoom-peripherals.c
index 6b39849..3128cd4 100644
--- a/arch/arm/mach-omap2/board-zoom-peripherals.c
+++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
@@ -16,11 +16,13 @@
#include <linux/gpio.h>
#include <linux/i2c/twl.h>
#include <linux/regulator/machine.h>
+#include <linux/mmc/host.h>

#include <asm/mach-types.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>

+#include <mach/board-zoom.h>
#include <plat/common.h>
#include <plat/usb.h>

@@ -168,6 +170,18 @@ static struct omap2_hsmmc_info mmc[] __initdata = {
.nonremovable = true,
.power_saving = true,
},
+#ifdef CONFIG_OMAP_ZOOM_WLAN
+ {
+ .mmc = 3,
+ .wires = 4,
+ .gpio_cd = -EINVAL,
+ .gpio_wp = -EINVAL,
+ .register_embedded_control =
+ omap_zoom_wlan_register_embedded_control,
+ .virtual_get_cd = omap_zoom_wlan_get_virtual_cd,
+ .ocr_mask = MMC_VDD_165_195,
+ },
+#endif
{} /* Terminator */
};

@@ -282,4 +296,5 @@ void __init zoom_peripherals_init(void)
omap_i2c_init();
usb_musb_init(&musb_board_data);
enable_board_wakeup_source();
+ omap_zoom_wlan_init();
}
--
1.7.0.4


2010-07-09 08:13:09

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Ohad,

On 07/08/2010 11:10 PM, ext Ohad Ben-Cohen wrote:
> Hi Nicolas and Roger,
>
> On Tue, Jul 6, 2010 at 8:42 PM, Nicolas Pitre<[email protected]> wrote:
>> On Tue, 6 Jul 2010, Roger Quadros wrote:
>>> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
>>> the SDIO/MMC core should tackle it, just like it deals with supply for slots
>>> with removable cards.
>>
> ...
>> Another function pair would be needed instead, which would do almost
>> like the suspend/resume code is already doing. Something like:
>
> Thanks a lot for your review and comments, and for taking the time to
> present your approach.

you're welcome :)
>
> I like it !
>
> It'd allow us to lose the software (or fake if you want ;) card detect
> mechanism, which is something that should have been added to each
> platform we wanted to support.
>
> We would only need to make it possible to deliver board-specific data
> to the function driver (e.g., in the case of the wl1271, we need irq
> and board_ref_clock data). That would require some board-level
> platform-data configuration, which will be specific to the controller
> to which the device is hardwired to. This data should propagate
> through the host controller to the SDIO core so it would eventually be

why should platform data go through the host controller? You are already
creating a new platform device for wl1271_sdio just to pass platform_data so the
platform data for that can be passed directly from board files to this platform
device. It doesn't need to go via host controller.

I think this case is specific to wl1271. Ideally wl1271 should have used the
SDIO's in-band interrupt delivery mechanism and SDIO Configuration space to
provide this information to make it truly plug n play.

br,
-roger

2010-07-08 03:40:00

by Ghorai, Sukumar

[permalink] [raw]
Subject: RE: [PATCH 14/15] omap: zoom: add WLAN device



> -----Original Message-----
> From: [email protected] [mailto:linux-mmc-
> [email protected]] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, July 06, 2010 6:08 AM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: [email protected]; [email protected];
> Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm@linux-
> foundation.org; San Mehat; Ben-cohen, Ohad
> Subject: [PATCH 14/15] omap: zoom: add WLAN device
>
> From: Ohad Ben-Cohen <[email protected]>
>
> Add WLAN platform device and control
> functions (power and virtual card detect)
> in order to allow software to control the
> embedded SDIO WLAN device which resides on
> the ZOOM board (TI's wl1271 device).
>
> Based on Android's WLAN control functions by
> San Mehat <[email protected]>.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> arch/arm/mach-omap2/board-zoom-wlan.c | 129
> +++++++++++++++++++++++++
> arch/arm/mach-omap2/include/mach/board-zoom.h | 5 +
> 2 files changed, 134 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/board-zoom-wlan.c
>
> diff --git a/arch/arm/mach-omap2/board-zoom-wlan.c b/arch/arm/mach-
> omap2/board-zoom-wlan.c
> new file mode 100644
> index 0000000..7ed5139
> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-zoom-wlan.c
> @@ -0,0 +1,129 @@
> +/* mach-omap2/board-zoom-wlan.c
> + *
> + * Board support for wl1271 embedded SDIO device.
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + *
> + * This file is licensed under the terms of the GNU General Public
> License
> + * version 2. This program is licensed "as is" without any warranty of
> any
> + * kind, whether express or implied.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/sdio_ids.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/wl12xx.h>
> +
> +#include "mux.h"
> +
> +#ifdef CONFIG_OMAP_ZOOM_WLAN
[Ghorai] Again the file itself is zoom specific, why we need the additional flag?

> +
> +/* these are zoom-specific board numbers */
> +#define OMAP_ZOOM_WLAN_PMENA_GPIO (101)
> +#define OMAP_ZOOM_WLAN_IRQ_GPIO (162)
> +
> +/* wl1271 virtual 'card detect' status */
> +static int omap_zoom_wlan_cd;
> +static void (*wlan_set_virtual_cd)(void *dev_id, int card_present);
> +static void (*wlan_set_data)(void *dev_id, void *priv);
> +static void *wlan_host_devid;
> +
> +int omap_zoom_wlan_register_embedded_control(void *dev_id,
> + void (*set_virtual_cd)(void *dev_id, int card_present),
> + void (*set_data)(void *dev_id, void *priv))
> +{
> + if (wlan_host_devid || wlan_set_virtual_cd || wlan_set_data)
> + return -EBUSY;
> +
> + wlan_set_virtual_cd = set_virtual_cd;
> + wlan_set_data = set_data;
> + wlan_host_devid = dev_id;
> +
> + return 0;
> +}
> +
> +int omap_zoom_wlan_get_virtual_cd(void)
> +{
> + return omap_zoom_wlan_cd;
> +}
> +
> +static void omap_zoom_wlan_set_embedded_data(void *priv)
> +{
> + if (wlan_set_data)
> + wlan_set_data(wlan_host_devid, priv);
> + else
> + pr_err("%s: host controller not registered yet\n", __func__);
> +}
> +
> +static void omap_zoom_wlan_set_carddetect(bool card_present)
> +{
> + omap_zoom_wlan_cd = card_present ? 1 : 0;
> +
> + pr_info("%s: %d\n", __func__, omap_zoom_wlan_cd);
> +
> + if (wlan_set_virtual_cd)
> + wlan_set_virtual_cd(wlan_host_devid, omap_zoom_wlan_cd);
> + else
> + pr_err("%s: host controller not registered yet\n", __func__);
> +}
> +
> +static void omap_zoom_wlan_power(bool enable)
> +{
> + int val = enable ? 1 : 0;
> +
> + pr_info("%s: set power %d\n", __func__, val);
> +
> + gpio_set_value(OMAP_ZOOM_WLAN_PMENA_GPIO, val);
> +}
> +
> +struct wl12xx_platform_data omap_zoom_wlan_control = {
> + .set_power = omap_zoom_wlan_power,
> + .set_carddetect = omap_zoom_wlan_set_carddetect,
> + .set_embedded_data = omap_zoom_wlan_set_embedded_data,
> + /* ZOOM ref clock is 26 MHz */
> + .board_ref_clock = 1,
> + .irq = OMAP_GPIO_IRQ(OMAP_ZOOM_WLAN_IRQ_GPIO),
> +};
> +
> +static struct platform_device omap_zoom_wlan_device = {
> + .name = "wl1271_sdio",
> + .id = 1,
> + .dev = {
> + .platform_data = &omap_zoom_wlan_control,
> + },
> +};
> +
> +int __init omap_zoom_wlan_init(void)
> +{
> + int ret;
> +
> + ret = gpio_request(OMAP_ZOOM_WLAN_PMENA_GPIO, "wlan_power");
> + if (ret < 0) {
> + pr_err("%s: power gpio request failed: %d\n", __func__, ret);
> + return ret;
> + }
> +
> + gpio_direction_output(OMAP_ZOOM_WLAN_PMENA_GPIO, 0);
> +
> + ret = gpio_request(OMAP_ZOOM_WLAN_IRQ_GPIO, "wlan_irq");
> + if (ret < 0) {
> + pr_err("%s: gpio request failed: %d\n", __func__, ret);
> + return ret;
> + }
> + gpio_direction_input(OMAP_ZOOM_WLAN_IRQ_GPIO);
> +
> + ret = platform_device_register(&omap_zoom_wlan_device);
> +
> + return ret;
> +}
> +
> +#else
> +int __init omap_zoom_wlan_init(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_OMAP_ZOOM_WLAN */
> diff --git a/arch/arm/mach-omap2/include/mach/board-zoom.h
> b/arch/arm/mach-omap2/include/mach/board-zoom.h
> index c93b29e..61bbd81 100644
> --- a/arch/arm/mach-omap2/include/mach/board-zoom.h
> +++ b/arch/arm/mach-omap2/include/mach/board-zoom.h
> @@ -3,3 +3,8 @@
> */
> extern int __init zoom_debugboard_init(void);
> extern void __init zoom_peripherals_init(void);
> +int __init omap_zoom_wlan_init(void);
> +int omap_zoom_wlan_register_embedded_control(void *dev_id,
> + void (*set_virtual_cd)(void *dev_id, int card_present),
> + void (*set_data)(void *dev_id, void *priv));
> +int omap_zoom_wlan_get_virtual_cd(void);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-07-06 00:41:04

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 14/15] omap: zoom: add WLAN device

From: Ohad Ben-Cohen <[email protected]>

Add WLAN platform device and control
functions (power and virtual card detect)
in order to allow software to control the
embedded SDIO WLAN device which resides on
the ZOOM board (TI's wl1271 device).

Based on Android's WLAN control functions by
San Mehat <[email protected]>.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-omap2/board-zoom-wlan.c | 129 +++++++++++++++++++++++++
arch/arm/mach-omap2/include/mach/board-zoom.h | 5 +
2 files changed, 134 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/board-zoom-wlan.c

diff --git a/arch/arm/mach-omap2/board-zoom-wlan.c b/arch/arm/mach-omap2/board-zoom-wlan.c
new file mode 100644
index 0000000..7ed5139
--- /dev/null
+++ b/arch/arm/mach-omap2/board-zoom-wlan.c
@@ -0,0 +1,129 @@
+/* mach-omap2/board-zoom-wlan.c
+ *
+ * Board support for wl1271 embedded SDIO device.
+ *
+ * Copyright (C) 2010 Texas Instruments, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/sdio_ids.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/wl12xx.h>
+
+#include "mux.h"
+
+#ifdef CONFIG_OMAP_ZOOM_WLAN
+
+/* these are zoom-specific board numbers */
+#define OMAP_ZOOM_WLAN_PMENA_GPIO (101)
+#define OMAP_ZOOM_WLAN_IRQ_GPIO (162)
+
+/* wl1271 virtual 'card detect' status */
+static int omap_zoom_wlan_cd;
+static void (*wlan_set_virtual_cd)(void *dev_id, int card_present);
+static void (*wlan_set_data)(void *dev_id, void *priv);
+static void *wlan_host_devid;
+
+int omap_zoom_wlan_register_embedded_control(void *dev_id,
+ void (*set_virtual_cd)(void *dev_id, int card_present),
+ void (*set_data)(void *dev_id, void *priv))
+{
+ if (wlan_host_devid || wlan_set_virtual_cd || wlan_set_data)
+ return -EBUSY;
+
+ wlan_set_virtual_cd = set_virtual_cd;
+ wlan_set_data = set_data;
+ wlan_host_devid = dev_id;
+
+ return 0;
+}
+
+int omap_zoom_wlan_get_virtual_cd(void)
+{
+ return omap_zoom_wlan_cd;
+}
+
+static void omap_zoom_wlan_set_embedded_data(void *priv)
+{
+ if (wlan_set_data)
+ wlan_set_data(wlan_host_devid, priv);
+ else
+ pr_err("%s: host controller not registered yet\n", __func__);
+}
+
+static void omap_zoom_wlan_set_carddetect(bool card_present)
+{
+ omap_zoom_wlan_cd = card_present ? 1 : 0;
+
+ pr_info("%s: %d\n", __func__, omap_zoom_wlan_cd);
+
+ if (wlan_set_virtual_cd)
+ wlan_set_virtual_cd(wlan_host_devid, omap_zoom_wlan_cd);
+ else
+ pr_err("%s: host controller not registered yet\n", __func__);
+}
+
+static void omap_zoom_wlan_power(bool enable)
+{
+ int val = enable ? 1 : 0;
+
+ pr_info("%s: set power %d\n", __func__, val);
+
+ gpio_set_value(OMAP_ZOOM_WLAN_PMENA_GPIO, val);
+}
+
+struct wl12xx_platform_data omap_zoom_wlan_control = {
+ .set_power = omap_zoom_wlan_power,
+ .set_carddetect = omap_zoom_wlan_set_carddetect,
+ .set_embedded_data = omap_zoom_wlan_set_embedded_data,
+ /* ZOOM ref clock is 26 MHz */
+ .board_ref_clock = 1,
+ .irq = OMAP_GPIO_IRQ(OMAP_ZOOM_WLAN_IRQ_GPIO),
+};
+
+static struct platform_device omap_zoom_wlan_device = {
+ .name = "wl1271_sdio",
+ .id = 1,
+ .dev = {
+ .platform_data = &omap_zoom_wlan_control,
+ },
+};
+
+int __init omap_zoom_wlan_init(void)
+{
+ int ret;
+
+ ret = gpio_request(OMAP_ZOOM_WLAN_PMENA_GPIO, "wlan_power");
+ if (ret < 0) {
+ pr_err("%s: power gpio request failed: %d\n", __func__, ret);
+ return ret;
+ }
+
+ gpio_direction_output(OMAP_ZOOM_WLAN_PMENA_GPIO, 0);
+
+ ret = gpio_request(OMAP_ZOOM_WLAN_IRQ_GPIO, "wlan_irq");
+ if (ret < 0) {
+ pr_err("%s: gpio request failed: %d\n", __func__, ret);
+ return ret;
+ }
+ gpio_direction_input(OMAP_ZOOM_WLAN_IRQ_GPIO);
+
+ ret = platform_device_register(&omap_zoom_wlan_device);
+
+ return ret;
+}
+
+#else
+int __init omap_zoom_wlan_init(void)
+{
+ return 0;
+}
+#endif /* CONFIG_OMAP_ZOOM_WLAN */
diff --git a/arch/arm/mach-omap2/include/mach/board-zoom.h b/arch/arm/mach-omap2/include/mach/board-zoom.h
index c93b29e..61bbd81 100644
--- a/arch/arm/mach-omap2/include/mach/board-zoom.h
+++ b/arch/arm/mach-omap2/include/mach/board-zoom.h
@@ -3,3 +3,8 @@
*/
extern int __init zoom_debugboard_init(void);
extern void __init zoom_peripherals_init(void);
+int __init omap_zoom_wlan_init(void);
+int omap_zoom_wlan_register_embedded_control(void *dev_id,
+ void (*set_virtual_cd)(void *dev_id, int card_present),
+ void (*set_data)(void *dev_id, void *priv));
+int omap_zoom_wlan_get_virtual_cd(void);
--
1.7.0.4


2010-07-06 07:11:56

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 10/15] wireless: wl1271: support return value for the set power func

On Tue, 2010-07-06 at 02:37 +0200, ext Ohad Ben-Cohen wrote:
> From: Ohad Ben-Cohen <[email protected]>
>
> Make it possible for the set power method to indicate a
> success/failure return value. This is needed to support
> more complex power on/off operations such as bringing up
> (and down) sdio functions.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---

Acked-by: Luciano Coelho <[email protected]>


--
Cheers,
Luca.


2010-07-06 00:40:49

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 12/15] wireless: wl1271: take irq info from platform data

From: Ohad Ben-Cohen <[email protected]>

Remove the hard coded irq information, and instead take
the irq information from the platform-specific data
which is supplied by the board device.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_sdio.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 96b8fc3..0af0dce 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -38,8 +38,6 @@
#include "wl12xx_80211.h"
#include "wl1271_io.h"

-
-#define RX71_WL1271_IRQ_GPIO 42
static DECLARE_COMPLETION(wl1271_sdio_ready);

static const struct sdio_device_id wl1271_devices[] = {
@@ -286,8 +284,6 @@ static int wl1271_plat_probe(struct platform_device *pdev)
goto out_free;
}

- wl->irq = gpio_to_irq(RX71_WL1271_IRQ_GPIO);
-
wl->set_power = pdata->set_power;
if (!wl->set_power) {
wl1271_error("set power function missing in platform data");
@@ -295,9 +291,10 @@ static int wl1271_plat_probe(struct platform_device *pdev)
goto out_free;
}

+ wl->irq = pdata->irq;
if (wl->irq < 0) {
- ret = wl->irq;
- wl1271_error("could not get irq!");
+ wl1271_error("irq missing in platform data");
+ ret = -ENODEV;
goto out_free;
}

--
1.7.0.4


2010-07-06 17:00:13

by Nicolas Pitre

[permalink] [raw]
Subject: RE: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On Tue, 6 Jul 2010, Madhusudhan wrote:

>
>
> > -----Original Message-----
> > From: Ohad Ben-Cohen [mailto:[email protected]]
> > Sent: Tuesday, July 06, 2010 6:48 AM
> > To: Nicolas Pitre
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; Chikkature Rajashekar Madhusudhan; Luciano Coelho;
> > Andrew Morton; San Mehat; Pandita, Vikram
> > Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support
> >
> > On Tue, Jul 6, 2010 at 1:22 PM, Ohad Ben-Cohen <[email protected]> wrote:
> > > Note: the wl1271 device does support standard card detection, but
> > > AFAIK there's a limitation to use that with the specific omap
> > > controller the device is hardwired to. I will try to get more info
> > > about that, but probably Madhu can comment on that better.
> >
> >
> > Some correction and additional info:
> >
> > The wl1271 device has an issue which makes the standard card detect
> > mechanism irrelevant: it is always up, even if the power enable gpio
> > input of the device is down (the power enable input does not supply
> > the power to the chip, it's just logical digital high/low input upon
> > which the device reacts). That's why we must use software control for
> > emulating card detect with that device.
> >
> > In addition, as far as I could find out, the card detect mechanism on
> > the ZOOM is implemented by mechanical means, and thus is not relevant
> > for hardwired embedded SDIO devices (I'm not even sure card detect is
> > supported for the 3rd mmc controller).
>
> The card detect is supported through T2 GPIO interrupts only for MMC1 and
> MMC2. Such a mechanism is not present for MMC3 to which the WLAN chip is
> hardwired.

Many existing implementations simply have no (or a broken) card
detection signal. In that case, either the host controller passes the
MMC_CAP_NEEDS_POLL flag to the core so that the bus is probed on a
regular interval for card presence. Or, like in this case where the
"card" is always hardwired on the board, you simply rely on the initial
probe which is always performed at least once when the host controller
driver is registered.

Now the issue of having the card powered off when not in use is a valid
one, whether or not it is actually hardwired on the board or
hot insertable/removable. This fake insertion thing is not the best way
to go about it.

It would be way more useful, generic, and less hackish to simply improve
the generic code so to power down the card when it is 1) not claimed by
any function driver, and 2) provide an API to let a function driver
signify to the core and host controller that it is not interested by the
hardware at the moment (if the network interface is not up for example)
and therefore the core could again power down the card. This would work
in all cases with no need for exceptions for so called "enbedded
controllers".


Nicolas

2010-07-06 11:03:07

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On 07/06/2010 01:22 PM, ext Ohad Ben-Cohen wrote:
> Hi Nicolas,
>
> On Tue, Jul 6, 2010 at 4:45 AM, Nicolas Pitre<[email protected]> wrote:
>> On Tue, 6 Jul 2010, Ohad Ben-Cohen wrote:
>>
>>> From: Ohad Ben-Cohen<[email protected]>
>>>
>>> Add support for software emulation of card detect
>>> events.
>>>
>>> This is required for specific controllers
>>> that are hard wired with embedded SDIO devices
>>> (such as TI's wl1271 WLAN device).
>>
>> Why?
>>
>> Many instances of hardwired SDIO based WLAN devices do exist already,
>> and they don't need extra card detect events to be generated. The core
>> MMC code already triggers a probe for cards whenever a new host
>> controller is registered.
>
> We prefer not to power up the chip as early as boot time; instead, we
Why? if the card is hard wired with no card detect then it should be powered up.
The function driver should power it down later if required. no?

> would like to have it powered off until the wlan interface is brought
> up, power it on when the interface is brought up,

If it was powered OFF then how did the wlan interface come into picture?

> and power it off again as soon as the interface is brought down again
> (to minimize power consumption when the wlan is not in use).
I agree, we some how need to power down the card when not in use in the right way.

2010-07-08 03:39:58

by Ghorai, Sukumar

[permalink] [raw]
Subject: RE: [PATCH 00/15] wlan+omap+mmc: out-of-the-box WLAN support for ZOOM2/3



> -----Original Message-----
> From: [email protected] [mailto:linux-mmc-
> [email protected]] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, July 06, 2010 6:08 AM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: [email protected]; [email protected];
> Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm@linux-
> foundation.org; San Mehat; Ben-cohen, Ohad
> Subject: [PATCH 00/15] wlan+omap+mmc: out-of-the-box WLAN support for
> ZOOM2/3
>
> From: Ohad Ben-Cohen <[email protected]>
>
> The ZOOM2/3 boards include TI's wl1271 wlan sdio device,
> hardwired to the 3rd mmc controller.
>
> These patches add support for WLAN on the ZOOM2/3 boards
> using only mainline components (most notably mac80211 and wl1271).
>
> Patches were tested on both ZOOM2 and ZOOM3.
>
> In short, these patches add software control for emulating
> card detect events, add board configurations to support the
> wl1271 device, and update the wl1271 driver to make use of
> these new mechanisms.
>
> Software card detect emulation is based on Android's
> EMBEDDED_SDIO patch by San Mehat <[email protected]> (thanks, San!).
>
> These patches span over several differnt subsystems, but since
> they are highly dependent on each other, it is preferrable
> to pull them all together into a single tree (once approved).
>
> Patches are available at:
>
> git://wizery.com/pub/linux-2.6.git wl1271
>
> And will also be sent as a follow-on to this message to the
> omap, mmc, arm and wireless mailing lists.
>
> Patches are based on mainline 2.6.35-rc4, but can easily be applied
> on wireless-testing (with two minor conflicts). If desired, I can
> rebase to wireless-testing and resend.
>
> Note: last missing part for full mainline community support
> of the wl1271 on ZOOM is the firmware, and for that there is already
> on-going TI work to provide it in linux-firmware. Hopefully
> that would be resolved soon.
>
> Thanks,
>
> Ohad Ben-Cohen (15):
> sdio: add TI + wl1271 ids
> wireless: wl1271: remove SDIO IDs from driver
> omap: mmc: prepare for software card detect support
> mmc: support embedded data field in mmc_host
> omap: hsmmc: add virtual card detect support
> omap zoom2: wlan board muxing
> omap zoom3: wlan board muxing
> wireless: wl1271: make wl12xx.h common to both spi and sdio
> wireless: wl12xx: support pdata SDIO handlers
> wireless: wl1271: support return value for the set power func
> wireless: wl1271: introduce platform device support
> wireless: wl1271: take irq info from platform data
> wireless: wl1271: make ref_clock configurable by board
> omap: zoom: add WLAN device
> omap: zoom: enable WLAN device
>
> arch/arm/mach-omap2/Kconfig | 5 +
> arch/arm/mach-omap2/Makefile | 1 +
> arch/arm/mach-omap2/board-zoom-peripherals.c | 15 ++
> arch/arm/mach-omap2/board-zoom-wlan.c | 129 ++++++++++++++++
> arch/arm/mach-omap2/board-zoom2.c | 15 ++
> arch/arm/mach-omap2/board-zoom3.c | 15 ++
> arch/arm/mach-omap2/hsmmc.c | 4 +
> arch/arm/mach-omap2/hsmmc.h | 5 +
> arch/arm/mach-omap2/include/mach/board-zoom.h | 5 +
> arch/arm/plat-omap/include/plat/mmc.h | 5 +
> drivers/mmc/core/Kconfig | 8 +
> drivers/mmc/host/omap_hsmmc.c | 37 +++++-
> drivers/net/wireless/wl12xx/Kconfig | 1 +
> drivers/net/wireless/wl12xx/wl1251_sdio.c | 2 +-
> drivers/net/wireless/wl12xx/wl1251_spi.c | 2 +-
> drivers/net/wireless/wl12xx/wl1271.h | 8 +-
> drivers/net/wireless/wl12xx/wl1271_boot.c | 13 +-
> drivers/net/wireless/wl12xx/wl1271_boot.h | 1 -
> drivers/net/wireless/wl12xx/wl1271_io.h | 8 +-
> drivers/net/wireless/wl12xx/wl1271_main.c | 4 +-
> drivers/net/wireless/wl12xx/wl1271_sdio.c | 204 +++++++++++++++++++-
> -----
> drivers/net/wireless/wl12xx/wl1271_spi.c | 8 +-
> include/linux/mmc/host.h | 16 ++
> include/linux/mmc/sdio_ids.h | 3 +
> include/linux/spi/wl12xx.h | 34 ----
> include/linux/wl12xx.h | 37 +++++
> 26 files changed, 486 insertions(+), 99 deletions(-)
> create mode 100644 arch/arm/mach-omap2/board-zoom-wlan.c
> delete mode 100644 include/linux/spi/wl12xx.h
> create mode 100644 include/linux/wl12xx.h
>
[Ghorai] This patch series having the CONFIG_MMC_EMBEDDED_SDIO as kconfig option and I feel we should void this. This could be a generic and can be get from sdio card at runtime. Quite long codes are adding in this patch series under this flag.

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

2010-07-08 08:55:46

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On 07/07/2010 04:52 PM, ext Nicolas Pitre wrote:
> On Wed, 7 Jul 2010, Roger Quadros wrote:
>
>> On 07/06/2010 08:42 PM, ext Nicolas Pitre wrote:
>>> On Tue, 6 Jul 2010, Roger Quadros wrote:
>>>
>>>> OK, this is how I see it.
>>>>
>>>> - Treat the non-removable card as non-removable. So no need to do card
>>>> detect
>>>> emulation.
>>>>
>>>> - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed
>>>> regulator
>>>> framework to define this regulator& supply. Even though you mention that
>>>> it
>>>> is not actually a supply, it fits well in the fixed supply framework.
>>>>
>>>> - When the host controller is enumerated, the mmc core will power up the
>>>> slot,
>>>> find the sdio card, and probe the function driver (i.e. wl1271_sdio).
>>>>
>>>> - if interface is not in use, the function driver must release the sdio
>>>> host,
>>>> and this should eventually disable the vmmc supply.
>>>>
>>>> - Whenever the wlan interface must be brought up, wl1271_sdio, can claim
>>>> the
>>>> sdio host. this will cause the vmmc supply to be enabled, for as long as
>>>> the
>>>> interface is up.
>>>>
>>>> Does this address all issues?
>>>
>>> This is mostly all good, except that claiming/releasing the SDIO host is
>>> about access to the bus. It must be claimed right before doing any IO,
>>> and released right after that, even when the card is expected to remain
>>> powered. This is not the proper place to hook power control.
>>
>> Agreed, but is it so that SDIO power may be removed between a host_release and
>> claim? This appears so from omap_hsmmc host controller.
>
> No, it is not because a host is not claimed that power should be
> dropped. The host claim/release is meant to provide exclusive access to
> the card that's all.
>
> If the OMAP controller is dropping power to the card upon
> host->disable() then it is wrong. AFAICS only the OMAP controller is
> playing such games at the moment and I suspect the semantics might not
> be all right. Shutting down the _controller_ when it is idle might be a
> good thing, but not power to the _card_. Only the function driver might
> know when it is fine to lose power.
>
>
I completely agree with you Nicolas. omap_hsmmc needs to be fixed so that it
does not control MMC/SDIO slot supply voltage. It should deal with only the mmc
controller power savings and not the card/function power savings.

regards,
-roger

2010-07-06 13:47:53

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 14/15] omap: zoom: add WLAN device

Hi Roger,

On Tue, Jul 6, 2010 at 3:33 PM, Roger Quadros <[email protected]> wrote:
>> +static void omap_zoom_wlan_power(bool enable)
>> +{
>> + ? ? ? int val = enable ? 1 : 0;
>> +
>> + ? ? ? pr_info("%s: set power %d\n", __func__, val);
>> +
>> + ? ? ? gpio_set_value(OMAP_ZOOM_WLAN_PMENA_GPIO, val);
>> +}
>
> Can we consider that OMAP_ZOOM_WLAN_PMENA_GPIO is equivalent to vmmc supply
> or equivalent to supply voltage to the SDIO card?

Not really, this gpio does not supply power to the chip. It's only a
digital indication that instructs the chip to go into on or off mode.

Thanks,
Ohad.

2010-07-06 13:45:12

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On Tue, Jul 6, 2010 at 3:39 PM, Roger Quadros <[email protected]> wrote:
> Just to clarify, is the wl1271 device always powered on the board?

Yes.

> And this GPIO power enable (OMAP_ZOOM_WLAN_PMENA_GPIO) is used to gate this
> supply internally?

Yes. It's a digital input that the chip does not draw current from
(well, only a very minimal one).

> and what do these do ?
>
> set_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
>
> clear_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);

This is an internal driver bit that maintains the power state of the chip.
AFAICT, it does not "do" anything. The only place I can see it is
being used is in a debugfs entry.

2010-07-06 14:31:28

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On 07/06/2010 03:53 PM, ext Ohad Ben-Cohen wrote:
> Hi Roger,
>
> On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros<[email protected]> wrote:
>> My point is that shouldn't this be handled by SDIO core?
>
> Care to explain what you mean / give a code example ?

If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
the SDIO/MMC core should tackle it, just like it deals with supply for slots
with removable cards.

see mmc_regulator_set_ocr()
mmc_power_up()
mmc_set_ios()

in drivers/mmc/core/core.c

and omap_hsmmc_set_ios()

in drivers/mmc/host/omap_hsmmc.c

>
>> If there are no users for the SDIO function and the card, doesn't the SDIO
>> core power down the slot and take care of re-initialization when it is
>> powered up?
>
> You need card detect events in order to trigger card& sdio function
> initialization and removals.
>
> Please share any alternative approach you may be thinking on.

OK, this is how I see it.

- Treat the non-removable card as non-removable. So no need to do card detect
emulation.

- Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed regulator
framework to define this regulator & supply. Even though you mention that it is
not actually a supply, it fits well in the fixed supply framework.

- When the host controller is enumerated, the mmc core will power up the slot,
find the sdio card, and probe the function driver (i.e. wl1271_sdio).

- if interface is not in use, the function driver must release the sdio host,
and this should eventually disable the vmmc supply.

- Whenever the wlan interface must be brought up, wl1271_sdio, can claim the
sdio host. this will cause the vmmc supply to be enabled, for as long as the
interface is up.

Does this address all issues?

regards,
-roger

2010-07-06 15:55:03

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 04/15] mmc: support embedded data field in mmc_host

On Tue, Jul 6, 2010 at 6:49 PM, Grazvydas Ignotas <[email protected]> wrote:
> Hm, do we really need a Kconfig option just for a single pointer? It
> only saves sizeof(void *) bytes per host, but adds rather confusing
> config option for users and some ifdef complexity.

No strong feelings about it, I can remove that if preferred.

>

2010-07-06 00:40:20

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 07/15] omap zoom3: wlan board muxing

From: Ohad Ben-Cohen <[email protected]>

Add board muxing to support the wlan wl1271 chip that is
hardwired to mmc2 (third mmc controller) on the ZOOM3.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-omap2/board-zoom3.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-zoom3.c b/arch/arm/mach-omap2/board-zoom3.c
index 3314704..0b62663 100644
--- a/arch/arm/mach-omap2/board-zoom3.c
+++ b/arch/arm/mach-omap2/board-zoom3.c
@@ -46,6 +46,21 @@ static void __init omap_zoom_init_irq(void)

#ifdef CONFIG_OMAP_MUX
static struct omap_board_mux board_mux[] __initdata = {
+#ifdef CONFIG_OMAP_ZOOM_WLAN
+ /* WLAN IRQ - GPIO 162 */
+ OMAP3_MUX(MCBSP1_CLKX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN POWER ENABLE - GPIO 101 */
+ OMAP3_MUX(CAM_D2, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+ /* WLAN SDIO: MMC3 CMD */
+ OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE3 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC3 CLK */
+ OMAP3_MUX(ETK_CLK, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC3 DAT[0-3] */
+ OMAP3_MUX(ETK_D3, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D4, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D5, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D6, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+#endif
{ .reg_offset = OMAP_MUX_TERMINATOR },
};
#else
--
1.7.0.4


2010-07-06 00:40:47

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 11/15] wireless: wl1271: introduce platform device support

From: Ohad Ben-Cohen <[email protected]>

Introduce a platform device support which is decoupled
from the life cycle of the sdio function.

The platform device objective is to deliver board-specific
values (like irq, ref_clock, and software card-detect
emulation control).

The driver can then dynamically create (or destroy)
the sdio function whenever the wlan interface is
brought up (or down), as part of the power() operation.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/Kconfig | 1 +
drivers/net/wireless/wl12xx/wl1271.h | 1 +
drivers/net/wireless/wl12xx/wl1271_sdio.c | 185 +++++++++++++++++++++++------
3 files changed, 150 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/Kconfig b/drivers/net/wireless/wl12xx/Kconfig
index 337fc7b..8fb7b5a 100644
--- a/drivers/net/wireless/wl12xx/Kconfig
+++ b/drivers/net/wireless/wl12xx/Kconfig
@@ -66,6 +66,7 @@ config WL1271_SPI
config WL1271_SDIO
tristate "TI wl1271 SDIO support"
depends on WL1271 && MMC && ARM
+ select MMC_EMBEDDED_SDIO
---help---
This module adds support for the SDIO interface of adapters using
TI wl1271 chipset. Select this if your platform is using
diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 5250361..2df57cc 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -352,6 +352,7 @@ struct wl1271 {
bool mac80211_registered;

void *if_priv;
+ void *if_plat_priv;

struct wl1271_if_operations *if_ops;

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 571c6b9..96b8fc3 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -28,6 +28,10 @@
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/completion.h>
+#include <linux/wl12xx.h>
+#include <linux/platform_device.h>
#include <plat/gpio.h>

#include "wl1271.h"
@@ -36,6 +40,7 @@


#define RX71_WL1271_IRQ_GPIO 42
+static DECLARE_COMPLETION(wl1271_sdio_ready);

static const struct sdio_device_id wl1271_devices[] = {
{ SDIO_DEVICE(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271) },
@@ -50,7 +55,9 @@ static inline struct sdio_func *wl_to_func(struct wl1271 *wl)

static struct device *wl1271_sdio_wl_to_dev(struct wl1271 *wl)
{
- return &(wl_to_func(wl)->dev);
+ struct platform_device *pdev = wl->if_plat_priv;
+
+ return &pdev->dev;
}

static irqreturn_t wl1271_irq(int irq, void *cookie)
@@ -146,21 +153,44 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,

static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
{
- struct sdio_func *func = wl_to_func(wl);
+ int ret = 0, timeleft;

- /* Let the SDIO stack handle wlan_enable control, so we
- * keep host claimed while wlan is in use to keep wl1271
- * alive.
- */
if (enable) {
- sdio_claim_host(func);
- sdio_enable_func(func);
+ /* save our wl struct as private mmc data */
+ wl->set_embedded_data(wl);
+
+ INIT_COMPLETION(wl1271_sdio_ready);
+ /* 1271 Power Up Sequence */
+ wl->set_power(true);
+ mdelay(15);
+ wl->set_power(false);
+ mdelay(1);
+ wl->set_power(true);
+ mdelay(70);
+
+ /* emulate card detect event */
+ wl->set_carddetect(true);
+
} else {
- sdio_disable_func(func);
- sdio_release_host(func);
+ wl->set_embedded_data(NULL);
+
+ INIT_COMPLETION(wl1271_sdio_ready);
+
+ wl->set_power(false);
+ mdelay(10);
+
+ wl->set_carddetect(false);
}

- return 0;
+ timeleft = wait_for_completion_interruptible_timeout(&wl1271_sdio_ready,
+ msecs_to_jiffies(5000));
+ if (timeleft <= 0) {
+ wl1271_error("%s: unsuccessful SDIO %s (%d)", __func__,
+ enable ? "init" : "deinit", timeleft);
+ ret = (timeleft == 0 ? -EAGAIN : timeleft);
+ }
+
+ return ret;
}

static struct wl1271_if_operations sdio_ops = {
@@ -174,30 +204,97 @@ static struct wl1271_if_operations sdio_ops = {
.disable_irq = wl1271_sdio_disable_interrupts
};

-static int __devinit wl1271_probe(struct sdio_func *func,
+static int wl1271_sdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
{
+ struct wl1271 *wl = mmc_get_embedded_data(func->card->host);
+
+ /* 2nd func is WLAN, make sure wl context is received */
+ if (func->num != 0x02 || !wl)
+ return -ENODEV;
+
+ sdio_set_drvdata(func, wl);
+
+ wl->if_priv = func;
+
+ /* Grab access to FN0 for ELP reg. */
+ func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
+
+ sdio_claim_host(func);
+ sdio_enable_func(func);
+
+ complete(&wl1271_sdio_ready);
+
+ return 0;
+}
+
+static void wl1271_sdio_remove(struct sdio_func *func)
+{
+ struct wl1271 *wl = sdio_get_drvdata(func);
+
+ wl->if_priv = NULL;
+
+ sdio_disable_func(func);
+ sdio_release_host(func);
+
+ complete(&wl1271_sdio_ready);
+}
+
+static struct sdio_driver wl1271_sdio_driver = {
+ .name = "wl1271_sdio_func",
+ .id_table = wl1271_devices,
+ .probe = wl1271_sdio_probe,
+ .remove = wl1271_sdio_remove,
+};
+
+static int wl1271_plat_probe(struct platform_device *pdev)
+{
+ struct wl12xx_platform_data *pdata;
struct ieee80211_hw *hw;
struct wl1271 *wl;
int ret;

- /* We are only able to handle the wlan function */
- if (func->num != 0x02)
+ pdata = pdev->dev.platform_data;
+ if (!pdata) {
+ wl1271_error("no platform data");
return -ENODEV;
+ }

hw = wl1271_alloc_hw();
- if (IS_ERR(hw))
+ if (IS_ERR(hw)) {
+ wl1271_error("wl1271_alloc_hw failed: %ld", PTR_ERR(hw));
return PTR_ERR(hw);
+ }

wl = hw->priv;

- wl->if_priv = func;
wl->if_ops = &sdio_ops;
+ wl->if_plat_priv = pdev;
+ platform_set_drvdata(pdev, wl);

- /* Grab access to FN0 for ELP reg. */
- func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
+ wl->set_embedded_data = pdata->set_embedded_data;
+ if (!wl->set_embedded_data) {
+ wl1271_error("set embedded_data func missing in platform data");
+ ret = -ENODEV;
+ goto out_free;
+ }
+
+ wl->set_carddetect = pdata->set_carddetect;
+ if (!wl->set_carddetect) {
+ wl1271_error("set carddetect func missing in platform data");
+ ret = -ENODEV;
+ goto out_free;
+ }

wl->irq = gpio_to_irq(RX71_WL1271_IRQ_GPIO);
+
+ wl->set_power = pdata->set_power;
+ if (!wl->set_power) {
+ wl1271_error("set power function missing in platform data");
+ ret = -ENODEV;
+ goto out_free;
+ }
+
if (wl->irq < 0) {
ret = wl->irq;
wl1271_error("could not get irq!");
@@ -215,53 +312,67 @@ static int __devinit wl1271_probe(struct sdio_func *func,
disable_irq(wl->irq);

ret = wl1271_init_ieee80211(wl);
- if (ret)
+ if (ret) {
+ wl1271_error("wl1271_init_ieee80211 failed: %d", ret);
goto out_irq;
+ }

ret = wl1271_register_hw(wl);
- if (ret)
+ if (ret) {
+ wl1271_error("wl1271_register_hw failed: %d", ret);
goto out_irq;
+ }

- sdio_set_drvdata(func, wl);
+ ret = sdio_register_driver(&wl1271_sdio_driver);
+ if (ret < 0) {
+ wl1271_error("failed to register sdio driver: %d", ret);
+ goto out_hw;
+ }

wl1271_notice("initialized");

return 0;

- out_irq:
+out_hw:
+ wl1271_unregister_hw(wl);
+out_irq:
free_irq(wl->irq, wl);
-
-
- out_free:
+out_free:
wl1271_free_hw(wl);
-
return ret;
}

-static void __devexit wl1271_remove(struct sdio_func *func)
+static int __devexit wl1271_plat_remove(struct platform_device *pdev)
{
- struct wl1271 *wl = sdio_get_drvdata(func);
+ struct wl1271 *wl = platform_get_drvdata(pdev);

- free_irq(wl->irq, wl);
+ sdio_unregister_driver(&wl1271_sdio_driver);

wl1271_unregister_hw(wl);
+
+ free_irq(wl->irq, wl);
+
wl1271_free_hw(wl);
+
+ return 0;
}

-static struct sdio_driver wl1271_sdio_driver = {
- .name = "wl1271_sdio",
- .id_table = wl1271_devices,
- .probe = wl1271_probe,
- .remove = __devexit_p(wl1271_remove),
+static struct platform_driver wl1271_plat_driver = {
+ .probe = wl1271_plat_probe,
+ .remove = __devexit_p(wl1271_plat_remove),
+ .driver = {
+ .name = "wl1271_sdio",
+ .owner = THIS_MODULE,
+ },
};

static int __init wl1271_init(void)
{
int ret;

- ret = sdio_register_driver(&wl1271_sdio_driver);
+ ret = platform_driver_register(&wl1271_plat_driver);
if (ret < 0) {
- wl1271_error("failed to register sdio driver: %d", ret);
+ wl1271_error("failed to register plat driver: %d", ret);
goto out;
}

@@ -271,7 +382,7 @@ out:

static void __exit wl1271_exit(void)
{
- sdio_unregister_driver(&wl1271_sdio_driver);
+ platform_driver_unregister(&wl1271_plat_driver);

wl1271_notice("unloaded");
}
--
1.7.0.4


2010-07-06 00:39:55

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 03/15] omap: mmc: prepare for software card detect support

From: Ohad Ben-Cohen <[email protected]>

Make it possible for board files to give the
host controller two handlers needed for emulating
card detect events in software:

* The virtual_get_cd handler will allow the host
controller to query the status of the software
card detect (for the given controller/slot).

* The register_embedded_control will allow the host
controller to register handlers that will be used by
board devices to:
1. trigger a software card detect event
2. set private data (e.g. device context) that can later
be used by an sdio function driver

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-omap2/hsmmc.c | 4 ++++
arch/arm/mach-omap2/hsmmc.h | 5 +++++
arch/arm/plat-omap/include/plat/mmc.h | 5 +++++
3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/hsmmc.c b/arch/arm/mach-omap2/hsmmc.c
index 1ef54b0..1361128 100644
--- a/arch/arm/mach-omap2/hsmmc.c
+++ b/arch/arm/mach-omap2/hsmmc.c
@@ -269,6 +269,10 @@ void __init omap2_hsmmc_init(struct omap2_hsmmc_info *controllers)

mmc->slots[0].remux = c->remux;

+ mmc->slots[0].register_embedded_control =
+ c->register_embedded_control;
+ mmc->slots[0].virtual_get_cd = c->virtual_get_cd;
+
if (c->cover_only)
mmc->slots[0].cover = 1;

diff --git a/arch/arm/mach-omap2/hsmmc.h b/arch/arm/mach-omap2/hsmmc.h
index 36f0ba8..dc12c90 100644
--- a/arch/arm/mach-omap2/hsmmc.h
+++ b/arch/arm/mach-omap2/hsmmc.h
@@ -23,6 +23,11 @@ struct omap2_hsmmc_info {
int ocr_mask; /* temporary HACK */
/* Remux (pad configuation) when powering on/off */
void (*remux)(struct device *dev, int slot, int power_on);
+ /* following methods are for embedded sdio devices, e.g. wl1271 */
+ int (*register_embedded_control)(void *dev_id,
+ void (*set_virtual_cd)(void *dev_id, int card_present),
+ void (*set_embedded_data)(void *dev_id, void *priv));
+ int (*virtual_get_cd)(void);
};

#if defined(CONFIG_MMC_OMAP_HS) || defined(CONFIG_MMC_OMAP_HS_MODULE)
diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
index c835f1e..0f5bf12 100644
--- a/arch/arm/plat-omap/include/plat/mmc.h
+++ b/arch/arm/plat-omap/include/plat/mmc.h
@@ -140,6 +140,11 @@ struct omap_mmc_platform_data {

unsigned int ban_openended:1;

+ /* Embedded SDIO devices control */
+ int (*register_embedded_control)(void *dev_id,
+ void (*set_virtual_cd)(void *dev_id, int card_present),
+ void (*set_embedded_data)(void *dev_id, void *priv));
+ int (*virtual_get_cd)(void);
} slots[OMAP_MMC_MAX_SLOTS];
};

--
1.7.0.4


2010-07-07 13:52:30

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Wed, 7 Jul 2010, Roger Quadros wrote:

> On 07/06/2010 08:42 PM, ext Nicolas Pitre wrote:
> > On Tue, 6 Jul 2010, Roger Quadros wrote:
> >
> > > OK, this is how I see it.
> > >
> > > - Treat the non-removable card as non-removable. So no need to do card
> > > detect
> > > emulation.
> > >
> > > - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed
> > > regulator
> > > framework to define this regulator& supply. Even though you mention that
> > > it
> > > is not actually a supply, it fits well in the fixed supply framework.
> > >
> > > - When the host controller is enumerated, the mmc core will power up the
> > > slot,
> > > find the sdio card, and probe the function driver (i.e. wl1271_sdio).
> > >
> > > - if interface is not in use, the function driver must release the sdio
> > > host,
> > > and this should eventually disable the vmmc supply.
> > >
> > > - Whenever the wlan interface must be brought up, wl1271_sdio, can claim
> > > the
> > > sdio host. this will cause the vmmc supply to be enabled, for as long as
> > > the
> > > interface is up.
> > >
> > > Does this address all issues?
> >
> > This is mostly all good, except that claiming/releasing the SDIO host is
> > about access to the bus. It must be claimed right before doing any IO,
> > and released right after that, even when the card is expected to remain
> > powered. This is not the proper place to hook power control.
>
> Agreed, but is it so that SDIO power may be removed between a host_release and
> claim? This appears so from omap_hsmmc host controller.

No, it is not because a host is not claimed that power should be
dropped. The host claim/release is meant to provide exclusive access to
the card that's all.

If the OMAP controller is dropping power to the card upon
host->disable() then it is wrong. AFAICS only the OMAP controller is
playing such games at the moment and I suspect the semantics might not
be all right. Shutting down the _controller_ when it is idle might be a
good thing, but not power to the _card_. Only the function driver might
know when it is fine to lose power.


Nicolas

2010-07-06 00:40:11

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 05/15] omap: hsmmc: add virtual card detect support

From: Ohad Ben-Cohen <[email protected]>

Add support for software emulation of card detect
events.

This is required for specific controllers
that are hard wired with embedded SDIO devices
(such as TI's wl1271 WLAN device).

Board-specific configuration is required to
enable this software card detect control.

Based on Android's EMBEDDED SDIO patch by
San Mehat <[email protected]>.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/mmc/host/omap_hsmmc.c | 37 ++++++++++++++++++++++++++++++++++++-
1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index b032828..60694df 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1208,6 +1208,38 @@ static void omap_hsmmc_detect(struct work_struct *work)
mmc_detect_change(host->mmc, (HZ * 50) / 1000);
}

+#ifdef CONFIG_MMC_EMBEDDED_SDIO
+static void omap_hsmmc_set_embedded_data(void *dev_id, void *priv)
+{
+ struct omap_hsmmc_host *host = dev_id;
+ struct mmc_host *mmc = host->mmc;
+
+ mmc_set_embedded_data(mmc, priv);
+}
+
+static void omap_hsmmc_virtual_detect(void *dev_id, int carddetect)
+{
+ struct omap_hsmmc_host *host = dev_id;
+
+ printk(KERN_DEBUG "%s: card detect %d\n", mmc_hostname(host->mmc),
+ carddetect);
+
+ sysfs_notify(&host->mmc->class_dev.kobj, NULL, "cover_switch");
+
+ if (carddetect)
+ mmc_detect_change(host->mmc, (HZ * 200) / 1000);
+ else
+ mmc_detect_change(host->mmc, (HZ * 50) / 1000);
+}
+#else
+static void omap_hsmmc_set_embedded_data(void *dev_id, void *priv)
+{
+}
+static void omap_hsmmc_virtual_detect(void *dev_id, int carddetect)
+{
+}
+#endif
+
/*
* ISR for handling card insertion and removal
*/
@@ -2170,7 +2202,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
"Unable to grab MMC CD IRQ\n");
goto err_irq_cd;
}
- }
+ } else if (mmc_slot(host).register_embedded_control)
+ mmc_slot(host).register_embedded_control(host,
+ omap_hsmmc_virtual_detect,
+ omap_hsmmc_set_embedded_data);

omap_hsmmc_disable_irq(host);

--
1.7.0.4


2010-07-06 19:51:52

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Nicolas Pitre wrote:
> On Tue, 6 Jul 2010, Roger Quadros wrote:
>
>> On 07/06/2010 03:53 PM, ext Ohad Ben-Cohen wrote:
>>> Hi Roger,
>>>
>>> On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros<[email protected]>
>>> wrote:
>>>> My point is that shouldn't this be handled by SDIO core?
>>> Care to explain what you mean / give a code example ?
>> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
>> the SDIO/MMC core should tackle it, just like it deals with supply for slots
>> with removable cards.
>
> Exact.
>
>>> You need card detect events in order to trigger card& sdio function
>>> initialization and removals.
>
> Why would you trigger function initialization and removal? Just to turn
> off power? That's a bit like pulling off the battery from your laptop
> when you want to suspend it. There is a better way to go about things.
>
>>> Please share any alternative approach you may be thinking on.
>> OK, this is how I see it.
>>
>> - Treat the non-removable card as non-removable. So no need to do card detect
>> emulation.
>>
>> - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed regulator
>> framework to define this regulator & supply. Even though you mention that it
>> is not actually a supply, it fits well in the fixed supply framework.
>>
>> - When the host controller is enumerated, the mmc core will power up the slot,
>> find the sdio card, and probe the function driver (i.e. wl1271_sdio).
>>
>> - if interface is not in use, the function driver must release the sdio host,
>> and this should eventually disable the vmmc supply.
>>
>> - Whenever the wlan interface must be brought up, wl1271_sdio, can claim the
>> sdio host. this will cause the vmmc supply to be enabled, for as long as the
>> interface is up.
>>
>> Does this address all issues?
>
> This is mostly all good, except that claiming/releasing the SDIO host is
> about access to the bus. It must be claimed right before doing any IO,
> and released right after that, even when the card is expected to remain
> powered. This is not the proper place to hook power control.
>
> Another function pair would be needed instead, which would do almost
> like the suspend/resume code is already doing. Something like:
>
> /*
> * Indicate to the core SDIO layer that we're not requiring that the
> * function remain powered. If all functions for the card are in the
> * same "no power" state, then the host controller can remove power from
> * the card. Note: the function driver must preserve hardware states if
> * necessary.
> */
> int sdio_release_power(struct sdio_func *func);
>
> /*
> * Indicate to the core SDIO layer that we want power back for this
> * SDIO function. The power may or may not actually have been removed
> * since last call to sdio_release_power(), so the function driver must
> * not assume any preserved state at the hardware level and re-perform
> * all the necessary hardware config. This function returns 0 when
> * power is actually restored, or some error code if this cannot be
> * achieved. One error reason might be that the card is no longer
> * available on the bus (was removed while powered down and card
> * detection didn't trigger yet).
> */
> int sdio_claim_power(struct sdio_func *func);
>
> That's it. When the network interface is down and the hardware is not
> needed, you call sdio_release_power(). When the request to activate the
> network interface is received, you call sdio_claim_power() and configure
> the hardware appropriately. If sdio_claim_power() returns an error,
> then you just return an error to the network request, and eventually the
> driver's remove method will be called if this is indeed because the card
> was removed.
>
> In the core SDIO code, this is almost identical to a suspend/resume
> request, except that the request comes from the function driver instead
> of the core MMC code.

For eMMC in omap_hsmmc, this is all done via claim_host / release_host
which call ->enable() / ->disable() methods. omap_hsmmc makes use of
mmc_power_restore_host() which calls host->bus_ops->power_restore()
which is not implemented for SDIO, but for MMC and SD it reinitializes
the card.

Set omap2_hsmmc_info mmc[x] {.nonremovable=true, .power_saving=true} and
implement host->bus_ops->power_restore() for SDIO, then the power will
go off 9 seconds after sdio_release_host() is called. Then tweak omap_hsmmc
so that it doesn't wait 9 seconds for the SDIO case

>
>
> Nicolas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2010-07-08 04:34:33

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Wed, 7 Jul 2010, Adrian Hunter wrote:

> Nicolas Pitre wrote:
> > On Wed, 7 Jul 2010, Roger Quadros wrote:
> >
> > > On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
> > > > For eMMC in omap_hsmmc, this is all done via claim_host / release_host
> > > > which call ->enable() / ->disable() methods. omap_hsmmc makes use of
> > > > mmc_power_restore_host() which calls host->bus_ops->power_restore()
> > > > which is not implemented for SDIO, but for MMC and SD it reinitializes
> > > > the card.
> >
> > This is IMHO a really bad design. The power control decision has to come
> > from the top, not from the bottom. And certainly not with a U-turn
> > dependency the omap_hsmmc is using.
>
> The power control decision does come from the top via mmc_claim_host /
> mmc_release_host.

NO!!! THIS IS NOT ABOUT POWER!

To the risk of repeating myself, mmc_claim_host and mmc_release_host are
about getting exclusive access to the host controller. This should not
have any relationship with power. If anything, you might infer some
idleness of the host through that, but only to save power at the host
controller level, but not at the card level.

> > I regret to say this, but the omap_hsmmc driver is becoming a total mess.
> > The host controller driver has to be a dumb interface serving requests from
> > the hardware used by the upper layer stack, not the place where decisions
> > such as power handling should be made. Think of it like an ethernet driver.
> > No ethernet driver in Linux is telling the IP stack when to shut down.
>
> The omap_hsmmc driver does not tell the core to shut down the upper layers.

What is this call to mmc_power_save_host() and mmc_power_restore_host()
then?

> Instead the core tells the omap_hsmmc driver that it is "disabled" i.e.
> not currently being used so it can start taking steps to save power.

That's not how I see things implemented right now.

> That is sensible because not even the upper layers know when the next
> activity will be.

I don't agree with you. The upper layer, even if it cannot predict
exactly when the next activity will occur, still has a hell of a better
visibility on what is going on. In the context of an MMC/SD card,
the upper layer knows about the block subsystem and it can look for
dirty blocks that might be flushed in a near future. In the context of
a SDIO card, it is the SDIO function driver that knows when it is
important to preserve power to the card and when it is not.

All the host controller driver must do is to simply and dumbly process
requests from the core MMC code, including power control requests.

> > > Shouldn't the power control intelligence (i.e. when to turn power ON/OFF)
> > > lie
> > > with the bus drivers?
> >
> > Absolutely! And in the SDIO case that should lie with each function
> > drivers. Please let's stop this omap_hsmmc madness.
>
> You are dealing with a trivial case - turn off the power when not in use.

And apparently this cannot be implemented sanely on OMAP without faking
a card removal.

> We have 3 power saving actions: enable DPS, put the card to sleep,
> and finally power it off. Each increases the latency to start up
> again and so must be staggered. With DPS-enabled the host controller can
> be powered off at any time. In that case the controller registers must be
> restored.

You could implement the first one based on some idle period. The core
code probably doesn't need to know as this should be transparent to the
upper layer. But the other two definitely should be handled by the core
code.

> There are numerous entry points to the driver. Checking whether
> to restore registers at every entry point is error prone and messy.

Please give me something else than this lame excuse. There is
effectively only _one_ main entry point, namely the request method of
the mmc_host_ops structure. You might need to poke at the hardware when
the set_ios or the enable_sdio_irq methods are called, and if the
controller is latent then you'd only have to update the register cache.
This is certainly not what I would call numerous.

> Instead we require that the core tells the driver when to enable and
> disable.

No you don't. You are abusing the mmc_claim_host interface with power
management issues. Those power issues are then guessed in the host
controller driver, and then it eventually calls back into the core to
tell the upper layer to shut off.

What I'm telling you is that:

1) If you want to save power after some idle period you don't need
host->ops->enable and host->ops->disable at all. Simply keep a timer
alive in your host driver and reset it when a new request comes in.
The code for mmc_host_enable() looks rather redundant, and fishy to
me with its flag to prevent recursion, so this all could be removed.

2) Putting the card to sleep and/or removing power to it must be
completely managed by the core code and certainly not from the host
controller driver. The fact that mmc_power_save_host() is currently
called from a host driver and not from the core code or function
drivers is completely backward. Contrary to the suspend/resume
requests which come from the machine bus where the host controller is
attached (yes, this concept applies to you even if you don't have
PCI), the possibility for shutting down power to the card when not in
use must come from the so called "MMC bus driver" like the MMC
block driver, or the SDIO function driver. Putting a SDIO card to
sleep is card dependent anyway, and that knowledge has to live in the
SDIO function driver for that card, and certainly not in the host
driver nor in the platform code.

3) And yet that does _not_ require any new capability or method to be
implemented in the host controller driver as mmc_set_ios() can do
power handling already.

And incidentally, implementing 3 and 4 would suddenly work for all hosts
without any change to the existing host controller drivers and prevent a
whole lot of functionality duplications.


Nicolas

2010-07-06 08:55:39

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi,

On 07/06/2010 03:37 AM, ext Ohad Ben-Cohen wrote:
> From: Ohad Ben-Cohen<[email protected]>
>
> Introduce a platform device support which is decoupled
> from the life cycle of the sdio function.
>
> The platform device objective is to deliver board-specific
> values (like irq, ref_clock, and software card-detect
> emulation control).
>
> The driver can then dynamically create (or destroy)
> the sdio function whenever the wlan interface is
> brought up (or down), as part of the power() operation.

Is this the right approach? Why should you emulate a card disconnect event when
the wlan interface is brought down?

Physically the card is never disconnected. So I don't see why we should fake it.
Could you please explain why you need to do this?

br,
-roger

2010-07-06 00:40:25

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 08/15] wireless: wl1271: make wl12xx.h common to both spi and sdio

From: Ohad Ben-Cohen <[email protected]>

Move wl12xx.h outside of the spi-specific location,
so it can be shared with both spi and sdio solutions.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1251_sdio.c | 2 +-
drivers/net/wireless/wl12xx/wl1251_spi.c | 2 +-
drivers/net/wireless/wl12xx/wl1271_spi.c | 2 +-
include/linux/spi/wl12xx.h | 34 -----------------------------
include/linux/wl12xx.h | 34 +++++++++++++++++++++++++++++
5 files changed, 37 insertions(+), 37 deletions(-)
delete mode 100644 include/linux/spi/wl12xx.h
create mode 100644 include/linux/wl12xx.h

diff --git a/drivers/net/wireless/wl12xx/wl1251_sdio.c b/drivers/net/wireless/wl12xx/wl1251_sdio.c
index c561332..416d5aa 100644
--- a/drivers/net/wireless/wl12xx/wl1251_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1251_sdio.c
@@ -24,7 +24,7 @@
#include <linux/mmc/sdio_func.h>
#include <linux/mmc/sdio_ids.h>
#include <linux/platform_device.h>
-#include <linux/spi/wl12xx.h>
+#include <linux/wl12xx.h>
#include <linux/irq.h>

#include "wl1251.h"
diff --git a/drivers/net/wireless/wl12xx/wl1251_spi.c b/drivers/net/wireless/wl12xx/wl1251_spi.c
index e814742..4847b6a 100644
--- a/drivers/net/wireless/wl12xx/wl1251_spi.c
+++ b/drivers/net/wireless/wl12xx/wl1251_spi.c
@@ -26,7 +26,7 @@
#include <linux/slab.h>
#include <linux/crc7.h>
#include <linux/spi/spi.h>
-#include <linux/spi/wl12xx.h>
+#include <linux/wl12xx.h>

#include "wl1251.h"
#include "wl1251_reg.h"
diff --git a/drivers/net/wireless/wl12xx/wl1271_spi.c b/drivers/net/wireless/wl12xx/wl1271_spi.c
index 5189b81..e866049 100644
--- a/drivers/net/wireless/wl12xx/wl1271_spi.c
+++ b/drivers/net/wireless/wl12xx/wl1271_spi.c
@@ -25,7 +25,7 @@
#include <linux/module.h>
#include <linux/crc7.h>
#include <linux/spi/spi.h>
-#include <linux/spi/wl12xx.h>
+#include <linux/wl12xx.h>
#include <linux/slab.h>

#include "wl1271.h"
diff --git a/include/linux/spi/wl12xx.h b/include/linux/spi/wl12xx.h
deleted file mode 100644
index a223ecb..0000000
--- a/include/linux/spi/wl12xx.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * This file is part of wl12xx
- *
- * Copyright (C) 2009 Nokia Corporation
- *
- * Contact: Kalle Valo <[email protected]>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
- *
- */
-
-#ifndef _LINUX_SPI_WL12XX_H
-#define _LINUX_SPI_WL12XX_H
-
-struct wl12xx_platform_data {
- void (*set_power)(bool enable);
- /* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
- int irq;
- bool use_eeprom;
-};
-
-#endif
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
new file mode 100644
index 0000000..137ac89
--- /dev/null
+++ b/include/linux/wl12xx.h
@@ -0,0 +1,34 @@
+/*
+ * This file is part of wl12xx
+ *
+ * Copyright (C) 2009 Nokia Corporation
+ *
+ * Contact: Kalle Valo <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef _LINUX_WL12XX_H
+#define _LINUX_WL12XX_H
+
+struct wl12xx_platform_data {
+ void (*set_power)(bool enable);
+ /* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
+ int irq;
+ bool use_eeprom;
+};
+
+#endif
--
1.7.0.4


2010-07-06 12:03:11

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On Tue, Jul 6, 2010 at 2:02 PM, Roger Quadros <[email protected]> wrote:
> On 07/06/2010 01:22 PM, ext Ohad Ben-Cohen wrote:
>> We prefer not to power up the chip as early as boot time; instead, we
>
> Why?

Let's say you boot your device but never use WLAN.
In this scenario, we prefer the wl1271 device to stay powered off to
minimize power consumption.
This way we will power on the wl1271 device only when the user
actually turns WLAN on.

> The function driver should power it down later if required. no?

Care to explain this ? I'm not sure I'm following: the sdio function
is added and the sdio driver is probed only after you power on the
wl1271 device and let the mmc layer initialize and configure it. Why
would you want to do that if WLAN is disabled (i.e. user didn't turn
wlan on) ?

> If it was powered OFF then how did the wlan interface come into picture?

As soon as you insmod wl1271 and wl1271_sdio, you will have a wlan
interface (which is still down). At this point the wl1271 device is
still powered off and not consuming power.

If the user chooses to enable wlan (i.e. ifconfig wlan0 up), the chip
is powered on, it is initialized and configured by the mmc layer, and
then it can be used for WLAN activities.

> I agree, we some how need to power down the card when not in use in the
> right way.

In this patchset proposal, as soon as the user disables wlan (i.e.
ifconfig wlan0 down), the wl1271 device is powered off, and the
corresponding sdio function is removed.


Thanks,
Ohad.

2010-07-06 00:39:50

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 02/15] wireless: wl1271: remove SDIO IDs from driver

From: Ohad Ben-Cohen <[email protected]>

Remove SDIO IDs from the driver code since now it is
included in linux/mmc/sdio_ids.h.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271_sdio.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index d3d6f30..9903ae9 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -37,14 +37,6 @@

#define RX71_WL1271_IRQ_GPIO 42

-#ifndef SDIO_VENDOR_ID_TI
-#define SDIO_VENDOR_ID_TI 0x0097
-#endif
-
-#ifndef SDIO_DEVICE_ID_TI_WL1271
-#define SDIO_DEVICE_ID_TI_WL1271 0x4076
-#endif
-
static const struct sdio_device_id wl1271_devices[] = {
{ SDIO_DEVICE(SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271) },
{}
--
1.7.0.4


2010-07-08 09:46:05

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 06/15] omap zoom2: wlan board muxing

* Ghorai, Sukumar <[email protected]> [100708 06:34]:
> > @@ -71,6 +71,21 @@ static struct twl4030_platform_data zoom2_twldata = {
> >
> > #ifdef CONFIG_OMAP_MUX
> > static struct omap_board_mux board_mux[] __initdata = {
> > +#ifdef CONFIG_OMAP_ZOOM_WLAN
>
> [Ghorai] This is zoom board specific file, So why need this additional flag?

Good point, the ifdef is unnecessary in for both zoom2 and zoom3.
Will update in my queue to remove the ifdefs.

Tony

2010-07-08 20:14:15

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 15/15] omap: zoom: enable WLAN device

On Thu, Jul 8, 2010 at 6:39 AM, Ghorai, Sukumar <[email protected]> wrote:
> [Ghorai] In general we can avoid OMAP_ZOOM_WLAN and MMC_EMBEDDED_SDIO as kconfig option. 1st one is board specific and 2nd one could be generic sdio code. As I mentioned in other patch too.


Thanks, will get rid of the extra config options.


>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2010-07-06 15:34:43

by Madhusudhan

[permalink] [raw]
Subject: RE: [PATCH 05/15] omap: hsmmc: add virtual card detect support



> -----Original Message-----
> From: Ohad Ben-Cohen [mailto:[email protected]]
> Sent: Tuesday, July 06, 2010 6:48 AM
> To: Nicolas Pitre
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; Chikkature Rajashekar Madhusudhan; Luciano Coelho;
> Andrew Morton; San Mehat; Pandita, Vikram
> Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support
>
> On Tue, Jul 6, 2010 at 1:22 PM, Ohad Ben-Cohen <[email protected]> wrote:
> > Note: the wl1271 device does support standard card detection, but
> > AFAIK there's a limitation to use that with the specific omap
> > controller the device is hardwired to. I will try to get more info
> > about that, but probably Madhu can comment on that better.
>
>
> Some correction and additional info:
>
> The wl1271 device has an issue which makes the standard card detect
> mechanism irrelevant: it is always up, even if the power enable gpio
> input of the device is down (the power enable input does not supply
> the power to the chip, it's just logical digital high/low input upon
> which the device reacts). That's why we must use software control for
> emulating card detect with that device.
>
> In addition, as far as I could find out, the card detect mechanism on
> the ZOOM is implemented by mechanical means, and thus is not relevant
> for hardwired embedded SDIO devices (I'm not even sure card detect is
> supported for the 3rd mmc controller).

The card detect is supported through T2 GPIO interrupts only for MMC1 and
MMC2. Such a mechanism is not present for MMC3 to which the WLAN chip is
hardwired.

Regards,
Madhu


2010-07-06 00:40:57

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 13/15] wireless: wl1271: make ref_clock configurable by board

From: Ohad Ben-Cohen <[email protected]>

The wl1271 device is using a reference clock that may change
between board to board.

Make the ref_clock parameter configurable by the board
files that set up the device, instead of having a hard coded
value in the driver source itself.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271.h | 1 +
drivers/net/wireless/wl12xx/wl1271_boot.c | 13 +++++++++----
drivers/net/wireless/wl12xx/wl1271_boot.h | 1 -
drivers/net/wireless/wl12xx/wl1271_sdio.c | 2 ++
drivers/net/wireless/wl12xx/wl1271_spi.c | 2 ++
include/linux/wl12xx.h | 1 +
6 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 2df57cc..cb3126d 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -362,6 +362,7 @@ struct wl1271 {
void (*set_embedded_data)(void *priv);

int irq;
+ int ref_clock;

spinlock_t wl_lock;

diff --git a/drivers/net/wireless/wl12xx/wl1271_boot.c b/drivers/net/wireless/wl12xx/wl1271_boot.c
index 1a36d8a..fa1b8b9 100644
--- a/drivers/net/wireless/wl12xx/wl1271_boot.c
+++ b/drivers/net/wireless/wl12xx/wl1271_boot.c
@@ -455,17 +455,22 @@ int wl1271_boot(struct wl1271 *wl)
{
int ret = 0;
u32 tmp, clk, pause;
+ int ref_clock = wl->ref_clock;

wl1271_boot_hw_version(wl);

- if (REF_CLOCK == 0 || REF_CLOCK == 2 || REF_CLOCK == 4)
+ if (ref_clock == 0 || ref_clock == 2 || ref_clock == 4)
/* ref clk: 19.2/38.4/38.4-XTAL */
clk = 0x3;
- else if (REF_CLOCK == 1 || REF_CLOCK == 3)
+ else if (ref_clock == 1 || ref_clock == 3)
/* ref clk: 26/52 */
clk = 0x5;
+ else {
+ ret = -EINVAL;
+ goto out;
+ }

- if (REF_CLOCK != 0) {
+ if (ref_clock != 0) {
u16 val;
/* Set clock type (open drain) */
val = wl1271_top_reg_read(wl, OCP_REG_CLK_TYPE);
@@ -514,7 +519,7 @@ int wl1271_boot(struct wl1271 *wl)
wl1271_debug(DEBUG_BOOT, "clk2 0x%x", clk);

/* 2 */
- clk |= (REF_CLOCK << 1) << 4;
+ clk |= (ref_clock << 1) << 4;
wl1271_write32(wl, DRPW_SCRATCH_START, clk);

wl1271_set_partition(wl, &part_table[PART_WORK]);
diff --git a/drivers/net/wireless/wl12xx/wl1271_boot.h b/drivers/net/wireless/wl12xx/wl1271_boot.h
index f829699..f73b0b1 100644
--- a/drivers/net/wireless/wl12xx/wl1271_boot.h
+++ b/drivers/net/wireless/wl12xx/wl1271_boot.h
@@ -46,7 +46,6 @@ struct wl1271_static_data {
/* delay between retries */
#define INIT_LOOP_DELAY 50

-#define REF_CLOCK 2
#define WU_COUNTER_PAUSE_VAL 0x3FF
#define WELP_ARM_COMMAND_VAL 0x4

diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 0af0dce..4ed097f 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -270,6 +270,8 @@ static int wl1271_plat_probe(struct platform_device *pdev)
wl->if_plat_priv = pdev;
platform_set_drvdata(pdev, wl);

+ wl->ref_clock = pdata->board_ref_clock;
+
wl->set_embedded_data = pdata->set_embedded_data;
if (!wl->set_embedded_data) {
wl1271_error("set embedded_data func missing in platform data");
diff --git a/drivers/net/wireless/wl12xx/wl1271_spi.c b/drivers/net/wireless/wl12xx/wl1271_spi.c
index 85a167f..2cb125b 100644
--- a/drivers/net/wireless/wl12xx/wl1271_spi.c
+++ b/drivers/net/wireless/wl12xx/wl1271_spi.c
@@ -373,6 +373,8 @@ static int __devinit wl1271_probe(struct spi_device *spi)
goto out_free;
}

+ wl->ref_clock = pdata->ref_clock;
+
wl->irq = spi->irq;
if (wl->irq < 0) {
wl1271_error("irq missing in platform data");
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index 4106184..9053d9c 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -31,6 +31,7 @@ struct wl12xx_platform_data {
/* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
int irq;
bool use_eeprom;
+ int board_ref_clock;
};

#endif
--
1.7.0.4


2010-07-06 15:49:26

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH 04/15] mmc: support embedded data field in mmc_host

On Tue, Jul 6, 2010 at 3:37 AM, Ohad Ben-Cohen <[email protected]> wrote:
> From: Ohad Ben-Cohen <[email protected]>
>
> Add support to set/get mmc_host private embedded
> data.
>
> This is needed to allow software to dynamically
> create (and remove) SDIO functions which represents
> embedded SDIO devices.
>
> Typically, it will be used to set the context of
> a driver that is creating a new SDIO function
> (and would then expect to be able to get that context
> back upon creation of the new sdio func).
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> ?drivers/mmc/core/Kconfig | ? ?8 ++++++++
> ?include/linux/mmc/host.h | ? 16 ++++++++++++++++
> ?2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index bb22ffd..ab27eb3 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -16,3 +16,11 @@ config MMC_UNSAFE_RESUME
>
> ? ? ? ? ?This option sets a default which can be overridden by the
> ? ? ? ? ?module parameter "removable=0" or "removable=1".
> +
> +config MMC_EMBEDDED_SDIO
> + ? ? ? boolean "MMC embedded SDIO device support"
> + ? ? ? help
> + ? ? ? ? If you say Y here, support will be added for embedded SDIO
> + ? ? ? ? devices (e.g. hardwired embedded WLAN SDIO devices).
> + ? ? ? ? Such devices require software support for emulating
> + ? ? ? ? card detect events.
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f65913c..9a48486 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -209,6 +209,10 @@ struct mmc_host {
> ? ? ? ?struct led_trigger ? ? ?*led; ? ? ? ? ? /* activity led */
> ?#endif
>
> +#ifdef CONFIG_MMC_EMBEDDED_SDIO
> + ? ? ? void ? ? ? ? ? ? ? ? ? ?*embedded_data;
> +#endif
> +

Hm, do we really need a Kconfig option just for a single pointer? It
only saves sizeof(void *) bytes per host, but adds rather confusing
config option for users and some ifdef complexity.

2010-07-06 00:40:37

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 09/15] wireless: wl12xx: support pdata SDIO handlers

From: Ohad Ben-Cohen <[email protected]>

Make it possible for board code to send SDIO handlers to
the wl12xx driver.

These SDIO handlers are needed to allow software to trigger
card detect events for hard-wired SDIO devices such as the
wl1271 device.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271.h | 4 ++++
include/linux/wl12xx.h | 2 ++
2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 6f1b6b5..fc4d122 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -356,6 +356,10 @@ struct wl1271 {
struct wl1271_if_operations *if_ops;

void (*set_power)(bool enable);
+ /* SDIO card detect emulation */
+ void (*set_carddetect)(bool card_present);
+ void (*set_embedded_data)(void *priv);
+
int irq;

spinlock_t wl_lock;
diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
index 137ac89..4106184 100644
--- a/include/linux/wl12xx.h
+++ b/include/linux/wl12xx.h
@@ -26,6 +26,8 @@

struct wl12xx_platform_data {
void (*set_power)(bool enable);
+ void (*set_carddetect)(bool card_present);
+ void (*set_embedded_data)(void *priv);
/* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
int irq;
bool use_eeprom;
--
1.7.0.4


2010-07-06 12:53:23

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Roger,

On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros <[email protected]> wrote:
> My point is that shouldn't this be handled by SDIO core?

Care to explain what you mean / give a code example ?

> If there are no users for the SDIO function and the card, doesn't the SDIO
> core power down the slot and take care of re-initialization when it is
> powered up?

You need card detect events in order to trigger card & sdio function
initialization and removals.

Please share any alternative approach you may be thinking on.

Thanks,
Ohad.


>

2010-07-07 07:48:46

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On 07/06/2010 08:42 PM, ext Nicolas Pitre wrote:
> On Tue, 6 Jul 2010, Roger Quadros wrote:
>
>> On 07/06/2010 03:53 PM, ext Ohad Ben-Cohen wrote:
>>> Hi Roger,
>>>
>>> On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros<[email protected]>
>>> wrote:
>>>> My point is that shouldn't this be handled by SDIO core?
>>>
>>> Care to explain what you mean / give a code example ?
>>
>> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
>> the SDIO/MMC core should tackle it, just like it deals with supply for slots
>> with removable cards.
>
> Exact.
>
>>> You need card detect events in order to trigger card& sdio function
>>> initialization and removals.
>
> Why would you trigger function initialization and removal? Just to turn
> off power? That's a bit like pulling off the battery from your laptop
> when you want to suspend it. There is a better way to go about things.
>
>>> Please share any alternative approach you may be thinking on.
>>
>> OK, this is how I see it.
>>
>> - Treat the non-removable card as non-removable. So no need to do card detect
>> emulation.
>>
>> - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed regulator
>> framework to define this regulator& supply. Even though you mention that it
>> is not actually a supply, it fits well in the fixed supply framework.
>>
>> - When the host controller is enumerated, the mmc core will power up the slot,
>> find the sdio card, and probe the function driver (i.e. wl1271_sdio).
>>
>> - if interface is not in use, the function driver must release the sdio host,
>> and this should eventually disable the vmmc supply.
>>
>> - Whenever the wlan interface must be brought up, wl1271_sdio, can claim the
>> sdio host. this will cause the vmmc supply to be enabled, for as long as the
>> interface is up.
>>
>> Does this address all issues?
>
> This is mostly all good, except that claiming/releasing the SDIO host is
> about access to the bus. It must be claimed right before doing any IO,
> and released right after that, even when the card is expected to remain
> powered. This is not the proper place to hook power control.

Agreed, but is it so that SDIO power may be removed between a host_release and
claim? This appears so from omap_hsmmc host controller.

if we have sdio_claim_power() and sdio_release_power() in place then power
control should depend on it.

regards,
-roger

2010-07-09 09:24:08

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Fri, Jul 9, 2010 at 11:32 AM, Ohad Ben-Cohen <[email protected]> wrote:
> Hi Roger,
>
> On Fri, Jul 9, 2010 at 11:12 AM, Roger Quadros <[email protected]> wrote:
>> You are already
>> creating a new platform device for wl1271_sdio just to pass platform_data
>
> With the new approach, I can remove this platform device altogether,
> and completely rely on the sdio function device for driver probing (as
> it was before). I just need to add this little support of delivering
> small bits of platform data - the mechanism should be simple and
> generic, and would considerably simplify the driver.
>
>> I think this case is specific to wl1271.
>
> Yes, it is.

wl1251 also has this issue. In fact it's even worse there as it's
missing all SDIO registers and cannot be probed at all.. Those devices
are not plug n play at all.

2010-07-07 14:02:43

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Wed, 7 Jul 2010, Roger Quadros wrote:

> On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
> > For eMMC in omap_hsmmc, this is all done via claim_host / release_host
> > which call ->enable() / ->disable() methods. omap_hsmmc makes use of
> > mmc_power_restore_host() which calls host->bus_ops->power_restore()
> > which is not implemented for SDIO, but for MMC and SD it reinitializes
> > the card.

This is IMHO a really bad design. The power control decision has to
come from the top, not from the bottom. And certainly not with a
U-turn dependency the omap_hsmmc is using.

I regret to say this, but the omap_hsmmc driver is becoming a total
mess. The host controller driver has to be a dumb interface serving
requests from the hardware used by the upper layer stack, not the place
where decisions such as power handling should be made. Think of it like
an ethernet driver. No ethernet driver in Linux is telling the IP stack
when to shut down.

> Shouldn't the power control intelligence (i.e. when to turn power ON/OFF) lie
> with the bus drivers?

Absolutely! And in the SDIO case that should lie with each function
drivers. Please let's stop this omap_hsmmc madness.


Nicolas

2010-07-06 00:40:03

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 04/15] mmc: support embedded data field in mmc_host

From: Ohad Ben-Cohen <[email protected]>

Add support to set/get mmc_host private embedded
data.

This is needed to allow software to dynamically
create (and remove) SDIO functions which represents
embedded SDIO devices.

Typically, it will be used to set the context of
a driver that is creating a new SDIO function
(and would then expect to be able to get that context
back upon creation of the new sdio func).

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/mmc/core/Kconfig | 8 ++++++++
include/linux/mmc/host.h | 16 ++++++++++++++++
2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index bb22ffd..ab27eb3 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -16,3 +16,11 @@ config MMC_UNSAFE_RESUME

This option sets a default which can be overridden by the
module parameter "removable=0" or "removable=1".
+
+config MMC_EMBEDDED_SDIO
+ boolean "MMC embedded SDIO device support"
+ help
+ If you say Y here, support will be added for embedded SDIO
+ devices (e.g. hardwired embedded WLAN SDIO devices).
+ Such devices require software support for emulating
+ card detect events.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f65913c..9a48486 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -209,6 +209,10 @@ struct mmc_host {
struct led_trigger *led; /* activity led */
#endif

+#ifdef CONFIG_MMC_EMBEDDED_SDIO
+ void *embedded_data;
+#endif
+
struct dentry *debugfs_root;

unsigned long private[0] ____cacheline_aligned;
@@ -264,5 +268,17 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
host->disable_delay = disable_delay;
}

+#ifdef CONFIG_MMC_EMBEDDED_SDIO
+static inline void *mmc_get_embedded_data(struct mmc_host *host)
+{
+ return host->embedded_data;
+}
+
+static inline void mmc_set_embedded_data(struct mmc_host *host, void *data)
+{
+ host->embedded_data = data;
+}
+#endif
+
#endif

--
1.7.0.4


2010-07-06 12:39:41

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

Hi,

On 07/06/2010 02:48 PM, ext Ohad Ben-Cohen wrote:
> On Tue, Jul 6, 2010 at 1:22 PM, Ohad Ben-Cohen<[email protected]> wrote:
>> Note: the wl1271 device does support standard card detection, but
>> AFAIK there's a limitation to use that with the specific omap
>> controller the device is hardwired to. I will try to get more info
>> about that, but probably Madhu can comment on that better.
>
>
> Some correction and additional info:
>
> The wl1271 device has an issue which makes the standard card detect
> mechanism irrelevant: it is always up, even if the power enable gpio
> input of the device is down (the power enable input does not supply
> the power to the chip, it's just logical digital high/low input upon
> which the device reacts). That's why we must use software control for
> emulating card detect with that device.

Just to clarify, is the wl1271 device always powered on the board?

And this GPIO power enable (OMAP_ZOOM_WLAN_PMENA_GPIO) is used to gate this
supply internally?

and what do these do ?

set_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);

clear_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);

regards,
-roger

2010-07-07 14:55:18

by Madhusudhan

[permalink] [raw]
Subject: RE: [PATCH 11/15] wireless: wl1271: introduce platform device support



> -----Original Message-----
> From: Nicolas Pitre [mailto:[email protected]]
> Sent: Wednesday, July 07, 2010 9:03 AM
> To: Roger Quadros
> Cc: Hunter Adrian (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; Chikkature Rajashekar Madhusudhan; Coelho Luciano
> (Nokia-MS/Helsinki); [email protected]; San Mehat
> Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device
> support
>
> On Wed, 7 Jul 2010, Roger Quadros wrote:
>
> > On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
> > > For eMMC in omap_hsmmc, this is all done via claim_host / release_host
> > > which call ->enable() / ->disable() methods. omap_hsmmc makes use of
> > > mmc_power_restore_host() which calls host->bus_ops->power_restore()
> > > which is not implemented for SDIO, but for MMC and SD it reinitializes
> > > the card.
>
> This is IMHO a really bad design. The power control decision has to
> come from the top, not from the bottom. And certainly not with a
> U-turn dependency the omap_hsmmc is using.
>
> I regret to say this, but the omap_hsmmc driver is becoming a total
> mess. The host controller driver has to be a dumb interface serving
> requests from the hardware used by the upper layer stack, not the place
> where decisions such as power handling should be made. Think of it like
> an ethernet driver. No ethernet driver in Linux is telling the IP stack
> when to shut down.
>

The point is that MMC/SD core files were patched to provide this kind of a
support. Any controller driver can use that framework today, right?. As an
example omap_hsmmc driver was patched and it works fine.

Why blame the controller driver for using a support provided by core files?

Regards,
Madhu

> > Shouldn't the power control intelligence (i.e. when to turn power
> ON/OFF) lie
> > with the bus drivers?
>
> Absolutely! And in the SDIO case that should lie with each function
> drivers. Please let's stop this omap_hsmmc madness.
>
>
> Nicolas


2010-07-07 08:02:21

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
> Nicolas Pitre wrote:
>> On Tue, 6 Jul 2010, Roger Quadros wrote:
>>
>>> On 07/06/2010 03:53 PM, ext Ohad Ben-Cohen wrote:
>>>> Hi Roger,
>>>>
>>>> On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros<[email protected]>
>>>> wrote:
>>>>> My point is that shouldn't this be handled by SDIO core?
>>>> Care to explain what you mean / give a code example ?
>>> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
>>> the SDIO/MMC core should tackle it, just like it deals with supply for slots
>>> with removable cards.
>>
>> Exact.
>>
>>>> You need card detect events in order to trigger card& sdio function
>>>> initialization and removals.
>>
>> Why would you trigger function initialization and removal? Just to turn
>> off power? That's a bit like pulling off the battery from your laptop
>> when you want to suspend it. There is a better way to go about things.
>>
>>>> Please share any alternative approach you may be thinking on.
>>> OK, this is how I see it.
>>>
>>> - Treat the non-removable card as non-removable. So no need to do card detect
>>> emulation.
>>>
>>> - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed regulator
>>> framework to define this regulator& supply. Even though you mention that it
>>> is not actually a supply, it fits well in the fixed supply framework.
>>>
>>> - When the host controller is enumerated, the mmc core will power up the slot,
>>> find the sdio card, and probe the function driver (i.e. wl1271_sdio).
>>>
>>> - if interface is not in use, the function driver must release the sdio host,
>>> and this should eventually disable the vmmc supply.
>>>
>>> - Whenever the wlan interface must be brought up, wl1271_sdio, can claim the
>>> sdio host. this will cause the vmmc supply to be enabled, for as long as the
>>> interface is up.
>>>
>>> Does this address all issues?
>>
>> This is mostly all good, except that claiming/releasing the SDIO host is
>> about access to the bus. It must be claimed right before doing any IO,
>> and released right after that, even when the card is expected to remain
>> powered. This is not the proper place to hook power control.
>>
>> Another function pair would be needed instead, which would do almost
>> like the suspend/resume code is already doing. Something like:
>>
>> /*
>> * Indicate to the core SDIO layer that we're not requiring that the
>> * function remain powered. If all functions for the card are in the
>> * same "no power" state, then the host controller can remove power from
>> * the card. Note: the function driver must preserve hardware states if
>> * necessary.
>> */
>> int sdio_release_power(struct sdio_func *func);
>>
>> /*
>> * Indicate to the core SDIO layer that we want power back for this
>> * SDIO function. The power may or may not actually have been removed
>> * since last call to sdio_release_power(), so the function driver must
>> * not assume any preserved state at the hardware level and re-perform
>> * all the necessary hardware config. This function returns 0 when
>> * power is actually restored, or some error code if this cannot be
>> * achieved. One error reason might be that the card is no longer
>> * available on the bus (was removed while powered down and card
>> * detection didn't trigger yet).
>> */
>> int sdio_claim_power(struct sdio_func *func);
>>
>> That's it. When the network interface is down and the hardware is not
>> needed, you call sdio_release_power(). When the request to activate the
>> network interface is received, you call sdio_claim_power() and configure
>> the hardware appropriately. If sdio_claim_power() returns an error,
>> then you just return an error to the network request, and eventually the
>> driver's remove method will be called if this is indeed because the card
>> was removed.
>>
>> In the core SDIO code, this is almost identical to a suspend/resume
>> request, except that the request comes from the function driver instead
>> of the core MMC code.
>
> For eMMC in omap_hsmmc, this is all done via claim_host / release_host
> which call ->enable() / ->disable() methods. omap_hsmmc makes use of
> mmc_power_restore_host() which calls host->bus_ops->power_restore()
> which is not implemented for SDIO, but for MMC and SD it reinitializes
> the card.

Shouldn't the power control intelligence (i.e. when to turn power ON/OFF) lie
with the bus drivers?

>
> Set omap2_hsmmc_info mmc[x] {.nonremovable=true, .power_saving=true} and
> implement host->bus_ops->power_restore() for SDIO, then the power will
> go off 9 seconds after sdio_release_host() is called. Then tweak omap_hsmmc
> so that it doesn't wait 9 seconds for the SDIO case
>
is the wl1271 supposed to be used only with omap_hsmmc? We need to have a
solution that works neatly irrespective of which host controller is being used.

regards,
-roger

2010-07-08 03:39:48

by Ghorai, Sukumar

[permalink] [raw]
Subject: RE: [PATCH 06/15] omap zoom2: wlan board muxing



> -----Original Message-----
> From: [email protected] [mailto:linux-mmc-
> [email protected]] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, July 06, 2010 6:08 AM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: [email protected]; [email protected];
> Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm@linux-
> foundation.org; San Mehat; Ben-cohen, Ohad
> Subject: [PATCH 06/15] omap zoom2: wlan board muxing
>
> From: Ohad Ben-Cohen <[email protected]>
>
> Add board muxing to support the wlan wl1271 chip that is
> hardwired to mmc2 (third mmc controller) on the ZOOM2.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> arch/arm/mach-omap2/board-zoom2.c | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-
> omap2/board-zoom2.c
> index 803ef14..00871be 100644
> --- a/arch/arm/mach-omap2/board-zoom2.c
> +++ b/arch/arm/mach-omap2/board-zoom2.c
> @@ -71,6 +71,21 @@ static struct twl4030_platform_data zoom2_twldata = {
>
> #ifdef CONFIG_OMAP_MUX
> static struct omap_board_mux board_mux[] __initdata = {
> +#ifdef CONFIG_OMAP_ZOOM_WLAN
[Ghorai] This is zoom board specific file, So why need this additional flag?

> + /* WLAN IRQ - GPIO 162 */
> + OMAP3_MUX(MCBSP1_CLKX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP),
> + /* WLAN POWER ENABLE - GPIO 101 */
> + OMAP3_MUX(CAM_D2, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
> + /* WLAN SDIO: MMC3 CMD */
> + OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE3 | OMAP_PIN_INPUT_PULLUP),
> + /* WLAN SDIO: MMC3 CLK */
> + OMAP3_MUX(ETK_CLK, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
> + /* WLAN SDIO: MMC3 DAT[0-3] */
> + OMAP3_MUX(ETK_D3, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
> + OMAP3_MUX(ETK_D4, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
> + OMAP3_MUX(ETK_D5, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
> + OMAP3_MUX(ETK_D6, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
> +#endif
> { .reg_offset = OMAP_MUX_TERMINATOR },
> };
> #else
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-07-06 00:39:42

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 01/15] sdio: add TI + wl1271 ids

From: Ohad Ben-Cohen <[email protected]>

Add SDIO IDs for TI and for TI's wl1271 wlan device.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
include/linux/mmc/sdio_ids.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/mmc/sdio_ids.h b/include/linux/mmc/sdio_ids.h
index 33b2ea0..0d313c6 100644
--- a/include/linux/mmc/sdio_ids.h
+++ b/include/linux/mmc/sdio_ids.h
@@ -43,4 +43,7 @@
#define SDIO_DEVICE_ID_SIANO_NOVA_A0 0x1100
#define SDIO_DEVICE_ID_SIANO_STELLAR 0x5347

+#define SDIO_VENDOR_ID_TI 0x0097
+#define SDIO_DEVICE_ID_TI_WL1271 0x4076
+
#endif
--
1.7.0.4


2010-07-06 10:22:48

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

Hi Nicolas,

On Tue, Jul 6, 2010 at 4:45 AM, Nicolas Pitre <[email protected]> wrote:
> On Tue, 6 Jul 2010, Ohad Ben-Cohen wrote:
>
>> From: Ohad Ben-Cohen <[email protected]>
>>
>> Add support for software emulation of card detect
>> events.
>>
>> This is required for specific controllers
>> that are hard wired with embedded SDIO devices
>> (such as TI's wl1271 WLAN device).
>
> Why?
>
> Many instances of hardwired SDIO based WLAN devices do exist already,
> and they don't need extra card detect events to be generated. ?The core
> MMC code already triggers a probe for cards whenever a new host
> controller is registered.

We prefer not to power up the chip as early as boot time; instead, we
would like to have it powered off until the wlan interface is brought
up, power it on when the interface is brought up,
and power it off again as soon as the interface is brought down again
(to minimize power consumption when the wlan is not in use).

For that we can't rely on the probing done when the controller is
registered, we want to have a mechanism to allow dynamic detection and
removal of the card.

Note: the wl1271 device does support standard card detection, but
AFAIK there's a limitation to use that with the specific omap
controller the device is hardwired to. I will try to get more info
about that, but probably Madhu can comment on that better.

>> Board-specific configuration is required to
>> enable this software card detect control.
>
> Could you elaborate please?

Please check out the last patch - that patch is doing that configuration.
In essence, this virtual card detect mechanism is enabled only for
specific controllers which we know there's an embedded sdio device
hardwired to. This knowledge is board-specific, and that's why we
enable this mechanism in the board files, per a specific mmc
controller.

Thanks,
Ohad.


>
>
> Nicolas
>

2010-07-06 07:15:28

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 08/15] wireless: wl1271: make wl12xx.h common to both spi and sdio

On Tue, 2010-07-06 at 02:37 +0200, ext Ohad Ben-Cohen wrote:
> From: Ohad Ben-Cohen <[email protected]>
>
> Move wl12xx.h outside of the spi-specific location,
> so it can be shared with both spi and sdio solutions.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> drivers/net/wireless/wl12xx/wl1251_sdio.c | 2 +-
> drivers/net/wireless/wl12xx/wl1251_spi.c | 2 +-
> drivers/net/wireless/wl12xx/wl1271_spi.c | 2 +-
> include/linux/spi/wl12xx.h | 34
> -----------------------------
> include/linux/wl12xx.h | 34
> +++++++++++++++++++++++++++++
> 5 files changed, 37 insertions(+), 37 deletions(-)
> delete mode 100644 include/linux/spi/wl12xx.h
> create mode 100644 include/linux/wl12xx.h

For the wl1271 part:

Acked-by: Luciano Coelho <[email protected]>

For the wl12xx.h and wl1251 parts, this needs to be acked by Kalle Valo.


--
Cheers,
Luca.


2010-07-07 15:46:53

by Nicolas Pitre

[permalink] [raw]
Subject: RE: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Wed, 7 Jul 2010, Madhusudhan wrote:

>
>
> > -----Original Message-----
> > From: Nicolas Pitre [mailto:[email protected]]
> > Sent: Wednesday, July 07, 2010 9:03 AM
> > To: Roger Quadros
> > Cc: Hunter Adrian (Nokia-MS/Helsinki); Ohad Ben-Cohen; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; Chikkature Rajashekar Madhusudhan; Coelho Luciano
> > (Nokia-MS/Helsinki); [email protected]; San Mehat
> > Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device
> > support
> >
> > On Wed, 7 Jul 2010, Roger Quadros wrote:
> >
> > > On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
> > > > For eMMC in omap_hsmmc, this is all done via claim_host / release_host
> > > > which call ->enable() / ->disable() methods. omap_hsmmc makes use of
> > > > mmc_power_restore_host() which calls host->bus_ops->power_restore()
> > > > which is not implemented for SDIO, but for MMC and SD it reinitializes
> > > > the card.
> >
> > This is IMHO a really bad design. The power control decision has to
> > come from the top, not from the bottom. And certainly not with a
> > U-turn dependency the omap_hsmmc is using.
> >
> > I regret to say this, but the omap_hsmmc driver is becoming a total
> > mess. The host controller driver has to be a dumb interface serving
> > requests from the hardware used by the upper layer stack, not the place
> > where decisions such as power handling should be made. Think of it like
> > an ethernet driver. No ethernet driver in Linux is telling the IP stack
> > when to shut down.
> >
>
> The point is that MMC/SD core files were patched to provide this kind of a
> support. Any controller driver can use that framework today, right?. As an
> example omap_hsmmc driver was patched and it works fine.

It is not because you are twisting the layers and customizing the core
stack around your own controller that it is good software design.

And the presence of the mmc_power_restore_host() code doesn't mean it is
sane. My point is that no one should ever use that, not even
omap_hsmmc.

Proof: it works so fine that now you must come up with yet another
contorted hack piled on top called "fake^H^H^H^Hsoftware insert/remove
events" to support power handling with SDIO cards.

This MMC_CAP_DISABLE is ridiculous. Why would this have to be a host
capability? This should be a core feature that should be _transparent_
to all hosts with no changes to any of the host drivers. When the core
code knows that the card can be shut down after some idle period, it
should do so with the *existing* API calls, namely the set_ios method.
In the SDIO case it would be a simple matter of mapping the
sdio_release_power() onto that. Instead, some contorted power
management support was sneaked in, which is misdesigned to the point of
breaking proper SDIO support for which more misdesigned features are now
needed to work around the former ones.

Now the OMAP driver is becoming a stack of its own, making decisions
that clearly should be made at a higher level of abstraction. To me
this denotes some laziness from the involved developers who didn't take
the time to design something sensible and generic in the core code, but
rather hacked a quick solution specific to omap_hmmc.c. Of course the
former would require a greater understanding of common code and some
additional effort to make the solution truly generic. Instead, the easy
solution was taken which is to stuff magic behaviors in a host driver
while other people are not paying as much attention to it than core
code.

Needless to say that I'm not impressed at all.


Nicolas

2010-07-08 03:39:54

by Ghorai, Sukumar

[permalink] [raw]
Subject: RE: [PATCH 04/15] mmc: support embedded data field in mmc_host



> -----Original Message-----
> From: [email protected] [mailto:linux-mmc-
> [email protected]] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, July 06, 2010 6:08 AM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: [email protected]; [email protected];
> Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm@linux-
> foundation.org; San Mehat; Ben-cohen, Ohad
> Subject: [PATCH 04/15] mmc: support embedded data field in mmc_host
>
> From: Ohad Ben-Cohen <[email protected]>
>
> Add support to set/get mmc_host private embedded
> data.
>
> This is needed to allow software to dynamically
> create (and remove) SDIO functions which represents
> embedded SDIO devices.
>
> Typically, it will be used to set the context of
> a driver that is creating a new SDIO function
> (and would then expect to be able to get that context
> back upon creation of the new sdio func).
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> drivers/mmc/core/Kconfig | 8 ++++++++
> include/linux/mmc/host.h | 16 ++++++++++++++++
> 2 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index bb22ffd..ab27eb3 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -16,3 +16,11 @@ config MMC_UNSAFE_RESUME
>
> This option sets a default which can be overridden by the
> module parameter "removable=0" or "removable=1".
> +
> +config MMC_EMBEDDED_SDIO
> + boolean "MMC embedded SDIO device support"
> + help
> + If you say Y here, support will be added for embedded SDIO
> + devices (e.g. hardwired embedded WLAN SDIO devices).
> + Such devices require software support for emulating
> + card detect events.
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f65913c..9a48486 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -209,6 +209,10 @@ struct mmc_host {
> struct led_trigger *led; /* activity led */
> #endif
>
> +#ifdef CONFIG_MMC_EMBEDDED_SDIO
> + void *embedded_data;
> +#endif
> +
> struct dentry *debugfs_root;
>
> unsigned long private[0] ____cacheline_aligned;
> @@ -264,5 +268,17 @@ static inline void mmc_set_disable_delay(struct
> mmc_host *host,
> host->disable_delay = disable_delay;
> }
>
> +#ifdef CONFIG_MMC_EMBEDDED_SDIO
> +static inline void *mmc_get_embedded_data(struct mmc_host *host)
> +{
> + return host->embedded_data;
> +}
> +
> +static inline void mmc_set_embedded_data(struct mmc_host *host, void
> *data)
> +{
> + host->embedded_data = data;
> +}
> +#endif
> +
[Ghorai] we can avoid CONFIG_MMC_EMBEDDED_SDIO flag itself. Why we are passing fixed data? We can enquire form card for the functions, features,.. and at runtime itself. And we are adding many compile-time/kconfig options in this patch series.
> #endif
>
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-07-06 12:33:57

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 14/15] omap: zoom: add WLAN device

Hi Ohad,

On 07/06/2010 03:37 AM, ext Ohad Ben-Cohen wrote:
> From: Ohad Ben-Cohen<[email protected]>
>
> Add WLAN platform device and control
> functions (power and virtual card detect)
> in order to allow software to control the
> embedded SDIO WLAN device which resides on
> the ZOOM board (TI's wl1271 device).
>
> Based on Android's WLAN control functions by
> San Mehat<[email protected]>.
>
> Signed-off-by: Ohad Ben-Cohen<[email protected]>
> ---
> arch/arm/mach-omap2/board-zoom-wlan.c | 129 +++++++++++++++++++++++++
> arch/arm/mach-omap2/include/mach/board-zoom.h | 5 +
> 2 files changed, 134 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/mach-omap2/board-zoom-wlan.c
>
> diff --git a/arch/arm/mach-omap2/board-zoom-wlan.c b/arch/arm/mach-omap2/board-zoom-wlan.c
> new file mode 100644
> index 0000000..7ed5139
> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-zoom-wlan.c
> @@ -0,0 +1,129 @@
> +/* mach-omap2/board-zoom-wlan.c
> + *
> + * Board support for wl1271 embedded SDIO device.
> + *
> + * Copyright (C) 2010 Texas Instruments, Inc.
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +#include<linux/kernel.h>
> +#include<linux/init.h>
> +#include<linux/platform_device.h>
> +#include<linux/mmc/host.h>
> +#include<linux/mmc/sdio_ids.h>
> +#include<linux/err.h>
> +#include<linux/gpio.h>
> +#include<linux/wl12xx.h>
> +
> +#include "mux.h"
> +
> +#ifdef CONFIG_OMAP_ZOOM_WLAN
> +
> +/* these are zoom-specific board numbers */
> +#define OMAP_ZOOM_WLAN_PMENA_GPIO (101)
> +#define OMAP_ZOOM_WLAN_IRQ_GPIO (162)
> +
> +/* wl1271 virtual 'card detect' status */
> +static int omap_zoom_wlan_cd;
> +static void (*wlan_set_virtual_cd)(void *dev_id, int card_present);
> +static void (*wlan_set_data)(void *dev_id, void *priv);
> +static void *wlan_host_devid;
> +
> +int omap_zoom_wlan_register_embedded_control(void *dev_id,
> + void (*set_virtual_cd)(void *dev_id, int card_present),
> + void (*set_data)(void *dev_id, void *priv))
> +{
> + if (wlan_host_devid || wlan_set_virtual_cd || wlan_set_data)
> + return -EBUSY;
> +
> + wlan_set_virtual_cd = set_virtual_cd;
> + wlan_set_data = set_data;
> + wlan_host_devid = dev_id;
> +
> + return 0;
> +}
> +
> +int omap_zoom_wlan_get_virtual_cd(void)
> +{
> + return omap_zoom_wlan_cd;
> +}
> +
> +static void omap_zoom_wlan_set_embedded_data(void *priv)
> +{
> + if (wlan_set_data)
> + wlan_set_data(wlan_host_devid, priv);
> + else
> + pr_err("%s: host controller not registered yet\n", __func__);
> +}
> +
> +static void omap_zoom_wlan_set_carddetect(bool card_present)
> +{
> + omap_zoom_wlan_cd = card_present ? 1 : 0;
> +
> + pr_info("%s: %d\n", __func__, omap_zoom_wlan_cd);
> +
> + if (wlan_set_virtual_cd)
> + wlan_set_virtual_cd(wlan_host_devid, omap_zoom_wlan_cd);
> + else
> + pr_err("%s: host controller not registered yet\n", __func__);
> +}
> +
> +static void omap_zoom_wlan_power(bool enable)
> +{
> + int val = enable ? 1 : 0;
> +
> + pr_info("%s: set power %d\n", __func__, val);
> +
> + gpio_set_value(OMAP_ZOOM_WLAN_PMENA_GPIO, val);
> +}

Can we consider that OMAP_ZOOM_WLAN_PMENA_GPIO is equivalent to vmmc supply or
equivalent to supply voltage to the SDIO card?

If yes, then did you consider using the fixed regulator framework to define a
'vmmc' supply (based on OMAP_ZOOM_WLAN_PMENA_GPIO). Then the SDIO/MMC core
should take care of controlling this supply.

regards,
-roger

2010-07-08 20:13:15

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 06/15] omap zoom2: wlan board muxing

On Thu, Jul 8, 2010 at 12:45 PM, Tony Lindgren <[email protected]> wrote:
>
> * Ghorai, Sukumar <[email protected]> [100708 06:34]:
> > > @@ -71,6 +71,21 @@ static struct twl4030_platform_data zoom2_twldata = {
> > >
> > > ?#ifdef CONFIG_OMAP_MUX
> > > ?static struct omap_board_mux board_mux[] __initdata = {
> > > +#ifdef CONFIG_OMAP_ZOOM_WLAN
> >
> > [Ghorai] This is zoom board specific file, So why need this additional flag?
>
> Good point, the ifdef is unnecessary in for both zoom2 and zoom3.
> Will update in my queue to remove the ifdefs.

Thanks a lot, Tony.

>
> Tony

2010-07-06 00:40:14

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 06/15] omap zoom2: wlan board muxing

From: Ohad Ben-Cohen <[email protected]>

Add board muxing to support the wlan wl1271 chip that is
hardwired to mmc2 (third mmc controller) on the ZOOM2.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
arch/arm/mach-omap2/board-zoom2.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-zoom2.c b/arch/arm/mach-omap2/board-zoom2.c
index 803ef14..00871be 100644
--- a/arch/arm/mach-omap2/board-zoom2.c
+++ b/arch/arm/mach-omap2/board-zoom2.c
@@ -71,6 +71,21 @@ static struct twl4030_platform_data zoom2_twldata = {

#ifdef CONFIG_OMAP_MUX
static struct omap_board_mux board_mux[] __initdata = {
+#ifdef CONFIG_OMAP_ZOOM_WLAN
+ /* WLAN IRQ - GPIO 162 */
+ OMAP3_MUX(MCBSP1_CLKX, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN POWER ENABLE - GPIO 101 */
+ OMAP3_MUX(CAM_D2, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+ /* WLAN SDIO: MMC3 CMD */
+ OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE3 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC3 CLK */
+ OMAP3_MUX(ETK_CLK, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC3 DAT[0-3] */
+ OMAP3_MUX(ETK_D3, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D4, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D5, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(ETK_D6, OMAP_MUX_MODE2 | OMAP_PIN_INPUT_PULLUP),
+#endif
{ .reg_offset = OMAP_MUX_TERMINATOR },
};
#else
--
1.7.0.4


2010-07-06 07:05:57

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 02/15] wireless: wl1271: remove SDIO IDs from driver

On Tue, 2010-07-06 at 02:37 +0200, ext Ohad Ben-Cohen wrote:
> From: Ohad Ben-Cohen <[email protected]>
>
> Remove SDIO IDs from the driver code since now it is
> included in linux/mmc/sdio_ids.h.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---

Acked-by: Luciano Coelho <[email protected]>

--
Cheers,
Luca.


2010-07-06 10:35:54

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Ohad,

On 07/06/2010 12:30 PM, ext Ohad Ben-Cohen wrote:
> Hi Roger,
>
> On Tue, Jul 6, 2010 at 11:53 AM, Roger Quadros<[email protected]> wrote:
>> Could you please explain why you need to do this?
>
> To minimize power consumption when the wlan device is not in use, we
> would like to keep the device powered off as long as the interface is
> down, and only power it on when the user brings up the interface.
>
Agreed.

> Whenever the chip is powered on, it must be initialized and
> reconfigured by mmc_attach_sdio, and the wl1271 driver have to wait
> for that phase to successfully complete (essentially waiting for the
> sdio_driver's probe function to be called).
>
> To make sure this SDIO init step occurs correctly every time we toggle
> the device's power back on, and to prevent potential races, we also
> have to make sure that the sdio function is removed every time we
> power off the chip (the driver waits for the sdio_driver's remove
> function to be called).
>
> That's why we let the mmc layer think that the card is removed:
> physically it is still there, but for all practical purposes it is
> really removed, because once you power off the chip, you must
> reinitialize it again next time you power it on, as if the card was
> really removed and re-inserted.
>
My point is that shouldn't this be handled by SDIO core?
If there are no users for the SDIO function and the card, doesn't the SDIO core
power down the slot, and take care of re-initialization when it is powered up?

2010-07-07 20:00:17

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Nicolas Pitre wrote:
> On Wed, 7 Jul 2010, Roger Quadros wrote:
>
>> On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
>>> For eMMC in omap_hsmmc, this is all done via claim_host / release_host
>>> which call ->enable() / ->disable() methods. omap_hsmmc makes use of
>>> mmc_power_restore_host() which calls host->bus_ops->power_restore()
>>> which is not implemented for SDIO, but for MMC and SD it reinitializes
>>> the card.
>
> This is IMHO a really bad design. The power control decision has to
> come from the top, not from the bottom. And certainly not with a
> U-turn dependency the omap_hsmmc is using.

The power control decision does come from the top via mmc_claim_host /
mmc_release_host.

>
> I regret to say this, but the omap_hsmmc driver is becoming a total
> mess. The host controller driver has to be a dumb interface serving
> requests from the hardware used by the upper layer stack, not the place
> where decisions such as power handling should be made. Think of it like
> an ethernet driver. No ethernet driver in Linux is telling the IP stack
> when to shut down.

The omap_hsmmc driver does not tell the core to shut down the upper layers.
Instead the core tells the omap_hsmmc driver that it is "disabled" i.e.
not currently being used so it can start taking steps to save power.
That is sensible because not even the upper layers know when the next
activity will be.

>
>> Shouldn't the power control intelligence (i.e. when to turn power ON/OFF) lie
>> with the bus drivers?
>
> Absolutely! And in the SDIO case that should lie with each function
> drivers. Please let's stop this omap_hsmmc madness.

You are dealing with a trivial case - turn off the power when not in use.
We have 3 power saving actions: enable DPS, put the card to sleep,
and finally power it off. Each increases the latency to start up
again and so must be staggered. With DPS-enabled the host controller can
be powered off at any time. In that case the controller registers must be
restored. There are numerous entry points to the driver. Checking whether
to restore registers at every entry point is error prone and messy.
Instead we require that the core tells the driver when to enable and
disable.

>
>
> Nicolas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2010-07-06 09:31:12

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Roger,

On Tue, Jul 6, 2010 at 11:53 AM, Roger Quadros <[email protected]> wrote:
> Could you please explain why you need to do this?

To minimize power consumption when the wlan device is not in use, we
would like to keep the device powered off as long as the interface is
down, and only power it on when the user brings up the interface.

Whenever the chip is powered on, it must be initialized and
reconfigured by mmc_attach_sdio, and the wl1271 driver have to wait
for that phase to successfully complete (essentially waiting for the
sdio_driver's probe function to be called).

To make sure this SDIO init step occurs correctly every time we toggle
the device's power back on, and to prevent potential races, we also
have to make sure that the sdio function is removed every time we
power off the chip (the driver waits for the sdio_driver's remove
function to be called).

That's why we let the mmc layer think that the card is removed:
physically it is still there, but for all practical purposes it is
really removed, because once you power off the chip, you must
reinitialize it again next time you power it on, as if the card was
really removed and re-inserted.

Thanks,
Ohad.

>
> br,
> -roger
>

2010-07-06 17:42:37

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Tue, 6 Jul 2010, Roger Quadros wrote:

> On 07/06/2010 03:53 PM, ext Ohad Ben-Cohen wrote:
> > Hi Roger,
> >
> > On Tue, Jul 6, 2010 at 1:35 PM, Roger Quadros<[email protected]>
> > wrote:
> > > My point is that shouldn't this be handled by SDIO core?
> >
> > Care to explain what you mean / give a code example ?
>
> If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
> the SDIO/MMC core should tackle it, just like it deals with supply for slots
> with removable cards.

Exact.

> > You need card detect events in order to trigger card& sdio function
> > initialization and removals.

Why would you trigger function initialization and removal? Just to turn
off power? That's a bit like pulling off the battery from your laptop
when you want to suspend it. There is a better way to go about things.

> > Please share any alternative approach you may be thinking on.
>
> OK, this is how I see it.
>
> - Treat the non-removable card as non-removable. So no need to do card detect
> emulation.
>
> - Treat the GPIO power enable on wl1271 as VMMC supply. Use fixed regulator
> framework to define this regulator & supply. Even though you mention that it
> is not actually a supply, it fits well in the fixed supply framework.
>
> - When the host controller is enumerated, the mmc core will power up the slot,
> find the sdio card, and probe the function driver (i.e. wl1271_sdio).
>
> - if interface is not in use, the function driver must release the sdio host,
> and this should eventually disable the vmmc supply.
>
> - Whenever the wlan interface must be brought up, wl1271_sdio, can claim the
> sdio host. this will cause the vmmc supply to be enabled, for as long as the
> interface is up.
>
> Does this address all issues?

This is mostly all good, except that claiming/releasing the SDIO host is
about access to the bus. It must be claimed right before doing any IO,
and released right after that, even when the card is expected to remain
powered. This is not the proper place to hook power control.

Another function pair would be needed instead, which would do almost
like the suspend/resume code is already doing. Something like:

/*
* Indicate to the core SDIO layer that we're not requiring that the
* function remain powered. If all functions for the card are in the
* same "no power" state, then the host controller can remove power from
* the card. Note: the function driver must preserve hardware states if
* necessary.
*/
int sdio_release_power(struct sdio_func *func);

/*
* Indicate to the core SDIO layer that we want power back for this
* SDIO function. The power may or may not actually have been removed
* since last call to sdio_release_power(), so the function driver must
* not assume any preserved state at the hardware level and re-perform
* all the necessary hardware config. This function returns 0 when
* power is actually restored, or some error code if this cannot be
* achieved. One error reason might be that the card is no longer
* available on the bus (was removed while powered down and card
* detection didn't trigger yet).
*/
int sdio_claim_power(struct sdio_func *func);

That's it. When the network interface is down and the hardware is not
needed, you call sdio_release_power(). When the request to activate the
network interface is received, you call sdio_claim_power() and configure
the hardware appropriately. If sdio_claim_power() returns an error,
then you just return an error to the network request, and eventually the
driver's remove method will be called if this is indeed because the card
was removed.

In the core SDIO code, this is almost identical to a suspend/resume
request, except that the request comes from the function driver instead
of the core MMC code.


Nicolas

2010-07-09 08:33:10

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Roger,

On Fri, Jul 9, 2010 at 11:12 AM, Roger Quadros <[email protected]> wrote:
> You are already
> creating a new platform device for wl1271_sdio just to pass platform_data

With the new approach, I can remove this platform device altogether,
and completely rely on the sdio function device for driver probing (as
it was before). I just need to add this little support of delivering
small bits of platform data - the mechanism should be simple and
generic, and would considerably simplify the driver.

> I think this case is specific to wl1271.

Yes, it is.

There's an hw issue with the wl1271's in-band interrupt, so we can't
use it (the issue is fixed in the wl1281 btw).

But we also have to deliver the reference clock info to the driver,
which is board-specific data, and would be relevant to all wl12xx
devices.

Thanks,
Ohad.

2010-07-08 03:39:58

by Ghorai, Sukumar

[permalink] [raw]
Subject: RE: [PATCH 15/15] omap: zoom: enable WLAN device



> -----Original Message-----
> From: [email protected] [mailto:linux-mmc-
> [email protected]] On Behalf Of Ohad Ben-Cohen
> Sent: Tuesday, July 06, 2010 6:08 AM
> To: [email protected]; [email protected]; linux-
> [email protected]
> Cc: [email protected]; [email protected];
> Chikkature Rajashekar, Madhusudhan; Luciano Coelho; akpm@linux-
> foundation.org; San Mehat; Ben-cohen, Ohad
> Subject: [PATCH 15/15] omap: zoom: enable WLAN device
>
> From: Ohad Ben-Cohen <[email protected]>
>
> Make it possible to build and use TI's wl1271
> device on the ZOOM boards.
>
> The device is an embedded SDIO WLAN chip
> that is hardwired to the 3rd mmc controller
> of the ZOOM2/3 boards.
>
> Signed-off-by: Ohad Ben-Cohen <[email protected]>
> ---
> arch/arm/mach-omap2/Kconfig | 5 +++++
> arch/arm/mach-omap2/Makefile | 1 +
> arch/arm/mach-omap2/board-zoom-peripherals.c | 15 +++++++++++++++
> 3 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index b31b6f1..7fee11b 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -131,6 +131,11 @@ config MACH_OMAP_ZOOM3
> depends on ARCH_OMAP3
> select OMAP_PACKAGE_CBP
>
> +config OMAP_ZOOM_WLAN
> + bool "OMAP Zoom board WLAN support"
> + depends on MACH_OMAP_ZOOM2 || MACH_OMAP_ZOOM3
> + select MMC_EMBEDDED_SDIO
> +
> config MACH_CM_T35
> bool "CompuLab CM-T35 module"
> depends on ARCH_OMAP3
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index ea52b03..ac1bad9 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -129,6 +129,7 @@ obj-$(CONFIG_MACH_OMAP_ZOOM3) += board-
> zoom3.o \
> board-zoom-peripherals.o \
> hsmmc.o \
> board-zoom-debugboard.o
> +obj-y += board-zoom-wlan.o
> obj-$(CONFIG_MACH_OMAP_3630SDP) += board-3630sdp.o \
> board-zoom-peripherals.o \
> hsmmc.o
> diff --git a/arch/arm/mach-omap2/board-zoom-peripherals.c b/arch/arm/mach-
> omap2/board-zoom-peripherals.c
> index 6b39849..3128cd4 100644
> --- a/arch/arm/mach-omap2/board-zoom-peripherals.c
> +++ b/arch/arm/mach-omap2/board-zoom-peripherals.c
> @@ -16,11 +16,13 @@
> #include <linux/gpio.h>
> #include <linux/i2c/twl.h>
> #include <linux/regulator/machine.h>
> +#include <linux/mmc/host.h>
>
> #include <asm/mach-types.h>
> #include <asm/mach/arch.h>
> #include <asm/mach/map.h>
>
> +#include <mach/board-zoom.h>
> #include <plat/common.h>
> #include <plat/usb.h>
>
> @@ -168,6 +170,18 @@ static struct omap2_hsmmc_info mmc[] __initdata = {
> .nonremovable = true,
> .power_saving = true,
> },
> +#ifdef CONFIG_OMAP_ZOOM_WLAN
> + {
> + .mmc = 3,
> + .wires = 4,
> + .gpio_cd = -EINVAL,
> + .gpio_wp = -EINVAL,
> + .register_embedded_control =
> + omap_zoom_wlan_register_embedded_control,
> + .virtual_get_cd = omap_zoom_wlan_get_virtual_cd,
> + .ocr_mask = MMC_VDD_165_195,
> + },
> +#endif
> {} /* Terminator */
> };
>
> @@ -282,4 +296,5 @@ void __init zoom_peripherals_init(void)
> omap_i2c_init();
> usb_musb_init(&musb_board_data);
> enable_board_wakeup_source();
> + omap_zoom_wlan_init();
> }
[Ghorai] In general we can avoid OMAP_ZOOM_WLAN and MMC_EMBEDDED_SDIO as kconfig option. 1st one is board specific and 2nd one could be generic sdio code. As I mentioned in other patch too.
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-07-06 11:48:50

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On Tue, Jul 6, 2010 at 1:22 PM, Ohad Ben-Cohen <[email protected]> wrote:
> Note: the wl1271 device does support standard card detection, but
> AFAIK there's a limitation to use that with the specific omap
> controller the device is hardwired to. I will try to get more info
> about that, but probably Madhu can comment on that better.


Some correction and additional info:

The wl1271 device has an issue which makes the standard card detect
mechanism irrelevant: it is always up, even if the power enable gpio
input of the device is down (the power enable input does not supply
the power to the chip, it's just logical digital high/low input upon
which the device reacts). That's why we must use software control for
emulating card detect with that device.

In addition, as far as I could find out, the card detect mechanism on
the ZOOM is implemented by mechanical means, and thus is not relevant
for hardwired embedded SDIO devices (I'm not even sure card detect is
supported for the 3rd mmc controller).

2010-07-08 20:10:55

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

Hi Nicolas and Roger,

On Tue, Jul 6, 2010 at 8:42 PM, Nicolas Pitre <[email protected]> wrote:
> On Tue, 6 Jul 2010, Roger Quadros wrote:
> > If the Power enable GPIO can be treated as SDIO slot supply (i.e. vmmc), then
> > the SDIO/MMC core should tackle it, just like it deals with supply for slots
> > with removable cards.
>
...
> Another function pair would be needed instead, which would do almost
> like the suspend/resume code is already doing. Something like:

Thanks a lot for your review and comments, and for taking the time to
present your approach.

I like it !

It'd allow us to lose the software (or fake if you want ;) card detect
mechanism, which is something that should have been added to each
platform we wanted to support.

We would only need to make it possible to deliver board-specific data
to the function driver (e.g., in the case of the wl1271, we need irq
and board_ref_clock data). That would require some board-level
platform-data configuration, which will be specific to the controller
to which the device is hardwired to. This data should propagate
through the host controller to the SDIO core so it would eventually be
accessible by the function driver (e.g. via func->dev.pdata).

We'll adapt and post follow-up patches.

Thanks again,
Ohad.

2010-07-06 01:45:14

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH 05/15] omap: hsmmc: add virtual card detect support

On Tue, 6 Jul 2010, Ohad Ben-Cohen wrote:

> From: Ohad Ben-Cohen <[email protected]>
>
> Add support for software emulation of card detect
> events.
>
> This is required for specific controllers
> that are hard wired with embedded SDIO devices
> (such as TI's wl1271 WLAN device).

Why?

Many instances of hardwired SDIO based WLAN devices do exist already,
and they don't need extra card detect events to be generated. The core
MMC code already triggers a probe for cards whenever a new host
controller is registered.

> Board-specific configuration is required to
> enable this software card detect control.

Could you elaborate please?


Nicolas

2010-07-06 11:43:37

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 06/15] omap zoom2: wlan board muxing

* Ohad Ben-Cohen <[email protected]> [100706 03:37]:
> From: Ohad Ben-Cohen <[email protected]>
>
> Add board muxing to support the wlan wl1271 chip that is
> hardwired to mmc2 (third mmc controller) on the ZOOM2.

I'll pick this and following patch into omap for-next.

Regards,

Tony

2010-07-06 00:40:39

by Ohad Ben Cohen

[permalink] [raw]
Subject: [PATCH 10/15] wireless: wl1271: support return value for the set power func

From: Ohad Ben-Cohen <[email protected]>

Make it possible for the set power method to indicate a
success/failure return value. This is needed to support
more complex power on/off operations such as bringing up
(and down) sdio functions.

Signed-off-by: Ohad Ben-Cohen <[email protected]>
---
drivers/net/wireless/wl12xx/wl1271.h | 2 +-
drivers/net/wireless/wl12xx/wl1271_io.h | 8 +++++---
drivers/net/wireless/wl12xx/wl1271_main.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_sdio.c | 4 +++-
drivers/net/wireless/wl12xx/wl1271_spi.c | 4 +++-
5 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index fc4d122..5250361 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -340,7 +340,7 @@ struct wl1271_if_operations {
bool fixed);
void (*reset)(struct wl1271 *wl);
void (*init)(struct wl1271 *wl);
- void (*power)(struct wl1271 *wl, bool enable);
+ int (*power)(struct wl1271 *wl, bool enable);
struct device* (*dev)(struct wl1271 *wl);
void (*enable_irq)(struct wl1271 *wl);
void (*disable_irq)(struct wl1271 *wl);
diff --git a/drivers/net/wireless/wl12xx/wl1271_io.h b/drivers/net/wireless/wl12xx/wl1271_io.h
index bc806c7..4a5b92c 100644
--- a/drivers/net/wireless/wl12xx/wl1271_io.h
+++ b/drivers/net/wireless/wl12xx/wl1271_io.h
@@ -144,10 +144,12 @@ static inline void wl1271_power_off(struct wl1271 *wl)
clear_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
}

-static inline void wl1271_power_on(struct wl1271 *wl)
+static inline int wl1271_power_on(struct wl1271 *wl)
{
- wl->if_ops->power(wl, true);
- set_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
+ int ret = wl->if_ops->power(wl, true);
+ if (ret == 0)
+ set_bit(WL1271_FLAG_GPIO_POWER, &wl->flags);
+ return ret;
}


diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c b/drivers/net/wireless/wl12xx/wl1271_main.c
index b7d9137..6bd748e 100644
--- a/drivers/net/wireless/wl12xx/wl1271_main.c
+++ b/drivers/net/wireless/wl12xx/wl1271_main.c
@@ -620,7 +620,9 @@ static int wl1271_chip_wakeup(struct wl1271 *wl)
int ret = 0;

msleep(WL1271_PRE_POWER_ON_SLEEP);
- wl1271_power_on(wl);
+ ret = wl1271_power_on(wl);
+ if (ret < 0)
+ goto out;
msleep(WL1271_POWER_ON_SLEEP);
wl1271_io_reset(wl);
wl1271_io_init(wl);
diff --git a/drivers/net/wireless/wl12xx/wl1271_sdio.c b/drivers/net/wireless/wl12xx/wl1271_sdio.c
index 9903ae9..571c6b9 100644
--- a/drivers/net/wireless/wl12xx/wl1271_sdio.c
+++ b/drivers/net/wireless/wl12xx/wl1271_sdio.c
@@ -144,7 +144,7 @@ static void wl1271_sdio_raw_write(struct wl1271 *wl, int addr, void *buf,

}

-static void wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
+static int wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
{
struct sdio_func *func = wl_to_func(wl);

@@ -159,6 +159,8 @@ static void wl1271_sdio_set_power(struct wl1271 *wl, bool enable)
sdio_disable_func(func);
sdio_release_host(func);
}
+
+ return 0;
}

static struct wl1271_if_operations sdio_ops = {
diff --git a/drivers/net/wireless/wl12xx/wl1271_spi.c b/drivers/net/wireless/wl12xx/wl1271_spi.c
index e866049..85a167f 100644
--- a/drivers/net/wireless/wl12xx/wl1271_spi.c
+++ b/drivers/net/wireless/wl12xx/wl1271_spi.c
@@ -313,10 +313,12 @@ static irqreturn_t wl1271_irq(int irq, void *cookie)
return IRQ_HANDLED;
}

-static void wl1271_spi_set_power(struct wl1271 *wl, bool enable)
+static int wl1271_spi_set_power(struct wl1271 *wl, bool enable)
{
if (wl->set_power)
wl->set_power(enable);
+
+ return 0;
}

static struct wl1271_if_operations spi_ops = {
--
1.7.0.4


2010-08-10 21:21:50

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH 11/15] wireless: wl1271: introduce platform device support

On Tue, Jul 6, 2010 at 8:42 PM, Nicolas Pitre <[email protected]> wrote:
> Another function pair would be needed instead, which would do almost
> like the suspend/resume code is already doing. ?Something like:
>
> /*
> ?* Indicate to the core SDIO layer that we're not requiring that the
> ?* function remain powered. ?If all functions for the card are in the
> ?* same "no power" state, then the host controller can remove power from
> ?* the card. ?Note: the function driver must preserve hardware states if
> ?* necessary.
> ?*/
> int sdio_release_power(struct sdio_func *func);
>
> /*
> ?* Indicate to the core SDIO layer that we want power back for this
> ?* SDIO function. ?The power may or may not actually have been removed
> ?* since last call to sdio_release_power(), so the function driver must
> ?* not assume any preserved state at the hardware level and re-perform
> ?* all the necessary hardware config. ?This function returns 0 when
> ?* power is actually restored, or some error code if this cannot be
> ?* achieved. ?One error reason might be that the card is no longer
> ?* available on the bus (was removed while powered down and card
> ?* detection didn't trigger yet).
> ?*/
> int sdio_claim_power(struct sdio_func *func);
>
> That's it. ?When the network interface is down and the hardware is not
> needed, you call sdio_release_power(). ?When the request to activate the
> network interface is received, you call sdio_claim_power() and configure
> the hardware appropriately. ?If sdio_claim_power() returns an error,
> then you just return an error to the network request, and eventually the
> driver's remove method will be called if this is indeed because the card
> was removed.
>

After playing with this a little, I am getting convinced that the way
to go here is to add runtime pm support to MMC/SDIO.

It just fits perfectly into this model:

Runtime PM already takes care of usage count per device (so it would
tell if an SDIO function is idle), takes into account parent-child
relationships (so if all SDIO functions are idle, it would
automatically send an idle request to the card device), doesn't break
existing drivers (drivers needs to explicitly enable it), play along
nicely with the system suspend/resume, etc..

I'm going to split the patches to two:
- first half would be basic wl1271 support for omap/zoom. it's pretty
straight forward and can probably already go thru.
- second half would be adding runtime PM to SDIO/MMC. I'll post some
initial patches to set off a discussion.

Thanks,
Ohad.