Re: [PATCH v2 6/6] arm: davinci: DA850: Add wl1271/wlan support

From: Harishkumar V <harishpresent-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Ido Yariv <ido-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>
Cc: "davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org"
	<davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org>,
	Russell King - ARM Linux
	<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Nicolas Pitre <nico-vtqb6HGKxmzR7s880joybQ@public.gmane.org>,
	"linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH v2 6/6] arm: davinci: DA850: Add wl1271/wlan support
Date: Mon, 22 Aug 2011 13:07:39 +0530	[thread overview]
Message-ID: <CANDx4kUXRaCAnYxBOY32tZjcWsrAJG4oYi1YiTxAz5f94Gwe6g@mail.gmail.com> (raw)
In-Reply-To: <20110728183454.GE1985@WorkStation>

[-- Attachment #1.1: Type: text/plain, Size: 2750 bytes --]

Hi,

to which davinci git repository this is uploaded, i am using
http://gitorious.org/linux-davinci, I cannot find it board-da850-evm.c.

>From where to pull this repository.

On Fri, Jul 29, 2011 at 12:04 AM, Ido Yariv <ido-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org> wrote:

> 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.
>
> > > +static struct davinci_mmc_config da850_mmc_wl12xx_config = {
> > > +   .get_ro         = NULL,
> > > +   .get_cd         = NULL,
> >
> > You can get rid of these NULL initializers.
>
> Sure.
>
> > > +   .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,
> > > +};
>
> [...]
>
> > > +   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.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

-- 
Thanks and Regards,
Harish Kumar. V

[-- Attachment #1.2: Type: text/html, Size: 3891 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]