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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <483a38b80905142132q7642c57w1d12812bda6f845@mail.gmail.com>
Date:	Fri, 15 May 2009 13:32:43 +0900
From:	Kwangwoo Lee <kwangwoo.lee@...il.com>
To:	Manuel Lauss <mano@...rinelk.homelinux.net>
Cc:	Trilok Soni <soni.trilok@...il.com>,
	Kwangwoo Lee <kwlee@...kvision.com>,
	linux-kernel@...r.kernel.org,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Subject: Re: [PATCH] TSC2007: private data for platform callbacks.

On Thu, May 14, 2009 at 7:37 PM, Manuel Lauss
<mano@...rinelk.homelinux.net> 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@...rinelk.homelinux.net> 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@...r.kernel.org
> 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@...il.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/