Hello all,
thanks for all the suggestions regarding the dvb code, this is now my
2nd try... ;-)
Again, it's a patchset of 9 patches, which tries to sync the linuxtv.org
CVS with the kernel driver.
Due to the size of some of the patches, I don't post them on the list.
Please have a look at them at:
http://www.gdv.uni-hannover.de/~hunold1/dvb/
Below is a summary of what these patches actually do. I tried to
preserve everything that wasn't changed through the linuxtv.org's CVS,
so I hope I did not wipe something out again.
I understand that this is a big load Linus will most likely refuse to
merge at once. So I'd like to ask other users to have a look at these
patches and ask the maintainers (Christoph Hellwig, Alan Cox) to
actually do the merge.
@ Christoph Hellwig:
I hope I followed all your suggestions. These were:
- remove the DVB_DEVFS_ONLY completly
- remove all #ifdef LINUX_KERNEL magic
- remove all *internal* typedefs for structs and enums
- use c99 initializers
- use linux/errno.h instead of asm/errno.h
- follow the new devfs api
Still left:
- fix up dprintk() usage
CU
Michael.
--------------------------------------------------------------------------------
[1-09] update the firmware of the av7110 dvb driver
[2-09] update the generic saa7146 driver
- remove some #if LINUX_VERSION_CODE constructions
- sync with the interrupt handler changes in 2.5.69
- add a missing kfree() call which caused the kernel to
leak 32kB of kmalloc()ed memory. iieek!
- fixed the capture code to handle cards that have swapped fields
- added and fixed some debug messages
- changed from kmalloc() to pci_consistent()
- many small changes necessary to fix warnings/problems
when compiled for ppc64 for example
[3-09] update dvb subsystem core
- switched from user-land types like __u8 to u8 and uint16_t to u16
this makes the patch rather large.
- updated the dvr (digital videorecording) facility
- renamed some structures, like "struct dmxdev_s" to "struct dmxdev"
- introduced dvb_functions.[ch], where some linux-kernel specific
functions are encapsulated. by this, the dvb subsystem stays quite
independent from deeper linux kernel functions.
- moved dvb_usercopy() to dvb_functions.c -- this is essentially
video_usercopy() which should be generic_usercopy() instead...
- Made the dvb-core in dvbdev.c work with devfs again.
- remove all typedefs from structs
- remove all typedefs from enums
[4-09] update the av7110 and budget drivers
- replaced ddelay() wait function with generic dvb_delay() implementation
- new DATA_MPEG_VIDEO_EVENT for direct mpeg2 video playback
- added support for DVB-C cards with MSP3400 mixer and analog tuner
- fixed up the av7110_ir handler and especially the write_proc()
function; this fixed the bug the Stanford Checker has found
[5-09] update dvb frontend drivers
- C99 initializers
- fix up some includes
- various bugfixes
[6-09] add a new dvb frontend driver
- add a new driver for the cx24110 frontend by Peter Hettkamp
<[email protected]>
[7-09] add dvb subsystem as a crc32 lib user
[8-09] update analog saa7146 drivers mxb and dpc7146
- add MODULE_DEVICE_TABLE entries, so that /sbin/hotplug can handle the
devices
- fixup due to the latest i2c changes
[9-09] correct the i2c address of the saa7111
- corrects the i2c address from "34>>1" to 0x24 and 0x25. Believe me --
or look at the data sheet, for example from
http://www.gdv.uni-hannover.de/~hunold1/linux/saa7146/specs/saa7111a.pdf
Page 41 says: "Slave address read = 49H or 4BH; note 2 write = 48H or
4AH" They use 8-bit addresses here, but i2c addresses are 7-bit,
ie. 0x48>>1 == 0x24 and 0x4a>>1 = 0x25
--------------------------------------------------------------------------------
01-av7110-firware.diff
- looks fine (obsiously :)). If the old driver works with the
new firware I'd suggets sending it to Linux now
02-saa7146-core.diff
- the WRITE_RPS0 macro is ugly as hell, you probably want to
replace it with a proper inline. But you can leave this to
a later patch. If it doesn't need the other updates I'd
suggest submitting it now.
03-dvb-core.diff
- okay, this is a big one.. Could you submit the typedef removal
as a first separate patch so the actual changes are reviewable
more easily?
- please use <linux/types.h> not <asm/types.h> everywhere
- please include <asm/*.h> headers after <linux/*.h> ones
- This is wrong:
-static struct dvb_device dvbdev_dvr = {
+static
+struct dvb_device dvbdev_dvr = {
instead of breaking the indention rather fix the reamining parts
of the driver
- dvb_kernel_thread_setup doesn't need the BKL
04-dvb-drivers.diff
- looks ok
05-update-frontends.diff
- looks ok
06-new-frontend.diff
- looks ok (except for the above mentioned indentation issues)
07-dvb-core-lib-user.diff
- looks ok (obviously)
08-analog-saa7146-update.diff
- looks ok (obviously)
09-saa7111-i2c-fix.diff
- looks ok (obviously)
Hello Christoph,
> - looks fine (obsiously :)). If the old driver works with the
> new firware I'd suggets sending it to Linux now
Yes, it should. It has some extended features, which are only visible
through new ioctl, though.
> 02-saa7146-core.diff
>
> - the WRITE_RPS0 macro is ugly as hell, you probably want to
> replace it with a proper inline. But you can leave this to
> a later patch. If it doesn't need the other updates I'd
> suggest submitting it now.
I'd leave this to a later patch.
> 03-dvb-core.diff
>
> - okay, this is a big one.. Could you submit the typedef removal
> as a first separate patch so the actual changes are reviewable
> more easily?
That's very difficult. I just had a look through the cvs changelogs and
talked to a collegue: apparently, there haven't been any important
bugfixes to the code -- only the major cleanups to get the code into
"kernel shape" (typdef removals, use 16 instead of uint16_t, ...)
It would cause me a major pain to separate these diffs, because I did
not tag the code very often. (my fault, I know)
The code just "works" for the dvb-subsystem, so I hope there is nothing
to worry about for other coders.
> - please use <linux/types.h> not <asm/types.h> everywhere
> - please include <asm/*.h> headers after <linux/*.h> ones
Ok, comes with the next patch.
> - This is wrong:
> -static struct dvb_device dvbdev_dvr = {
> +static
> +struct dvb_device dvbdev_dvr = {
> instead of breaking the indention rather fix the reamining parts
> of the driver
It's already hard enough to make all these changes, make sure everything
compiles and works for both 2.4 and 2.5, then create the patches and ...
get rejected. 8-)
If there aren't any other objections, I'd like to ask to apply these
patches.
The "kernel thread uses BKL" issue works for us, but can reviewed by the
responsible author of course.
CU
Michael.
On Wed, May 28, 2003 at 02:54:04PM +0200, Michael Hunold wrote:
> It's already hard enough to make all these changes, make sure everything
> compiles and works for both 2.4 and 2.5, then create the patches and ...
> get rejected. 8-)
>
> If there aren't any other objections, I'd like to ask to apply these
> patches.
Just add the fixes as incremental patches to your patchkit.
Hello Linus, Alan, Christoph,
again I hope to get the LinuxTV.org CVS "dvb-kernel" driver in
sync with the latest 2.5 kernel.
The main project page is http://www.linuxtv.org.
It's a patchset of 10 patches -- due to the size of some of the
patches, I don't post them on the list again.
Please have a look at them at:
http://www.gdv.uni-hannover.de/~hunold1/dvb/
I fixed my last patchset, so that it applies to 2.5.72 and added
a new patch that hopefully clean up the things according to the
comments on kernel mailing list (mainly by Christoph Hellwig).
The patch "10-clean-up.diff" is new and tries to fix the stuff
that was broken before or that I have broken with patches 1 - 9.
I'd like to ask you one last time to accept this patchset
as one big blob. It does not affect other subsystems beside /media/video
and /media/dvb, so it won't clash with other changes at all.
If there still are issues, then I can fix them up with a later patchset.
Thanks
Michael Hunold.
--------------------------------------------------------------------------
Below are the main comments for the last patchset:
> 01-av7110-firware.diff
>
> - looks fine (obsiously :)). If the old driver works with the
> new firware I'd suggets sending it to Linux now
Yes, the firmware would work with the old driver as well. So this
one can safely applied.
> 02-saa7146-core.diff
>
> - the WRITE_RPS0 macro is ugly as hell, you probably want to
> replace it with a proper inline. But you can leave this to
> a later patch. If it doesn't need the other updates I'd
> suggest submitting it now.
Fixed.
> 03-dvb-core.diff
>
> - okay, this is a big one.. Could you submit the typedef removal
> as a first separate patch so thed actual changes are reviewable
> more easily?
I'm very sorry, but as I already wrote to you (Christoph),
this is hardly possible, because there has been such a long time
beetwen the last patchset, the typedef changes and this new patchset.
> - please use <linux/types.h> not <asm/types.h> everywhere
> - please include <asm/*.h> headers after <linux/*.h> ones
> - This is wrong:
> -static struct dvb_device dvbdev_dvr = {
> +static
> +struct dvb_device dvbdev_dvr = {
> instead of breaking the indention rather fix the reamining parts
> of the driver
Fixed.
> - dvb_kernel_thread_setup doesn't need the BKL
>
> 04-dvb-drivers.diff
>
> - looks ok
>
> 05-update-frontends.diff
>
> - looks ok
>
> 06-new-frontend.diff
>
> - looks ok (except for the above mentioned indentation issues)
>
> 07-dvb-core-lib-user.diff
>
> - looks ok (obviously)
>
> 08-analog-saa7146-update.diff
>
> - looks ok (obviously)
>
> 09-saa7111-i2c-fix.diff
>
> - looks ok (obviously)
Fixed the indentation in many other files as well.
--------------------------------------------------------------------------
This is the README for both the old patches and the new patch:
[1-10] update the firmware of the av7110 dvb driver
[2-10] update the generic saa7146 driver
- remove some #if LINUX_VERSION_CODE constructions
- sync with the interrupt handler changes in 2.5.69
- add a missing kfree() call which caused the kernel to
leak 32kB of kmalloc()ed memory. iieek!
- fixed the capture code to handle cards that have swapped
field order (odd and even fields)
- added and fixed some debug messages
- changed from kmalloc() to pci_consistent()
- many small changes necessary to fix warnings/problems
when compiled for ppc64 for example
[3-10] update dvb subsystem core
- switched from user-land types like __u8 to u8 and uint16_t to u16
this makes the patch rather large.
- updated the dvr (digital videorecording) facility
- renamed some structures, like "struct dmxdev_s" to "struct dmxdev"
- introduced dvb_functions.[ch], where some linux-kernel specific
functions are encapsulated. by this, the dvb subsystem stays quite
independent from deeper linux kernel functions.
- moved dvb_usercopy() to dvb_functions.c -- this is essentially
video_usercopy() which should be generic_usercopy() instead...
- Made the dvb-core in dvbdev.c work with devfs again.
- remove all typedefs from structs
- remove all typedefs from enums
[4-10] update the av7110 and budget drivers
- replaced ddelay() wait function with generic dvb_delay() implementation
- new DATA_MPEG_VIDEO_EVENT for direct mpeg2 video playback
- added support for DVB-C cards with MSP3400 mixer and analog tuner
- fixed up the av7110_ir handler and especially the write_proc() function; this
fixed the bug the Stanford Checker has found
[5-10] update dvb frontend drivers
- C99 initializers
- fix up some includes
- various bugfixes
[6-10] add a new dvb frontend driver
- add a new driver for the cx24110 frontend by Peter Hettkamp
<[email protected]>
[7-10] add dvb subsystem as a crc32 lib user
[8-10] update analog saa7146 drivers mxb and dpc7146
- add MODULE_DEVICE_TABLE entries, so that /sbin/hotplug can handle the devices
- fixup due to the latest i2c changes
[9-10] correct the i2c address of the saa7111
- corrects the i2c address from "34>>1" to 0x24 and 0x25. Believe me -- or
look at the data sheet, for example from
http://www.gdv.uni-hannover.de/~hunold1/linux/saa7146/specs/saa7111a.pdf
Page 41 says: "Slave address read = 49H or 4BH; note 2 write = 48H or 4AH"
They use 8-bit addresses here, but i2c addresses are 7-bit,
ie. 0x48>>1 == 0x24 and 0x4a>>1 = 0x25
[10-10] clean up the parts according to the comments on kernel mailing list
(mainly by Christoph Hellwig)
- ugly WRITE_RPS0 define in saa7146_hlp.c has been replaced by a proper inline (I hope)
- use <linux/types.h> not <asm/types.h> everywhere
- include <asm/*.h> headers after <linux/*.h> ones
- revert the indentation from "static <newline> xxx to "static xxx"
On Wed, 18 Jun 2003, Michael Hunold wrote:
>
> again I hope to get the LinuxTV.org CVS "dvb-kernel" driver in
> sync with the latest 2.5 kernel.
>
> The main project page is http://www.linuxtv.org.
>
> It's a patchset of 10 patches -- due to the size of some of the
> patches, I don't post them on the list again.
I don't go out to fetch patches, I really want to see them come to me (and
even then preferably through a few people acting as quality control and
maintainership).
And if there is no clear maintainership relationship and you need to send
them directly to me, I really want plain-text patches (they can be big if
needed), with:
- one patch per email. Not attachements, and not several patches in one
email one after the other. Several emails, with one patch each, and if
they are interdependent, make the subject line be something like
[DVB PATCH 1/9] Replace frobutomic counter with sequence numbers
so that when they get re-ordered by the email system (as they
inevitably will be), I can see it from the overview.
The "[xxx]" syntax is important: my scripts will automatically strip
that part out and replace it with "PATCH", leaving the commit message
as just
[PATCH] Replace frobutomic counter with sequence numbers
since once it's in the tree, the ordering is implicit in the SC.
- explanation of the patch for the changelogs at the top of the patch, so
that I don't have to make up my own explanations for what it does and
get it wrong.
- Make sure your mailer doesn't corrupt the message with whitespace
damage (spaces at ends of lines should remain, and tabs should be
tabs), or with stupidities like 7-bit quoted-printable crap.
Please.
Linus
On Wed, Jun 18, 2003 at 08:58:43AM -0700, Linus Torvalds wrote:
> [DVB PATCH 1/9] Replace frobutomic counter with sequence numbers
People might find the below script (hacked from gregkh's version) useful
for doing this. Should be fairly obvious.
regards,
john
#!/usr/bin/perl -w
# horrible hack of a script to send off a large number of email messages, one after
# each other, all chained together. This is useful for large numbers of patches.
#
# Use at your own risk!!!!
#
# greg kroah-hartman Jan 8, 2002
# <[email protected]>
#
# Released under the artistic license.
#
#
# modify these options each time you run the script
#
#$to = '[email protected], [email protected]';
$to = '[email protected]';
# If you want to chain the first post, fill this in
$initial_reply_to = '';
# a list of patches to send out
@files = (
["shutdown.diff", "OProfile: small NMI shutdown fix"],
["ioapic.diff", "OProfile: IO-APIC based NMI delivery"],
["exec.diff", "OProfile: thread switching performance fix"],
);
# Put your name and address here
$from = "John Levon <levon\@movementarian.org>";
# Don't need to change anything below here...
use Mail::Sendmail;
# we make a "fake" message id by taking the current number
# of seconds since the beginning of Unix time and tacking on
# a random number to the end, in case we are called quicker than
# 1 second since the last time we were called.
sub make_message_id
{
my $date = `date "+\%s"`;
chomp($date);
my $pseudo_rand = int (rand(4200));
$message_id = "<$date$pseudo_rand\@movementarian.org>";
print "new message id = $message_id\n";
}
sub send_message
{
%mail = ( To => $to,
From => $from,
Subject => $subject,
Message => $message,
'In-Reply-To' => $reply_to,
'Message-ID' => $message_id,
'X-Mailer' => "gregkh_patchbomb_levon_offspring",
);
$mail{smtp} = 'localhost';
sendmail(%mail) or die $Mail::Sendmail::error;
print "OK. Log says:\n", $Mail::Sendmail::log;
print "\n\n"
}
$reply_to = $initial_reply_to;
make_message_id();
$nrfiles = @files;
$current = 1;
foreach $t (@files) {
($F, $subj) = @$t;
open F or die "can't open file $t";
undef $/;
$message = <F>; # slurp the whole file in
close F;
$/ = "\n";
$subject = "[PATCH $current/$nrfiles] $subj";
send_message();
# set up for the next message
$reply_to = $message_id;
make_message_id();
$current++;
}
Hello Linus,
Linus Torvalds wrote:
>
> I don't go out to fetch patches, I really want to see them come to me (and
> even then preferably through a few people acting as quality control and
> maintainership).
>
> And if there is no clear maintainership relationship and you need to send
> them directly to me, I really want plain-text patches (they can be big if
> needed), with:
It would be great if we could clarfiy DVB maintainership. It would
nice to "appoint" one maintainer to whom we should send the
patches. If no one else volunteers, we will send them to you directly,
if you want with Cc: to lkml.
Alan Cox was the first one to merge the DVB subsystem into his 2.5 kernel
tree, but hinted that future updates should be sent in your direction.
Maybe that was just because he was busy with the IDE drivers, maybe
there are other reasons. I don't know.
The code base for the DVB drivers has grown over the last 3..4 years,
and there was a lot of stuff we wanted to clean up before inclusion in
the kernel. The pre-halloween merge was somewhat too soon, and there
were considerable cleanups afterwards which resulted in rather large
patches. Even after this current patch set, there is already a pile of
stuff to be merged. But the drivers in our CVS are already in a stable
condition, so future patches might be smaller ;-)
Thanks,
Johannes
On Wed, Jun 18, 2003 at 05:12:54PM +0100, John Levon wrote:
> On Wed, Jun 18, 2003 at 08:58:43AM -0700, Linus Torvalds wrote:
>
> > [DVB PATCH 1/9] Replace frobutomic counter with sequence numbers
>
> People might find the below script (hacked from gregkh's version) useful
> for doing this. Should be fairly obvious.
>
> regards,
> john
>
> #!/usr/bin/perl -w
>
> # horrible hack of a script to send off a large number of email messages, one after
> # each other, all chained together. This is useful for large numbers of patches.
> #
> # Use at your own risk!!!!
> #
> # greg kroah-hartman Jan 8, 2002
> # <[email protected]>
> #
> # Released under the artistic license.
> #
>
> #
> # modify these options each time you run the script
> #
> #$to = '[email protected], [email protected]';
> $to = '[email protected]';
>
> # If you want to chain the first post, fill this in
> $initial_reply_to = '';
>
> # a list of patches to send out
> @files = (
> ["shutdown.diff", "OProfile: small NMI shutdown fix"],
> ["ioapic.diff", "OProfile: IO-APIC based NMI delivery"],
> ["exec.diff", "OProfile: thread switching performance fix"],
> );
>
> # Put your name and address here
> $from = "John Levon <levon\@movementarian.org>";
>
> # Don't need to change anything below here...
>
> use Mail::Sendmail;
>
>
> # we make a "fake" message id by taking the current number
> # of seconds since the beginning of Unix time and tacking on
> # a random number to the end, in case we are called quicker than
> # 1 second since the last time we were called.
> sub make_message_id
> {
> my $date = `date "+\%s"`;
> chomp($date);
> my $pseudo_rand = int (rand(4200));
> $message_id = "<$date$pseudo_rand\@movementarian.org>";
> print "new message id = $message_id\n";
> }
>
>
>
>
>
> sub send_message
> {
> %mail = ( To => $to,
> From => $from,
> Subject => $subject,
> Message => $message,
> 'In-Reply-To' => $reply_to,
> 'Message-ID' => $message_id,
> 'X-Mailer' => "gregkh_patchbomb_levon_offspring",
> );
>
> $mail{smtp} = 'localhost';
>
> sendmail(%mail) or die $Mail::Sendmail::error;
>
> print "OK. Log says:\n", $Mail::Sendmail::log;
> print "\n\n"
> }
>
>
> $reply_to = $initial_reply_to;
> make_message_id();
> $nrfiles = @files;
> $current = 1;
>
> foreach $t (@files) {
> ($F, $subj) = @$t;
> open F or die "can't open file $t";
> undef $/;
> $message = <F>; # slurp the whole file in
> close F;
> $/ = "\n";
> $subject = "[PATCH $current/$nrfiles] $subj";
> send_message();
>
> # set up for the next message
> $reply_to = $message_id;
Please don't do this. $reply_to ||= $message_id is OK but
having each patch as a reply to the previous one is
annoying. I think it was Greg who recently posted one set
of patches that was so large the indentation for the thread
went off the screen.
[PATCH 0/n] frob the niggle
|-> [PATCH 1/n] frob the niggle
|-> [PATCH 2/n] frob the niggle
|-> [PATCH 3/n] frob the niggle
|-> [PATCH 4/n] frob the niggle
|-> [PATCH 5/n] frob the niggle
|-> [PATCH 6/n] frob the niggle
|-> [PATCH 7/n] frob the niggle
|-> [PATCH 8/n] frob the niggle
vs
[PATCH 0/n] frob the niggle
|-> [PATCH 1/n] frob the niggle
|-> [PATCH 2/n] frob the niggle
|-> [PATCH 3/n] frob the niggle
|-> [PATCH 4/n] frob the niggle
|-> [PATCH 5/n] frob the niggle
|-> [PATCH 6/n] frob the niggle
|-> [PATCH 7/n] frob the niggle
|-> [PATCH 8/n] frob the niggle
> make_message_id();
> $current++;
> }
Other than that, thanks. If more people posted patch sets
like this it would be much nicer.
--
________________________________________________________________
J.W. Schultz Pegasystems Technologies
email address: [email protected]
Remember Cernan and Schmitt
On Wed, Jun 18, 2003 at 06:45:09PM -0700, jw schultz wrote:
> [over-quoting snipped]
> Please don't do this. $reply_to ||= $message_id is OK but
> having each patch as a reply to the previous one is
> annoying. I think it was Greg who recently posted one set
> of patches that was so large the indentation for the thread
> went off the screen.
>
> [PATCH 0/n] frob the niggle
> |-> [PATCH 1/n] frob the niggle
> |-> [PATCH 2/n] frob the niggle
> |-> [PATCH 3/n] frob the niggle
Then the patches don't appear in order in people's mail readers.
john
On Thu, Jun 19, 2003 at 03:13:19AM +0100, John Levon wrote:
> On Wed, Jun 18, 2003 at 06:45:09PM -0700, jw schultz wrote:
>
> > [over-quoting snipped]
>
> > Please don't do this. $reply_to ||= $message_id is OK but
> > having each patch as a reply to the previous one is
> > annoying. I think it was Greg who recently posted one set
> > of patches that was so large the indentation for the thread
> > went off the screen.
> >
> > [PATCH 0/n] frob the niggle
> > |-> [PATCH 1/n] frob the niggle
> > |-> [PATCH 2/n] frob the niggle
> > |-> [PATCH 3/n] frob the niggle
>
> Then the patches don't appear in order in people's mail readers.
So what? The only time display order matters if one is
dependant on another. Most of the patch sets i've seen
don't have such dependencies. Otherwise the enumeration is
just so you know if you have all the patches in a set.
--
________________________________________________________________
J.W. Schultz Pegasystems Technologies
email address: [email protected]
Remember Cernan and Schmitt