> -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > owner@xxxxxxxxxxxxxxx] On Behalf Of Reizer, Eyal > Sent: Thursday, January 27, 2011 3:20 PM > To: linux-omap@xxxxxxxxxxxxxxx > Subject: [PATCH] omap: omap3evm: add support for the WL12xx WLAN module to > the AM/DM3xx Evaluation Module [sp] prefix "omap:" seems redundant when "omap3evm" is specified. Similarly, " the AM/DM3xx Evaluation Module" seems redundant when "omap3evm" is specified. > > Adds platform initialization for working with the WLAN module attached to > the evaluation module. > The patch includes MMC2 initialization, SDIO and control pins muxing and > platform device registration [sp] The description text has too many characters per line. Consider reformatting the same. > > Signed-off-by: Eyal Reizer <eyalr@xxxxxx> > --- > arch/arm/mach-omap2/board-omap3evm.c | 78 > ++++++++++++++++++++++++++++++++++ > 1 files changed, 78 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- > omap2/board-omap3evm.c > index 323c380..2d2092a 100644 > --- a/arch/arm/mach-omap2/board-omap3evm.c > +++ b/arch/arm/mach-omap2/board-omap3evm.c > @@ -30,6 +30,8 @@ > #include <linux/usb/otg.h> > #include <linux/smsc911x.h> > > +#include <linux/wl12xx.h> > +#include <linux/regulator/fixed.h> > #include <linux/regulator/machine.h> > #include <linux/mmc/host.h> > > @@ -381,6 +383,16 @@ static struct omap2_hsmmc_info mmc[] = { > .gpio_cd = -EINVAL, > .gpio_wp = 63, > }, > +#ifdef CONFIG_WL12XX_PLATFORM_DATA [sp] Haven't seen this config option earlier. Just wanted to know where is this defined? Any reason why simple CONFIG_WL12XX wasn't sufficient? > + { > + .name = "wl1271", > + .mmc = 2, [sp] There are spaces before "=". Stands apart from rest of the assignments in this block. > + .caps = MMC_CAP_4_BIT_DATA | MMC_CAP_POWER_OFF_CARD, > + .gpio_wp = -EINVAL, > + .gpio_cd = -EINVAL, > + .nonremovable = true, > + }, > +#endif > {} /* Terminator */ > }; > > @@ -538,6 +550,50 @@ static struct regulator_init_data omap3_evm_vpll2 = { > .consumer_supplies = &omap3_evm_vpll2_supply, > }; > > +#ifdef CONFIG_WL12XX_PLATFORM_DATA > + > +#define OMAP3EVM_WLAN_PMENA_GPIO (150) > +#define OMAP3EVM_WLAN_IRQ_GPIO (149) > + > +static struct regulator_consumer_supply omap3evm_vmmc2_supply = { > + .supply = "vmmc", > + .dev_name = "mmci-omap-hs.1", > +}; [sp] Consider use of macro REGULATOR_SUPPLY() here. > + > +/* VMMC2 for driving the WL12xx module */ > +static struct regulator_init_data omap3evm_vmmc2 = { [sp] Consider following the convention is rest of the file e.g. omap3_evm_vdac, omap3_evm_vpll2, omap3_evm_vusb. I see another exception to this convention; in the current file but that should be fixed. > + .constraints = { > + .valid_ops_mask = REGULATOR_CHANGE_STATUS, [sp] Any specific reason for not including REGULATOR_CHANGE_MODE in the mask above? > + }, > + .num_consumer_supplies = 1, > + .consumer_supplies = &omap3evm_vmmc2_supply, > +}; > + > +static struct fixed_voltage_config omap3evm_vwlan = { > + .supply_name = "vwl1271", > + .microvolts = 1800000, /* 1.80V */ > + .gpio = OMAP3EVM_WLAN_PMENA_GPIO, > + .startup_delay = 70000, /* 70ms */ > + .enable_high = 1, > + .enabled_at_boot = 0, > + .init_data = &omap3evm_vmmc2, > +}; > + > +static struct platform_device omap3evm_vwlan_device = { > + .name = "reg-fixed-voltage", > + .id = 1, > + .dev = { > + .platform_data = &omap3evm_vwlan, > + }, > +}; [sp] The name of platform device here apears to be name of regulator. Otherwise, should you explain why it is named "reg-fixed-voltage". > + > +struct wl12xx_platform_data omap3evm_wlan_data __initdata = { > + .irq = OMAP_GPIO_IRQ(OMAP3EVM_WLAN_IRQ_GPIO), > + /* ref clock is 38.4 MHz */ > + .board_ref_clock = 2, [sp] Suggest putting the comment on ref clock after the assignment. > +}; > +#endif > + > static struct twl4030_platform_data omap3evm_twldata = { > .irq_base = TWL4030_IRQ_BASE, > .irq_end = TWL4030_IRQ_END, > @@ -658,6 +714,21 @@ static struct omap_board_mux board_mux[] __initdata = > { > OMAP_PIN_OFF_WAKEUPENABLE), > OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP | > OMAP_PIN_OFF_INPUT_PULLUP | > OMAP_PIN_OFF_OUTPUT_LOW), > +#ifdef CONFIG_WL12XX_PLATFORM_DATA > + /* WLAN IRQ - GPIO 149 */ > + OMAP3_MUX(UART1_RTS, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP), > + > + /* WLAN POWER ENABLE - GPIO 150 */ > + OMAP3_MUX(UART1_CTS, OMAP_MUX_MODE4 | OMAP_PIN_OUTPUT), > + > + /* MMC2 SDIO pin muxes for WL12xx */ > + OMAP3_MUX(SDMMC2_CLK, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP), > + OMAP3_MUX(SDMMC2_CMD, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP), > + 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), [sp] What about the MUX settings corresponding to OFF mode? OR, Do you believe that these are sufficient for WLAN to work with OFF mode enabled. > +#endif > { .reg_offset = OMAP_MUX_TERMINATOR }, > }; > #endif > @@ -715,6 +786,13 @@ static void __init omap3_evm_init(void) > ads7846_dev_init(); > omap3evm_init_smsc911x(); > omap3_evm_display_init(); > + > +#ifdef CONFIG_WL12XX_PLATFORM_DATA > + /* WL12xx WLAN Init */ > + if (wl12xx_set_platform_data(&omap3evm_wlan_data)) > + pr_err("error setting wl12xx data\n"); > + platform_device_register(&omap3evm_vwlan_device); > +#endif > } > > MACHINE_START(OMAP3EVM, "OMAP3 EVM") > -- > 1.7.0.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
