DVB updates, 2nd try

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,

Yes, it should. It has some extended features, which are only visible through new ioctl, though.

I'd leave this to a later patch.

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.

Ok, comes with the next patch.

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:

Yes, the firmware would work with the old driver as well. So this one can safely applied.

Fixed.

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.

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"

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

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,

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

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

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

Then the patches don't appear in order in people's mail readers.

john

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