[lkml]   [2003]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] 2.6.0-test3 zoran driver update
Hey Francois,

coming back to your other points now. (This means I've worked on them.
;) ).

On Thu, 2003-08-21 at 01:08, Francois Romieu wrote:
> - {adv7170/adv7175/bt819/saa7110/saa7185}_detect_client()
> for each of these functions, two error exit path leak on locally allocated
> variable "channel".

That one was fixed already, patch coming up.

> - {adv7170/adv7175/bt819/saa7111/saa7185}_write_block()
> The code duplication could surely be avoided.

I'm wondering how. I could make an inline function that each of them
includes (but that doesn't decrease binary size, code is still
duplicated), or I could make a parent module (and I don't want to do
that). The only bad thing of the current way is the maintainance, but I
don't really mind.

Does it matter if I just keep it the way it is right now? I don't really
mind at all.

> - always put a blank line between variables declaration and code pleae

Patch coming up. It fixes most of them. I might have missed one or two.

> - find_zr36057():
> what about replacing pci_find_device() by the modern pci insertion/removal
> api (which has been standing there for ~3 years)

That's planned (also, we aren't conforming to the latest DMA API yet,
that's planned for future fixage, too). I can't really fix that
short-term, though, I'm affraid...

> - pci_enable_device() in find_zr36057() isn't balanced by pci_disable_device()
> in zoran_release()

Patch coming up...

> - +irqreturn_t
> +zoran_irq (int irq,
> [...]
> + for (i = 0; i < 4; i++) {
> + if (zr->
> + stat_com[i] &
> + 1)
> + sv[i] =
> + '1';
> + }
> Post-modernism ?

Uh yes, that's an indent artifact, we occassionaly find that in some
places. I started with a codebase that hadn't been maintained for a year
or so, and it used 3 coding styles, so I fixed that by just running
indent -kr over it and fixing the most obvious style bugs as I
encountered them. However, I'm sure I missed some, like this. If you
find more, please let me know. Patch coming up. :).

> It looks interesting.

Thanks, and thanks for the comments, too.

I'm moving some things explicitely to the long-term list, btw, that's on
purpose. It'd be cool to handle the new DMA/PCI subsystem, but we've got
many things to work on, some of which are more important than this.
Also, kernel modules isn't all I'm working on, and this isn't my job. So
it's no deinterest or anything, it's simply a matter of (lack of) time
in combination with a bit of realism. I'll work on it, just not today.


Ronald Bultje <>

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

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