From mboxrd@z Thu Jan 1 00:00:00 1970 From: luciano.coelho@nokia.com (Luciano Coelho) Date: Thu, 16 Sep 2010 22:56:51 +0300 Subject: [PATCH v6 2/7] wl1271: propagate set_power's return value In-Reply-To: References: <1284592929-29616-1-git-send-email-ohad@wizery.com> <1284592929-29616-3-git-send-email-ohad@wizery.com> <1284666018.8951.66.camel@powerslave> Message-ID: <1284667011.8951.73.camel@powerslave> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, 2010-09-16 at 21:53 +0200, ext Ohad Ben-Cohen wrote: > On Thu, Sep 16, 2010 at 9:40 PM, Luciano Coelho > wrote: > >> + int ret = wl->if_ops->power(wl, true); > > > > I think it look nicer if you keep the "int ret" in one line by itself > > and then do a ret = wl->if_ops... on another one. > > Fixed. > > >> +static int wl1271_sdio_power_on(struct wl1271 *wl) > >> { > >> struct sdio_func *func = wl_to_func(wl); > >> > >> sdio_claim_host(func); > >> sdio_enable_func(func); > >> sdio_release_host(func); > >> + > >> + return 0; > >> } > > > > You seem to always return 0, so the whole chain to pass the value up > > seems unnecessary. Is this just a preparation for a future patch? > > Yes, it's soon going to be: > > static int wl1271_sdio_power_on(struct wl1271 *wl) > { > struct sdio_func *func = wl_to_func(wl); > int ret; > > ret = pm_runtime_get_sync(&func->dev); > if (ret) > goto out; > > sdio_claim_host(func); > sdio_enable_func(func); > sdio_release_host(func); > > out: > return ret; > } > Ok, that was the only explanation I could think of ;) Acked-by: Luciano Coelho -- Cheers, Luca.