2010-09-23 08:20:41

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Add board configuration for the wl1271 daughter board. This patch is based
on Ohad Ben-Cohen's patches for Zoom boards.

Cc: Ohad Ben-Cohen <[email protected]>
Signed-off-by: Luciano Coelho <[email protected]>
---
There was a useless variable defined in omap3_beagle_init() that was causing a
warning. I have removed it in v2.

arch/arm/mach-omap2/board-omap3beagle.c | 69 +++++++++++++++++++++++++++++++
drivers/net/wireless/wl12xx/wl1271.h | 2 +-
2 files changed, 70 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 87969c7..755df29 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -29,7 +29,9 @@
#include <linux/mtd/nand.h>

#include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
#include <linux/i2c/twl.h>
+#include <linux/wl12xx.h>

#include <mach/hardware.h>
#include <asm/mach-types.h>
@@ -48,6 +50,9 @@
#include "mux.h"
#include "hsmmc.h"

+#define OMAP_BEAGLE_WLAN_EN_GPIO (139)
+#define OMAP_BEAGLE_WLAN_IRQ_GPIO (137)
+
#define NAND_BLOCK_SIZE SZ_128K

static struct mtd_partition omap3beagle_nand_partitions[] = {
@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)

#include "sdram-micron-mt46h32m32lf-6.h"

+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
+ .irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
+ .board_ref_clock = 2, /* 38.4 MHz */
+};
+
static struct omap2_hsmmc_info mmc[] = {
{
.mmc = 1,
.wires = 8,
.gpio_wp = 29,
},
+ {
+ .name = "wl1271",
+ .mmc = 2,
+ .wires = 4,
+ .gpio_wp = -EINVAL,
+ .gpio_cd = -EINVAL,
+ .nonremovable = true,
+ },
{} /* Terminator */
};

@@ -176,10 +194,43 @@ static struct regulator_consumer_supply beagle_vmmc1_supply = {
.supply = "vmmc",
};

+static struct regulator_consumer_supply beagle_vmmc2_supply = {
+ .supply = "vmmc",
+ .dev_name = "mmci-omap-hs.1",
+};
+
+
static struct regulator_consumer_supply beagle_vsim_supply = {
.supply = "vmmc_aux",
};

+
+static struct regulator_init_data beagle_vmmc2 = {
+ .constraints = {
+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ },
+ .num_consumer_supplies = 1,
+ .consumer_supplies = &beagle_vmmc2_supply,
+};
+
+static struct fixed_voltage_config beagle_vwlan = {
+ .supply_name = "vwl1271",
+ .microvolts = 1800000, /* 1.8V */
+ .gpio = OMAP_BEAGLE_WLAN_EN_GPIO,
+ .startup_delay = 70000, /* 70ms */
+ .enable_high = 1,
+ .enabled_at_boot = 0,
+ .init_data = &beagle_vmmc2,
+};
+
+static struct platform_device omap_vwlan_device = {
+ .name = "reg-fixed-voltage",
+ .id = 1,
+ .dev = {
+ .platform_data = &beagle_vwlan,
+ },
+};
+
static struct gpio_led gpio_leds[];

