lkml.org 
[lkml]   [2004]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [ANNOUNCE] new driver for teletext decoder SAA5246A
From
On Wed, Feb 25, 2004 at 04:13:30AM -0800, Andrew Morton wrote:
> linux@MichaelGeng.de (Michael Geng) wrote:
> >
> > http://www.michaelgeng.de/linux/saa5246a-2.6.3.patch

Location of the updated patch:

http://www.michaelgeng.de/linux/saa5246a-rev1-2.6.3.patch

> jdelay() is odd - it's a shame it has to fiddle with signals in that
> manner. Are you sure it's really needed?

I copied this jdelay() stuff from the existing saa5249.c
without much reflecting about it.

In my opinion it is up to the user space program to request the
driver status in reasonable time intervals in order to see if a
requested videotext page has been received. The kernel driver
should answer without delay. It can't be the task of the kernel
driver to prevent a user space program from querying the status
10000 times a second.

Conclusion: You are right, the jdelay() stuff is not necessary
at all. I removed it completely from the code. After that I
tested the newly compiled driver using Martin Buck's vtxget
program. The command

time vtxget 100

consumed about 0.4secs sys time before and about 0.5secs after
removing jdelay(). I think this is ok, the driver still works
fine.

> Also, I suggest you try removing all those `inline' statements. Then see
> if /usr/bin/size claims that the code shrunk. Often it does.

No, it doesn't shrink. I already tested that when I developed
the driver with saa5249.c as a template. Now I made a completely
inline-free version, but /usr/bin/size shows exactly the same
size. By the way, this shows that gcc does a good job. Of course
this could also depend on the architecture, but on my Pentium 4
box gcc really puts the inline parts inline.

There are 2 reason why I used these inline functions:

1. A huge ioctl() function would be more difficult to understand
than those individual inline functions. Don't you agree?

2. Some of those inline functions are not only specific to the
SAA5246A. They could also be used for the existing driver to
the SAA5249 or other drivers of that series. Maybe this could
be useful if someone wants to write another driver or even
in order to merge the SAA5246A and SAA5249 drivers into 1
driver. Maybe this is possible, I'm not sure. I would
like to try, but unfortunately I don't have a SAA5249. Does
anybody want to sell his sample to me?

Conclusion: I would prefer to keep all those inline functions.
The inline functions are not changed in the updated patch.

> Apart from that, looks fine. Please send me a copy via email along with a
> suitable changelog for checking into Bitkeeper and copy Gerd Knorr
> <kraxel@bytesex.org>, thanks.

If you want to add the patch, how about the following changelog:

[V4L]: Added new driver for Teletext decoder SAA5246A from Philips

Thank you very much for your help,
Michael

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

\
 
 \ /
  Last update: 2005-03-22 14:01    [W:0.070 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site