public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michael Hunold <hunold@convergence.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-kernel@vger.kernel.org, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Linus Torvalds <torvalds@transmeta.com>
Subject: Re: DVB updates, 2nd try
Date: Wed, 28 May 2003 14:54:04 +0200	[thread overview]
Message-ID: <3ED4B16C.8030000@convergence.de> (raw)
In-Reply-To: <20030528111202.A27811@infradead.org>

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.


  reply	other threads:[~2003-05-28 12:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-05-27 13:08 DVB updates, 2nd try Michael Hunold
2003-05-28 10:12 ` Christoph Hellwig
2003-05-28 12:54   ` Michael Hunold [this message]
2003-05-28 12:58     ` Christoph Hellwig
2003-06-18 11:49   ` DVB updates, 3rd try Michael Hunold
2003-06-18 15:58     ` Linus Torvalds
2003-06-18 16:12       ` John Levon
2003-06-19  1:45         ` jw schultz
2003-06-19  2:13           ` John Levon
2003-06-19  8:55             ` jw schultz
2003-06-18 19:39       ` Johannes Stezenbach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ED4B16C.8030000@convergence.de \
    --to=hunold@convergence.de \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox