From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kwangwoo Lee Subject: Re: [PATCH] TSC2007: private data for platform callbacks. Date: Fri, 15 May 2009 13:32:43 +0900 Message-ID: <483a38b80905142132q7642c57w1d12812bda6f845@mail.gmail.com> References: <20090514100909.GA23626@roarinelk.homelinux.net> <5d5443650905140328v319e673o2f7b865e21619486@mail.gmail.com> <20090514103731.GA23814@roarinelk.homelinux.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20090514103731.GA23814@roarinelk.homelinux.net> Sender: linux-kernel-owner@vger.kernel.org To: Manuel Lauss Cc: Trilok Soni , Kwangwoo Lee , linux-kernel@vger.kernel.org, "linux-input@vger.kernel.org" List-Id: linux-input@vger.kernel.org On Thu, May 14, 2009 at 7:37 PM, Manuel Lauss wrote: > Hi Trilok, > > On Thu, May 14, 2009 at 03:58:19PM +0530, Trilok Soni wrote: >> CCing linux-input mailing list, so not deleting any code from this >> e-mail while replying. >> >> On Thu, May 14, 2009 at 3:39 PM, Manuel Lauss >> wrote: >> > Add a private data field to the tsc2007 platform data and pass thi= s >> > to the callbacks. >> >> I hope that you have created this patch on top of two recently poste= d >> patches by Kwangwoo Lee >> to merge changes from Thierry. > > No; it's against Linux's git. Will rediff. > > > >> > - ? ? ? int ? ? ? ? ? ? ? ? ? ? (*get_pendown_state)(void); >> > - ? ? ? void ? ? ? ? ? ? ? ? ? ?(*clear_penirq)(void); >> > + ? ? ? struct tsc2007_platform_data *pd; >> > ?}; >> > >> > +/* ask platform for pendown state */ >> > +static inline int ts_get_pendown_state(struct tsc2007 *ts) >> > +{ >> > + ? ? ? return ts->pd->get_pendown_state(ts->pd->priv); >> >> So, we don't need check here, like this: >> >> if (ts->pd->get_pendown_state) >> =A0 =A0 =A0 =A0 =A0 ret =3D =A0ts->pd->get_pendown_state(ts->pd->pri= v); > > The driver doesn't work right without that callback so the check > in here is kind of pointless. Better to check for it in the > probe function and refuse to probe if it isn't there. =A0All other > callbacks seem optional to me. I agree. This function should be checked in the probe one. > > >> > ? ? ? ?struct tsc2007 *ts; >> > - ? ? ? struct tsc2007_platform_data *pdata =3D pdata =3D client->= dev.platform_data; >> > + ? ? ? struct tsc2007_platform_data *pdata =3D client->dev.platfo= rm_data; >> >> This change shows that you need to rebase based on two patches >> submitted by Lee recently. > > Ah, that shouldn't be in there in the first place. This should be fixed. > > Thanks! > =A0 =A0 =A0 =A0Manuel Lauss > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kerne= l" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at =A0http://vger.kernel.org/majordomo-info.html > Please read the FAQ at =A0http://www.tux.org/lkml/ > Thanks, --=20 Kwangwoo Lee