Re: [PATCH] TSC2007: private data for platform callbacks.

[Kwangwoo Lee]

On Thu, May 14, 2009 at 7:37 PM, Manuel Lauss
<mano@xxxxxxxxxxxxxxxxxxxxxxx> 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
>> <mano@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
>> > Add a private data field to the tsc2007 platform data and pass this
>> > to the callbacks.
>>
>> I hope that you have created this patch on top of two recently posted
>> 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)
>>           ret =  ts->pd->get_pendown_state(ts->pd->priv);
>
> 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.  All 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 = pdata = client->dev.platform_data;
>> > + ? ? ? struct tsc2007_platform_data *pdata = client->dev.platform_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!
>        Manuel Lauss
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Thanks,

-- 
Kwangwoo Lee <kwangwoo.lee@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/