static int beagle_twl_gpio_setup(struct device *dev,
@@ -449,6 +500,19 @@ static const struct ehci_hcd_omap_platform_data ehci_pdata __initconst = {

#ifdef CONFIG_OMAP_MUX
static struct omap_board_mux board_mux[] __initdata = {
+ /* WLAN IRQ - GPIO 137 */
+ OMAP3_MUX(SDMMC2_DAT5, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN POWER ENABLE - GPIO 139 */
+ OMAP3_MUX(SDMMC2_DAT7, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT),
+ /* WLAN SDIO: MMC2 CMD */
+ OMAP3_MUX(SDMMC2_CMD, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC2 CLK */
+ OMAP3_MUX(SDMMC2_CLK, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
+ /* WLAN SDIO: MMC2 DAT[0-3] */
+ OMAP3_MUX(SDMMC2_DAT0, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(SDMMC2_DAT1, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(SDMMC2_DAT2, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
+ OMAP3_MUX(SDMMC2_DAT3, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP),
{ .reg_offset = OMAP_MUX_TERMINATOR },
};
#else
@@ -464,9 +528,14 @@ static struct omap_musb_board_data musb_board_data = {
static void __init omap3_beagle_init(void)
{
omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
+ if (wl12xx_set_platform_data(&omap_beagle_wlan_data))
+ pr_err("error setting wl12xx data\n");
+
omap3_beagle_i2c_init();
platform_add_devices(omap3_beagle_devices,
ARRAY_SIZE(omap3_beagle_devices));
+ platform_device_register(&omap_vwlan_device);
+
omap_serial_init();

omap_mux_init_gpio(170, OMAP_PIN_INPUT);
diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
index 4134f44..8bb028e 100644
--- a/drivers/net/wireless/wl12xx/wl1271.h
+++ b/drivers/net/wireless/wl12xx/wl1271.h
@@ -60,7 +60,7 @@ enum {
DEBUG_ALL = ~0,
};

-#define DEBUG_LEVEL (DEBUG_NONE)
+#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)

#define DEBUG_DUMP_LIMIT 1024

--
1.6.3.3



2010-09-23 12:17:37

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Hi Felipe,


On Thu, 2010-09-23 at 13:45 +0200, ext Felipe Balbi wrote:
> On Thu, Sep 23, 2010 at 03:20:03AM -0500, Luciano Coelho wrote:
> > static struct mtd_partition omap3beagle_nand_partitions[] = {
> >@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)
> >
> > #include "sdram-micron-mt46h32m32lf-6.h"
> >
> >+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
> >+ .irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
>
> can you pass IRQ as a struct resource ?

Sorry, I don't understand what you mean here. Can you clarify?


> >+static struct regulator_consumer_supply beagle_vmmc2_supply = {
> >+ .supply = "vmmc",
> >+ .dev_name = "mmci-omap-hs.1",
> >+};
>
> this should be:
>
> static struct regulator_consume_supply beagle_vmmc2_supplies = {
> {
> .supply = "vmmc",
> .dev_name = "mmci-omap-hs.1",
> },
> };

Okay, I'll do it. The other supplies that are already defined in this
board file don't do that, but it's better to do it as you proposed.
Thanks.


> >+static struct regulator_init_data beagle_vmmc2 = {
> >+ .constraints = {
> >+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> >+ },
> >+ .num_consumer_supplies = 1,
> >+ .consumer_supplies = &beagle_vmmc2_supply,
>
> please tabify, it's easier for reading, same for all below:

Sure, I'll do it. You're not the first one to complain. :)


> constrants = {
> .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> },
> .num_consumer_supplies = ARRAY_SIZE(beagle_vmmc2_supplies),
> .consumer_supplies = beagle_vmmc2_supplies,
>
> >@@ -464,9 +528,14 @@ static struct omap_musb_board_data musb_board_data = {
> > static void __init omap3_beagle_init(void)
> > {
> > omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
> >+ if (wl12xx_set_platform_data(&omap_beagle_wlan_data))
> >+ pr_err("error setting wl12xx data\n");
> >+
> > omap3_beagle_i2c_init();
> > platform_add_devices(omap3_beagle_devices,
> > ARRAY_SIZE(omap3_beagle_devices));
> >+ platform_device_register(&omap_vwlan_device);
> >+
>
> trailing blank line. Also, you could just add omap_vwlan_device to
> omap3_beagle_devices.

I'll fix it.


> >diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
> >index 4134f44..8bb028e 100644
> >--- a/drivers/net/wireless/wl12xx/wl1271.h
> >+++ b/drivers/net/wireless/wl12xx/wl1271.h
> >@@ -60,7 +60,7 @@ enum {
> > DEBUG_ALL = ~0,
> > };
> >
> >-#define DEBUG_LEVEL (DEBUG_NONE)
> >+#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)
>
> some debugging trash came into commit ??

Yes, I kept it by mistake. I've already removed it from the v3 I sent a
bit earlier today.



--
Cheers,
Luca.


2010-09-23 11:45:38

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Hi,

On Thu, Sep 23, 2010 at 03:20:03AM -0500, Luciano Coelho wrote:
> static struct mtd_partition omap3beagle_nand_partitions[] = {
>@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)
>
> #include "sdram-micron-mt46h32m32lf-6.h"
>
>+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
>+ .irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),

can you pass IRQ as a struct resource ?

>+ .board_ref_clock = 2, /* 38.4 MHz */
>+};
>+
> static struct omap2_hsmmc_info mmc[] = {
> {
> .mmc = 1,
> .wires = 8,
> .gpio_wp = 29,
> },
>+ {
>+ .name = "wl1271",
>+ .mmc = 2,
>+ .wires = 4,
>+ .gpio_wp = -EINVAL,
>+ .gpio_cd = -EINVAL,
>+ .nonremovable = true,
>+ },
> {} /* Terminator */
> };
>
>@@ -176,10 +194,43 @@ static struct regulator_consumer_supply beagle_vmmc1_supply = {
> .supply = "vmmc",
> };
>
>+static struct regulator_consumer_supply beagle_vmmc2_supply = {
>+ .supply = "vmmc",
>+ .dev_name = "mmci-omap-hs.1",
>+};

this should be:

static struct regulator_consume_supply beagle_vmmc2_supplies = {
{
.supply = "vmmc",
.dev_name = "mmci-omap-hs.1",
},
};

>+static struct regulator_init_data beagle_vmmc2 = {
>+ .constraints = {
>+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
>+ },
>+ .num_consumer_supplies = 1,
>+ .consumer_supplies = &beagle_vmmc2_supply,

please tabify, it's easier for reading, same for all below:

constrants = {
.valid_ops_mask = REGULATOR_CHANGE_STATUS,
},
.num_consumer_supplies = ARRAY_SIZE(beagle_vmmc2_supplies),
.consumer_supplies = beagle_vmmc2_supplies,

>@@ -464,9 +528,14 @@ static struct omap_musb_board_data musb_board_data = {
> static void __init omap3_beagle_init(void)
> {
> omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
>+ if (wl12xx_set_platform_data(&omap_beagle_wlan_data))
>+ pr_err("error setting wl12xx data\n");
>+
> omap3_beagle_i2c_init();
> platform_add_devices(omap3_beagle_devices,
> ARRAY_SIZE(omap3_beagle_devices));
>+ platform_device_register(&omap_vwlan_device);
>+

trailing blank line. Also, you could just add omap_vwlan_device to
omap3_beagle_devices.

>diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
>index 4134f44..8bb028e 100644
>--- a/drivers/net/wireless/wl12xx/wl1271.h
>+++ b/drivers/net/wireless/wl12xx/wl1271.h
>@@ -60,7 +60,7 @@ enum {
> DEBUG_ALL = ~0,
> };
>
>-#define DEBUG_LEVEL (DEBUG_NONE)
>+#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)

some debugging trash came into commit ??

--
balbi

2010-09-23 10:30:23

by Grazvydas Ignotas

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
<[email protected]> wrote:
> Add board configuration for the wl1271 daughter board. ?This patch is based
> on Ohad Ben-Cohen's patches for Zoom boards.

Hm can that daughter board be detected? With your patch all beagle
users will get GPIO139 toggled, and if someone has that wired to
chainsaw switch somebody might get hurt.

> Cc: Ohad Ben-Cohen <[email protected]>
> Signed-off-by: Luciano Coelho <[email protected]>
> ---
> There was a useless variable defined in omap3_beagle_init() that was causing a
> warning. ?I have removed it in v2.
>
> ?arch/arm/mach-omap2/board-omap3beagle.c | ? 69 +++++++++++++++++++++++++++++++
> ?drivers/net/wireless/wl12xx/wl1271.h ? ?| ? ?2 +-
> ?2 files changed, 70 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> index 87969c7..755df29 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c

<snip>

> +static struct regulator_consumer_supply beagle_vmmc2_supply = {
> + ? ? ? .supply ? ? ? ? = "vmmc",
> + ? ? ? .dev_name ? ? ? = "mmci-omap-hs.1",
> +};
> +
> +

single newline is enough.

> ?static struct regulator_consumer_supply beagle_vsim_supply = {
> ? ? ? ?.supply ? ? ? ? ? ? ? ? = "vmmc_aux",
> ?};
>
> +

here too.

> +static struct regulator_init_data beagle_vmmc2 = {
> + ? ? ? .constraints = {
> + ? ? ? ? ? ? ? .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> + ? ? ? },
> + ? ? ? .num_consumer_supplies = 1,
> + ? ? ? .consumer_supplies = &beagle_vmmc2_supply,
> +};
> +
> +static struct fixed_voltage_config beagle_vwlan = {
> + ? ? ? .supply_name = "vwl1271",
> + ? ? ? .microvolts = 1800000, ?/* 1.8V */
> + ? ? ? .gpio = OMAP_BEAGLE_WLAN_EN_GPIO,
> + ? ? ? .startup_delay = 70000, /* 70ms */
> + ? ? ? .enable_high = 1,
> + ? ? ? .enabled_at_boot = 0,
> + ? ? ? .init_data = &beagle_vmmc2,
> +};

We tabify all structures in board files, take a look at other structures.

<snip>

> diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
> index 4134f44..8bb028e 100644
> --- a/drivers/net/wireless/wl12xx/wl1271.h
> +++ b/drivers/net/wireless/wl12xx/wl1271.h
> @@ -60,7 +60,7 @@ enum {
> ? ? ? ?DEBUG_ALL ? ? ? = ~0,
> ?};
>
> -#define DEBUG_LEVEL (DEBUG_NONE)
> +#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)

I guess you didn't want that?

2010-09-23 12:28:36

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, 2010-09-23 at 14:20 +0200, ext Felipe Balbi wrote:
> Hi Luca,
>
> On Thu, Sep 23, 2010 at 07:17:33AM -0500, Luciano Coelho wrote:
> >On Thu, 2010-09-23 at 13:45 +0200, ext Felipe Balbi wrote:
> >> On Thu, Sep 23, 2010 at 03:20:03AM -0500, Luciano Coelho wrote:
> >> > static struct mtd_partition omap3beagle_nand_partitions[] = {
> >> >@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)
> >> >
> >> > #include "sdram-micron-mt46h32m32lf-6.h"
> >> >
> >> >+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
> >> >+ .irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
> >>
> >> can you pass IRQ as a struct resource ?
> >
> >Sorry, I don't understand what you mean here. Can you clarify?
>
> static struct resource wl12xx_resource[] = {
> {
> .start = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
> .flags = IORESOURCE_IRQ,
> },
> };
>
> static struct platform_device wl12xx_platform_device = {
> .resource = wl12xx_resource,
> .num_resources = ARRAY_SIZE(wl12xx_resource),
> .....
> };
>
> then on driver you would:
>
> irq = platform_get_irq(pdev, 0);
>
> something like that :-)

Ah, I see. Thanks for the explanation. I guess we could do that in a
separate patch. The zoom board file already uses this .irq element, so
this patch would not be the appropriate place to change it.

Is it okay if I deal with that after I'm done with the current patch?


2010-09-23 12:30:42

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Hi,

On Thu, Sep 23, 2010 at 07:28:28AM -0500, Luciano Coelho wrote:
>Ah, I see. Thanks for the explanation. I guess we could do that in a
>separate patch. The zoom board file already uses this .irq element, so
>this patch would not be the appropriate place to change it.
>
>Is it okay if I deal with that after I'm done with the current patch?

sure :-)

--
balbi

2010-09-23 12:03:35

by Robert Nelson

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, Sep 23, 2010 at 5:30 AM, Grazvydas Ignotas <[email protected]> wrote:
> On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
> <[email protected]> wrote:
>> Add board configuration for the wl1271 daughter board. ?This patch is based
>> on Ohad Ben-Cohen's patches for Zoom boards.
>
> Hm can that daughter board be detected? With your patch all beagle
> users will get GPIO139 toggled, and if someone has that wired to
> chainsaw switch somebody might get hurt.

Expansion boards really need to follow:

http://elinux.org/BeagleBoardPinMux#Expansion_boards

Is there any eeprom on i2c bus #2 for identification on this board?

Otherwise this is going to break a bunch of existing adapters.

Regards,

>
>> Cc: Ohad Ben-Cohen <[email protected]>
>> Signed-off-by: Luciano Coelho <[email protected]>
>> ---
>> There was a useless variable defined in omap3_beagle_init() that was causing a
>> warning. ?I have removed it in v2.
>>
>> ?arch/arm/mach-omap2/board-omap3beagle.c | ? 69 +++++++++++++++++++++++++++++++
>> ?drivers/net/wireless/wl12xx/wl1271.h ? ?| ? ?2 +-
>> ?2 files changed, 70 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> index 87969c7..755df29 100644
>> --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>
> <snip>
>
>> +static struct regulator_consumer_supply beagle_vmmc2_supply = {
>> + ? ? ? .supply ? ? ? ? = "vmmc",
>> + ? ? ? .dev_name ? ? ? = "mmci-omap-hs.1",
>> +};
>> +
>> +
>
> single newline is enough.
>
>> ?static struct regulator_consumer_supply beagle_vsim_supply = {
>> ? ? ? ?.supply ? ? ? ? ? ? ? ? = "vmmc_aux",
>> ?};
>>
>> +
>
> here too.
>
>> +static struct regulator_init_data beagle_vmmc2 = {
>> + ? ? ? .constraints = {
>> + ? ? ? ? ? ? ? .valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> + ? ? ? },
>> + ? ? ? .num_consumer_supplies = 1,
>> + ? ? ? .consumer_supplies = &beagle_vmmc2_supply,
>> +};
>> +
>> +static struct fixed_voltage_config beagle_vwlan = {
>> + ? ? ? .supply_name = "vwl1271",
>> + ? ? ? .microvolts = 1800000, ?/* 1.8V */
>> + ? ? ? .gpio = OMAP_BEAGLE_WLAN_EN_GPIO,
>> + ? ? ? .startup_delay = 70000, /* 70ms */
>> + ? ? ? .enable_high = 1,
>> + ? ? ? .enabled_at_boot = 0,
>> + ? ? ? .init_data = &beagle_vmmc2,
>> +};
>
> We tabify all structures in board files, take a look at other structures.
>
> <snip>
>
>> diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
>> index 4134f44..8bb028e 100644
>> --- a/drivers/net/wireless/wl12xx/wl1271.h
>> +++ b/drivers/net/wireless/wl12xx/wl1271.h
>> @@ -60,7 +60,7 @@ enum {
>> ? ? ? ?DEBUG_ALL ? ? ? = ~0,
>> ?};
>>
>> -#define DEBUG_LEVEL (DEBUG_NONE)
>> +#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)
>
> I guess you didn't want that?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
Robert Nelson
http://www.rcn-ee.com/

2010-09-23 12:52:49

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, 2010-09-23 at 14:03 +0200, ext Robert Nelson wrote:
> On Thu, Sep 23, 2010 at 5:30 AM, Grazvydas Ignotas <[email protected]> wrote:
> > On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
> > <[email protected]> wrote:
> >> Add board configuration for the wl1271 daughter board. This patch is based
> >> on Ohad Ben-Cohen's patches for Zoom boards.
> >
> > Hm can that daughter board be detected? With your patch all beagle
> > users will get GPIO139 toggled, and if someone has that wired to
> > chainsaw switch somebody might get hurt.
>
> Expansion boards really need to follow:
>
> http://elinux.org/BeagleBoardPinMux#Expansion_boards
>
> Is there any eeprom on i2c bus #2 for identification on this board?

Hmmm... <checking the schematics>

Yes, it does. :) This makes perfect sense.

My bootloader (U-Boot 2010.03) doesn't seem to detect it, though:

<clip>
Probing for expansion boards, if none are connected you'll see a
harmless I2C error.

No EEPROM on expansion board
<clip>

Do I need a special bootloader?

Is there any standard way to recognize the expansion board and configure
it properly?

--
Cheers,
Luca.


2010-09-23 18:52:05

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, 2010-09-23 at 15:42 +0200, ext Robert Nelson wrote:
> On Thu, Sep 23, 2010 at 7:52 AM, Luciano Coelho
> <[email protected]> wrote:
> > On Thu, 2010-09-23 at 14:03 +0200, ext Robert Nelson wrote:
> >> On Thu, Sep 23, 2010 at 5:30 AM, Grazvydas Ignotas <[email protected]> wrote:
> >> > On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
> >> > <[email protected]> wrote:
> >> >> Add board configuration for the wl1271 daughter board. This patch is based
> >> >> on Ohad Ben-Cohen's patches for Zoom boards.
> >> >
> >> > Hm can that daughter board be detected? With your patch all beagle
> >> > users will get GPIO139 toggled, and if someone has that wired to
> >> > chainsaw switch somebody might get hurt.
> >>
> >> Expansion boards really need to follow:
> >>
> >> http://elinux.org/BeagleBoardPinMux#Expansion_boards
> >>
> >> Is there any eeprom on i2c bus #2 for identification on this board?
> >
> > Hmmm... <checking the schematics>
> >
> > Yes, it does. :) This makes perfect sense.
> >
> > My bootloader (U-Boot 2010.03) doesn't seem to detect it, though:
> >
> > <clip>
> > Probing for expansion boards, if none are connected you'll see a
> > harmless I2C error.
> >
> > No EEPROM on expansion board
> > <clip>
>
> I'd first add the board to the list on the wiki to protect the
> expansion board id.

Okay, I'll do that.


> Here's the current patch for u-boot for these expansion boards, it
> only implements id's for the boards listed at the time.
>
> http://www.sakoman.com/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=95993d1fee62ef64b2f58c1e186176ca9033c35e

Thanks for the pointer. Now I understand how this works.


> > Do I need a special bootloader?
> >
> > Is there any standard way to recognize the expansion board and configure
> > it properly?
>
> Yeap, you need a special bootloader, which is a downside to the
> current implementation... It relies on u-boot to do the i2c probing
> and detect which expansion board is connected, it would be nice if the
> kernel could do it on it's own..

Is there any technical problem which would prevent this from being done
in the kernel? Or is it just legacy and nobody has implemented it
properly in the kernel side?


> So currently u-boot probes, then notifies the kernel thru a "buddy"
> variable that gets passed with the bootargs..

I see. The good thing is that if someone is using a boot loader that is
not probing and passing the variable, it can be specified manually in
the kernel bootargs.


> board-omap3beagle.c then parse's the "buddy" variable to setup the
> expansion device, like as shown for the zippy1/2 expansion boards:
>
> http://cgit.openembedded.org/cgit.cgi/openembedded/tree/recipes/linux/linux-omap-psp-2.6.32/0007-ARM-OMAP-beagleboard-Add-infrastructure-to-do-fixups.patch
>
> (note there are patches applied before this and after, so it's won't
> apply cleanly to mainline)

Thanks for this pointer too. I see that the beagle board files are
indeed different from what is in the mainline. Any ideas if/when this
is going to be integrated into the mainline?


--
Cheers,
Luca.


2010-09-23 13:42:47

by Robert Nelson

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, Sep 23, 2010 at 7:52 AM, Luciano Coelho
<[email protected]> wrote:
> On Thu, 2010-09-23 at 14:03 +0200, ext Robert Nelson wrote:
>> On Thu, Sep 23, 2010 at 5:30 AM, Grazvydas Ignotas <[email protected]> wrote:
>> > On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
>> > <[email protected]> wrote:
>> >> Add board configuration for the wl1271 daughter board. ?This patch is based
>> >> on Ohad Ben-Cohen's patches for Zoom boards.
>> >
>> > Hm can that daughter board be detected? With your patch all beagle
>> > users will get GPIO139 toggled, and if someone has that wired to
>> > chainsaw switch somebody might get hurt.
>>
>> Expansion boards really need to follow:
>>
>> http://elinux.org/BeagleBoardPinMux#Expansion_boards
>>
>> Is there any eeprom on i2c bus #2 for identification on this board?
>
> Hmmm... <checking the schematics>
>
> Yes, it does. :) This makes perfect sense.
>
> My bootloader (U-Boot 2010.03) doesn't seem to detect it, though:
>
> <clip>
> Probing for expansion boards, if none are connected you'll see a
> harmless I2C error.
>
> No EEPROM on expansion board
> <clip>

I'd first add the board to the list on the wiki to protect the
expansion board id.

Here's the current patch for u-boot for these expansion boards, it
only implements id's for the boards listed at the time.

http://www.sakoman.com/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=95993d1fee62ef64b2f58c1e186176ca9033c35e

> Do I need a special bootloader?
>
> Is there any standard way to recognize the expansion board and configure
> it properly?

Yeap, you need a special bootloader, which is a downside to the
current implementation... It relies on u-boot to do the i2c probing
and detect which expansion board is connected, it would be nice if the
kernel could do it on it's own..

So currently u-boot probes, then notifies the kernel thru a "buddy"
variable that gets passed with the bootargs..

board-omap3beagle.c then parse's the "buddy" variable to setup the
expansion device, like as shown for the zippy1/2 expansion boards:

http://cgit.openembedded.org/cgit.cgi/openembedded/tree/recipes/linux/linux-omap-psp-2.6.32/0007-ARM-OMAP-beagleboard-Add-infrastructure-to-do-fixups.patch

(note there are patches applied before this and after, so it's won't
apply cleanly to mainline)

Regards,

--
Robert Nelson
http://www.rcn-ee.com/

2010-09-23 12:46:54

by Ohad Ben Cohen

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, Sep 23, 2010 at 2:20 PM, Felipe Balbi <[email protected]> wrote:
> Hi Luca,
>
> On Thu, Sep 23, 2010 at 07:17:33AM -0500, Luciano Coelho wrote:
>>
>> On Thu, 2010-09-23 at 13:45 +0200, ext Felipe Balbi wrote:
>>>
>>> On Thu, Sep 23, 2010 at 03:20:03AM -0500, Luciano Coelho wrote:
>>> > static struct mtd_partition omap3beagle_nand_partitions[] = {
>>> >@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)
>>> >
>>> > #include "sdram-micron-mt46h32m32lf-6.h"
>>> >
>>> >+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
>>> >+ ? ? ?.irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
>>>
>>> can you pass IRQ as a struct resource ?
>>
>> Sorry, I don't understand what you mean here. ?Can you clarify?
>
> static struct resource wl12xx_resource[] = {
> ? ? ? ?{
> ? ? ? ? ? ? ? ?.start ?= OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
> ? ? ? ? ? ? ? ?.flags ?= IORESOURCE_IRQ,
> ? ? ? ?},
> };
>
> static struct platform_device wl12xx_platform_device = {
> ? ? ? ?.resource ? ? ? = wl12xx_resource,
> ? ? ? ?.num_resources ?= ARRAY_SIZE(wl12xx_resource),
> ? ? ? ?.....
> };
>
> then on driver you would:
>
> irq = platform_get_irq(pdev, 0);

only problem is that there is no platform device involved here :)

>
> something like that :-)
>
> --
> balbi
>

2010-09-23 12:21:08

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Hi Luca,

On Thu, Sep 23, 2010 at 07:17:33AM -0500, Luciano Coelho wrote:
>On Thu, 2010-09-23 at 13:45 +0200, ext Felipe Balbi wrote:
>> On Thu, Sep 23, 2010 at 03:20:03AM -0500, Luciano Coelho wrote:
>> > static struct mtd_partition omap3beagle_nand_partitions[] = {
>> >@@ -163,12 +168,25 @@ static void __init beagle_display_init(void)
>> >
>> > #include "sdram-micron-mt46h32m32lf-6.h"
>> >
>> >+struct wl12xx_platform_data omap_beagle_wlan_data __initdata = {
>> >+ .irq = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
>>
>> can you pass IRQ as a struct resource ?
>
>Sorry, I don't understand what you mean here. Can you clarify?

static struct resource wl12xx_resource[] = {
{
.start = OMAP_GPIO_IRQ(OMAP_BEAGLE_WLAN_IRQ_GPIO),
.flags = IORESOURCE_IRQ,
},
};

static struct platform_device wl12xx_platform_device = {
.resource = wl12xx_resource,
.num_resources = ARRAY_SIZE(wl12xx_resource),
.....
};

then on driver you would:

irq = platform_get_irq(pdev, 0);

something like that :-)

--
balbi

2010-09-23 12:50:15

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

Hi,

On Thu, Sep 23, 2010 at 07:46:33AM -0500, Ohad Ben-Cohen wrote:
>> irq = platform_get_irq(pdev, 0);
>
>only problem is that there is no platform device involved here :)

heh, I thought the omap_beagle_vwlan_device was it, but it's only for
the regulator :-p

nevermind.

--
balbi

2010-09-23 11:56:47

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] omap: beagle: add support for wl1271 on the board file

On Thu, 2010-09-23 at 12:30 +0200, ext Grazvydas Ignotas wrote:
> On Thu, Sep 23, 2010 at 11:20 AM, Luciano Coelho
> <[email protected]> wrote:
> > Add board configuration for the wl1271 daughter board. This patch is based
> > on Ohad Ben-Cohen's patches for Zoom boards.
>
> Hm can that daughter board be detected? With your patch all beagle
> users will get GPIO139 toggled, and if someone has that wired to
> chainsaw switch somebody might get hurt.

Very good point. This was just me, working on my chainsaw-free bubble,
who didn't realize the danger of the code! :)

But actually I have no idea how to detect what kind of daughter board is
connected. Hopefully there is generic way of doing this in beagle
boards. Otherwise the only way I can think of is to add a Kconfig
option for this... :/


> > Cc: Ohad Ben-Cohen <[email protected]>
> > Signed-off-by: Luciano Coelho <[email protected]>
> > ---
> > There was a useless variable defined in omap3_beagle_init() that was causing a
> > warning. I have removed it in v2.
> >
> > arch/arm/mach-omap2/board-omap3beagle.c | 69 +++++++++++++++++++++++++++++++
> > drivers/net/wireless/wl12xx/wl1271.h | 2 +-
> > 2 files changed, 70 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> > index 87969c7..755df29 100644
> > --- a/arch/arm/mach-omap2/board-omap3beagle.c
> > +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>
> <snip>
>
> > +static struct regulator_consumer_supply beagle_vmmc2_supply = {
> > + .supply = "vmmc",
> > + .dev_name = "mmci-omap-hs.1",
> > +};
> > +
> > +
>
> single newline is enough.

I'll fix it.


> > static struct regulator_consumer_supply beagle_vsim_supply = {
> > .supply = "vmmc_aux",
> > };
> >
> > +
>
> here too.

Here too.


> > +static struct regulator_init_data beagle_vmmc2 = {
> > + .constraints = {
> > + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > + },
> > + .num_consumer_supplies = 1,
> > + .consumer_supplies = &beagle_vmmc2_supply,
> > +};
> > +
> > +static struct fixed_voltage_config beagle_vwlan = {
> > + .supply_name = "vwl1271",
> > + .microvolts = 1800000, /* 1.8V */
> > + .gpio = OMAP_BEAGLE_WLAN_EN_GPIO,
> > + .startup_delay = 70000, /* 70ms */
> > + .enable_high = 1,
> > + .enabled_at_boot = 0,
> > + .init_data = &beagle_vmmc2,
> > +};
>
> We tabify all structures in board files, take a look at other structures.

Sure, I'll do that. I just copied this from a previous patch to which
someone commented the same thing. I just forgot to fix it before
submitting, because I got so excited when it worked :)


> > diff --git a/drivers/net/wireless/wl12xx/wl1271.h b/drivers/net/wireless/wl12xx/wl1271.h
> > index 4134f44..8bb028e 100644
> > --- a/drivers/net/wireless/wl12xx/wl1271.h
> > +++ b/drivers/net/wireless/wl12xx/wl1271.h
> > @@ -60,7 +60,7 @@ enum {
> > DEBUG_ALL = ~0,
> > };
> >
> > -#define DEBUG_LEVEL (DEBUG_NONE)
> > +#define DEBUG_LEVEL (DEBUG_MAC80211 | DEBUG_CMD | DEBUG_ACX | DEBUG_BOOT)
>
> I guess you didn't want that?

No, I certainly didn't. I have already sent a v3 without that.


--
Cheers,
Luca.