severity 464197 wishlist
tags 464197 help
thanks

On Wed, Feb 13, 2008 at 01:33:05PM -0600, Jan Hetges wrote:
> could please someone reenable snd-cs46xx ?

No. Not until you provided a patch which seperates the firmware.

Bastian

--
Prepare for tomorrow -- get ready.
-- Edith Keeler, "The City On the Edge of Forever",
stardate unknown



--
To UNSUBSCRIBE, email to debian-kernel-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

Here's a variant that I've actually dared to run.
The sound seems to be working, however rear channels
and S/PDIF probably won't work because SND_CS46XX_NEW_DSP
is still BROKEN.

Like before, sound/pci/cs46xx/write_images.c must be run
on a computer where unsigned long is little-endian 32-bit.
/usr/lib/hotplug/firmware/cs46xx/cwcealdr1_cwcimage saved
this way will then be usable on all architectures.

diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
index 3b060ab..d963904 100644
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -1,4 +1,5 @@
# ALSA PCI drivers
+# Modified on 2008-02-24 by Kalle Olavi Niemitalo

menu "PCI devices"
depends on SND!=n && PCI
@@ -197,8 +198,8 @@ config SND_CS4281

config SND_CS46XX
tristate "Cirrus Logic (Sound Fusion) CS4280/CS461x/CS462x/CS463x"
- depends on BROKEN
depends on SND
+ select FW_LOADER
select SND_RAWMIDI
select SND_AC97_CODEC
help
@@ -210,6 +211,7 @@ config SND_CS46XX

config SND_CS46XX_NEW_DSP
bool "Cirrus Logic (Sound Fusion) New DSP support"
+ depends on BROKEN
depends on SND_CS46XX
default y
help
diff --git a/sound/pci/cs46xx/cs46xx_lib.c b/sound/pci/cs46xx/cs46xx_lib.c
index 2c7bfc9..ed551e1 100644
--- a/sound/pci/cs46xx/cs46xx_lib.c
+++ b/sound/pci/cs46xx/cs46xx_lib.c
@@ -4,6 +4,8 @@
* Cirrus Logic, Inc.
* Routines for control of Cirrus Logic CS461x chips
*
+ * Modified on 2008-02-24 by Kalle Olavi Niemitalo.
+ *
* KNOWN BUGS:
* - Sometimes the SPDIF input DSP tasks get's unsynchronized
* and the SPDIF get somewhat "distorcionated", or/and left right channel
@@ -54,6 +56,7 @@
#include <linux/slab.h>
#include <linux/gameport.h>
#include <linux/mutex.h>
+#include <linux/firmware.h>


