[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20090115225532.8491d671.akpm@linux-foundation.org>
Date: Thu, 15 Jan 2009 22:55:32 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: arhuaco@...aks-unidos.net
Cc: linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
andy@...nmoko.com
Subject: Re: [PATCH 1/5] Add touchscreen filter API
On Fri, 16 Jan 2009 00:05:20 -0500 "Nelson Castillo" <arhuaco@...aks-unidos.net> wrote:
> On Thu, Jan 15, 2009 at 6:47 PM, Andrew Morton
> <akpm@...ux-foundation.org> wrote:
> > On Tue, 13 Jan 2009 18:39:39 -0500
> > Nelson <arhuaco@...aks-unidos.net> wrote:
> >
> >> From: Nelson Castillo <arhuaco@...aks-unidos.net>
> >>
> >> Generic filters are not useful by themselves. They define an API that
> >> actual filters have to implement.
> >>
> >> Signed-off-by: Nelson Castillo <arhuaco@...aks-unidos.net>
> >> ---
>
> (cut)
>
> >> +int ts_filter_create_chain(struct platform_device *pdev,
> >> + struct ts_filter_api **api, void **config,
> >> + struct ts_filter **list, int count_coords)
> >> +{
> >> + int count = 0;
> >> + struct ts_filter *last = NULL;
> >> +
> >> + if (!api)
> >> + return 0;
> >> +
> >> + while (*api && count < MAX_TS_FILTER_CHAIN) {
> >
> >Why does MAX_TS_FILTER_CHAIN exist? Why impose limits?
>
> "api" is actually a static array that is not zero_terminated by
> convention. It can be zero-terminated, though. This check avoids
> overrunning the loop if MAX_TS_FILTER_CHAIN filters are used.
>
> This snippet is taken from mach-gta02.c (link below) and
> "filter_sequence" is what
> we pass as the "api" array.
>
> static struct s3c2410_ts_mach_info gta02_ts_cfg = {
> .delay = 10000,
> .presc = 0xff, /* slow as we can go */
> .filter_sequence = {
> [0] = &ts_filter_group_api,
> [1] = &ts_filter_median_api,
> [2] = &ts_filter_mean_api,
> [3] = &ts_filter_linear_api,
> },
> .filter_config = {
> [0] = >a02_ts_group_config,
> [1] = >a02_ts_median_config,
> [2] = >a02_ts_mean_config,
> [3] = >a02_ts_linear_config,
> },
> };
so... change the interface to require that the array be
null-terminated, then fix callers.
Null-terminating variable-length arrays in this manner is a quite
common paradigm in the kernel.
> >> diff --git a/drivers/input/touchscreen/ts_filter.c b/drivers/input/touchscreen/ts_filter.c
> >> new file mode 100644
> >> index 0000000..1508388
> >> --- /dev/null
> >> +++ b/drivers/input/touchscreen/ts_filter.c
> >
> > Confused. Nothing appears to use the two functions which this file adds.
>
> http://git.openmoko.org/?p=kernel.git;a=shortlog;h=stable-tracking
>
> This branch is tracking the upstream Linux kernel and patches are
> rebased/updated
> when things change upstream. Openmoko has been doing this since one
> goal is to be able to use upstream kernels with no patches for OM
> devices.
>
> This is the driver can use the filters (s3c2410_ts.c) if they are
> defined in the machine configuration:
>
> http://git.openmoko.org/?p=kernel.git;a=blob_plain;f=drivers/input/touchscreen/s3c2410_ts.c;hb=stable-tracking
>
> We are ready to send this driver upstream but before we need to trim
> this file (mach-gta02.c) and send it:
>
> http://git.openmoko.org/?p=kernel.git;a=blob_plain;f=arch/arm/mach-s3c2440/mach-gta02.c;hb=stable-tracking
>
> We are depending on this mach-gta02.c file now and a streamlined
> version of it must be sent upstream so that we can start adding
> drivers. This file in turn depends on patches that are being sent
> upstream also.
>
> Thus we have a chicken and egg problem and we decided to send the
> filters for review because they are in the stable-tracking branch and
> they need to be suitable for upstream inclusion / included upstream.
Please hold off on the unused code until some code which actually uses
is is submitted as well.
--
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/
Powered by blists - more mailing lists