Hi Sekhar,

On Mon, Jul 25, 2011 at 11:10:55PM +0530, Nori, Sekhar wrote:

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

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

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

quoted

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

You can get rid of these NULL initializers.

Sure.

quoted

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

[...]

quoted

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

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

Sure.

Thanks for your review,
Ido.