Skip to contentSkip to conversation

Patch for ymfpci in 2.5.x

DISCUSSION Pete Zaitcev 2 participants 4 messages Jan 11, 2002 – Jan 22, 2002 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Pete Zaitcev

author
#1 lore raw

I am terribly sorry, but an additional fixup is needed, as identified by Anders Rune Jensen (mental note - always cc submissions to linux-kernel. So many good eyeballs).

drivers/sound/ymfpci.c renamed from drivers/sound/ymfpci.c.bak +1 -1
@@ -832,7 +832,7 @@
 	u32 lpfK = ymfpci_calc_lpfK(rate);
 	ymfpci_playback_bank_t *bank;
 	int nbank;
-	unsigned le_0x40000000 = 0x40;
+	unsigned le_0x40000000 = cpu_to_le32(0x40000000);
 
 	format = (stereo ? 0x00010000 : 0) | (w_16 ? 0 : 0x80000000);
 	if (stereo)

Humbled
-- Pete

Quoted 16 lines

Date: Fri, 11 Jan 2002 00:44:23 -0500
From: Pete Zaitcev <[email protected]>
To: [email protected]
Cc: [email protected], [email protected]

While 2.5.x was a bit unstable, ymfpci in 2.4 moved ahead a bit. Most of it is cleanup, but there is a real fix for artsd as well. ALSA is supposed to supersede this code, but unless it's happening tomorrow we better update it before the patch grows too big.

-- Pete

diff -ur -X dontdiff linux-2.5.2-pre11/drivers/sound/ymfpci.c linux-2.5.2-pre11-p3/drivers/sound/ymfpci.c

drivers/sound/ymfpci.c +0 -0
[...]
#2 lore raw
Quoted 4 lines · Pete Zaitcev

I am terribly sorry, but an additional fixup is needed, as identified by Anders Rune Jensen (mental note - always cc submissions to linux-kernel. So many good eyeballs).

It would have been even saner to give that bit some sane name, and have something like

	#define YMFPCI_XXXBIT (__constant_cpu_to_le32(0x40000000))

instead of creating a totally nonsensical random number.

I applied your patch, but if I had created that code I would be too ashamed to post such a patch on a public mailing list.

Linus

author
#3 lore raw
In reply to Pete Zaitcev
Quoted 9 lines

Date: Fri, 11 Jan 2002 10:36:30 -0800 (PST)
From: Linus Torvalds <[email protected]>

It would have been even saner to give that bit some sane name, and have something like

	#define YMFPCI_XXXBIT (__constant_cpu_to_le32(0x40000000))

instead of creating a totally nonsensical random number.

I think something like this would be better than one more totally nonsesical random #define:

drivers/sound/ymfpci.c +7 -0
@@ -832,6 +832,13 @@
 	u32 lpfK = ymfpci_calc_lpfK(rate);
 	ymfpci_playback_bank_t *bank;
 	int nbank;
+
+	/*
+	 * The gain is a floating point number. According to the manual,
+	 * bit 31 indicates a sign bit, bit 30 indicates an integer part,
+	 * and bits [29:15] indicate a decimal fraction part. Thus,
+	 * for a gain of 1.0 the constant of 0x40000000 is loaded.
+	 */
 	unsigned le_0x40000000 = cpu_to_le32(0x40000000);
 
 	format = (stereo ? 0x00010000 : 0) | (w_16 ? 0 : 0x80000000);

-- Pete

#4 lore raw
Quoted 3 lines · Pete Zaitcev

I think something like this would be better than one more totally nonsesical random #define:

No.

Why don't you call it something SANE, like "default_gain" or something.

Anybody who uses variable names like "le_0x40000000" is just horribly misguided. Never EVER do that. You're better off just writing it out as "cpu_to_le32(0x40000000)" in that case.

Linus