Re: [PATCH] ALSA fixes #1

From: Linus Torvalds (torvalds@transmeta.com)
Date: Thu Oct 03 2002 - 10:37:39 EST


On Thu, 3 Oct 2002, Jaroslav Kysela wrote:
>
> You can import this changeset into BK by piping this whole message to:
> '| bk receive [path to repository]' or apply the patch as usual.

Don't even bother with the BK patches, I've never had them apply
successfulyl for me, because your BK tree contains ChangeSets that simply
do not exist in mine. As a result, your BK patches refuse to apply. Please
read the BK usage documentation.

Note that BK really only helps if you are careful, and I can synchronize
with your BK tree. The fact that your BK tree contains non-alsa stuff
already means that I cannot sync with it, which makes it pretty much
unusable. But even if that was fixed, what is all the OLD_USB crap, and
stuff like this:

> ChangeSet@1.676, 2002-10-03 12:03:21+02:00, perex@suse.cz
> ALSA compilation update
> - save_flags/cli/restore_flags removal

Don't even bother fixing compilation, if the end result is patches like

> @@ -140,20 +139,18 @@
> snd_printdd("sgalaxy - setting up IRQ/DMA for WSS\n");
> #endif
>
> - save_flags(flags);
> - cli();
> + /* FIXME: this is really bogus --jk */
> + /* the irq line is not allocated (thus locked) for this device at the moment */
> + disable_irq(irq);
>
> /* initialize IRQ for WSS codec */
> tmp = interrupt_bits[irq % 16];
> - if (tmp < 0) {
> - restore_flags(flags);
> + if (tmp < 0)
> return -EINVAL;
> - }
> outb(tmp | 0x40, port);
> tmp1 = dma_bits[dma % 4];
> outb(tmp | tmp1, port);
>
> - restore_flags(flags);
> return 0;
> }
>

It's BETTER to not compile, than have obviously totally bogus code that
will break _other_ devices in the tree.

You're disabling (and never re-enabling) an interrupt that isn't even
YOURS, for chrissake! Which just means that if that irq happens to be
shared with the harddisk, for example, you just killed the whole machine!

Please, don't do crap like this. Sure, it may be an ISA driver, and if it
has the right irq you may hope that it isn't shared (technically illegal
in ISA, but certainly done nonetheless), but the old code didn't do
anything even _nearly_ that broken, and the trival spinlock replacement
should have worked quite as well as the old code ever did.

This, btw, is one reason why I don't like hidden CVS trees that are worked
on by people that don't know or care about the rest of the kernel -
because the end result ends up often being total crap, that is hidden by
large merges. I noticed this one only because the patch was of a
reasonable size, and I _shudder_ to think of what horrible things the big
broken merges have caused.

Yes, I'm frustrated. What can we do about things like this? I'll apply the
patch without the obviously horribly broken part, please don't
re-introduce it later.

                Linus

-
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/



This archive was generated by hypermail 2b29 : Mon Oct 07 2002 - 22:00:40 EST