#include <sound/core.h>
@@ -358,23 +361,91 @@ int snd_cs46xx_clear_BA1(struct snd_cs46xx *chip,

#else /* old DSP image */

-#include "cs46xx_image.h"
+struct cs46xx_cwcimage_hunk {
+ u32 offset;
+ u32 size;
+};

-int snd_cs46xx_download_image(struct snd_cs46xx *chip)
+/* some validation, mostly pointless from a security viewpoint
+ * as malicious firmware can presumably do random DMA anyway */
+static int snd_cs46xx_check_image_size(const struct firmware *firmware)
{
- int idx, err;
- unsigned long offset = 0;
+ const struct cs46xx_cwcimage_hunk *const hunks
+ = (const struct cs46xx_cwcimage_hunk *) firmware->data;
+ const size_t maxbytes = BA1_MEMORY_COUNT * 0x10000;
+ size_t offset;
+ int idx;

+ offset = BA1_MEMORY_COUNT * sizeof(struct cs46xx_cwcimage_hunk);
+ if (firmware->size < offset) {
+ snd_printk( KERN_ERR "cs46xx: firmware too small
");
+ return -EINVAL;
+ }
for (idx = 0; idx < BA1_MEMORY_COUNT; idx++) {
- if ((err = snd_cs46xx_download(chip,
- &BA1Struct.map[offset],
- BA1Struct.memory[idx].offset,
- BA1Struct.memory[idx].size)) < 0)
- return err;
- offset += BA1Struct.memory[idx].size >> 2;
- }
+ if (hunks[idx].offset % sizeof(u32)
+ || hunks[idx].size % sizeof(u32)) {
+ snd_printk( KERN_ERR "cs46xx: firmware hunk misaligned
");
+ return -EINVAL;
+ }
+
+ if (hunks[idx].offset >= maxbytes
+ || hunks[idx].size >= maxbytes - hunks[idx].offset) {
+ snd_printk( KERN_ERR "cs46xx: firmware hunk out of range
");
+ return -EINVAL;
+ }
+ offset += hunks[idx].size;
+ }
+ if (firmware->size != offset) {
+ snd_printk( KERN_ERR "cs46xx: firmware size mismatch
");
+ return -EINVAL;
+ }
+
return 0;
}
+
+static int snd_cs46xx_download_image(struct snd_cs46xx *chip)
+{
+ int idx, err;
+ size_t offset;
+ const struct firmware *firmware = NULL;
+ const struct cs46xx_cwcimage_hunk *hunks;
+
+ err = request_firmware(&firmware, "cs46xx/cwcealdr1_cwcimage",
+ &chip->pci->dev);
+ if (err < 0) {
+ snd_printk( KERN_ERR "cs46xx: no firmware
");
+ goto end;
+ }
+
+ if (firmware->size % sizeof(u32) != 0) {
+ snd_printk( KERN_ERR "cs46xx: firmware size misaligned
");
+ err = -EINVAL;
+ goto end;
+ }
+ for (offset = 0; offset < firmware->size; offset += sizeof(u32))
+ le32_to_cpup((u32 *) (firmware->data + offset));
+
+ err = snd_cs46xx_check_image_size(firmware);
+ if (err < 0)
+ goto end;
+ hunks = (const struct cs46xx_cwcimage_hunk *) firmware->data;
+
+ /* the actual download */
+ offset = BA1_MEMORY_COUNT * sizeof(struct cs46xx_cwcimage_hunk);
+ for (idx = 0; idx < BA1_MEMORY_COUNT; idx++) {
+ err = snd_cs46xx_download(chip,
+ (__u32 *) (firmware->data + offset),
+ hunks[idx].offset,
+ hunks[idx].size);
+ if (err < 0)
+ goto end;
+ offset += hunks[idx].size;
+ }
+ err = 0;
+end:
+ release_firmware(firmware);
+ return err;
+}
#endif /* CONFIG_SND_CS46XX_NEW_DSP */

/*
@@ -3942,3 +4013,5 @@ int __devinit snd_cs46xx_create(struct snd_card *card,
*rchip = chip;
return 0;
}
+
+MODULE_FIRMWARE("cs46xx/cwcealdr1_cwcimage");
diff --git a/sound/pci/cs46xx/write_images.c b/sound/pci/cs46xx/write_images.c
new file mode 100644
index 0000000..bc7f2c2
--- /dev/null
+++ b/sound/pci/cs46xx/write_images.c
@@ -0,0 +1,66 @@
+/*
+ * Write out firmware images for Cirrus Logic's Sound Fusion CS46XX
+ * based soundcards
+ * Copyright (c) by Jaroslav Kysela <perex@perex.cz>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Modified on 2008-04-06 by Kalle Olavi Niemitalo.
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+/* The following two #defines were copied from
+ * linux-2.6.24/sound/pci/cs46xx/cs46xx_lib.h,
+ * Copyright (c) by Jaroslav Kysela <perex@perex.cz> */
+/* 3*1024 parameter, 3.5*1024 sample, 2*3.5*1024 code */
+#define BA1_DWORD_SIZE (13 * 1024 + 512)
+#define BA1_MEMORY_COUNT 3
+
+typedef uint32_t u32;
+#include "../../../../../linux-2.6-2.6.22/sound/pci/cs46xx/cs46xx_image.h"
+
+static int write_image(const char *filename, const void *data, size_t len)
+{
+ FILE *file = fopen(filename, "wb");
+ if (file == NULL) {
+ perror(filename);
+ return 1;
+ }
+ if (fwrite(data, len, 1, file) != 1) {
+ perror(filename);
+ fclose(file);
+ remove(filename);
+ return 1;
+ }
+ if (fclose(file) != 0) {
+ perror(filename);
+ remove(filename);
+ return 1;
+ }
+ return 0;
+}
+
+int main(void)
+{
+ mkdir("cs46xx", S_IRWXU | S_IRWXG | S_IRWXO);
+ if (write_image("cs46xx/cwcealdr1_cwcimage", &BA1Struct, sizeof BA1Struct))
+ return 1;
+ return 0;
+}

The patch I posted leaves SND_CS46XX_NEW_DSP broken. Because I
have neither rear speakers nor an S/PDIF capable amplifier, and
the newer firmware consists of multiple structures and so is more
difficult to load, I don't currently intend to work on that.

One trap with the patch is that it converts the endianness in
place. With Linux 2.6.24, this works because request_firmware()
always makes a fresh copy. However if a reference count is later
added to struct firmware, this scheme will break. I see that
<linux/firmware.h> makes the struct firmware const but not the
bytes to which its "data" member points; might it be inferred
From this that the data is intended to be modified by drivers?

When testing the patch, I had linux-image-2.6.24-1-686 2.6.24-3
running. I then unpacked the corresponding sources, made the
changes, ran make -f debian/rules.gen binary-arch_i386_none_686,
and loaded the generated cs46xx.ko without rebooting; the
interfaces between modules remain compatible. With earlier
versions of the linux-2.6 source, building all the binary
packages has taken 11 to 22 hours here, so calling
debian/rules.gen directly was a huge timesaver. If this shortcut
is documented, I don't know where.

The patch assumes the binary firmware image is little-endian, and
converts it to host byte order. Personally, I prefer to have a
fixed format for each data file so that they can be freely shared
between architectures. I chose little-endian u32 here because
that was easiest for me to output. I did not bother to implement
the proper conversions in the write_images program though. If
that source code is going to be added to an Architecture: any
package, then both word-size and endianness conversions should be
implemented. The original cwcealdr1.zip/cwcimage.h does not
include the definition of struct BA1struct, so I suppose there
wouldn't be any problem in ripping it out again and instead
providing (in the #including file) a definition that has u32 in
the place of unsigned long. Alternatively, the driver could be
changed to assume that the firmware is already in a host-specific
format.

If Cirrus Logic grants permission to distribute the cs46xx
firmware included in the upstream Linux sources, will that
suffice for SND_CS46XX_NEW_DSP too, or will separate permission
be needed from the author of sound/pci/cs46xx/imgs/cwcdma.asp?