On Mon, 2009-04-20 at 21:45 -0600, dann frazier wrote: > On Wed, Apr 15, 2009 at 11:01:09AM +0200, Antonio Ospite wrote: > > Hi, > > > > I just wonder if there is any update on this one and if the split-out > > patch has been proposed to upstream yet. > > > > If you manage to get this upstream, with Linus keeping on distributing > > the binary images, debian can well choose not to distribute them, but > > debian users can still get the blob somewhere and have an easier life. > > Not ideal, I know, but that's the world. > > Yep - that's true. > > Kalle: would you mind submitting your patch upstream, if you haven't > already? A lot of similar patches for other drives have been accepted > in recent months. Kalle's patch has a serious problem in that it attempts to byte-swap the firmware in place. On a big-endian system where the firmware is built into the kernel, or if a cache is implemented, this will corrupt the image or cause an oops. Furthermore, I think any patch sent upstream will need to handle the "new" DSP code as well. Anyway, here's my proposed patch for unstable (against 2.6.30-rc4) that deals with the first problem. I'll have a go at handling the "new" DSP code as well, but as I don't have the hardware for this driver this will need testing by others. I made the following changes relative to Kalle's patch: - Remove "Modified on..." lines; that's what the commit message is for - Do not call release_firmware() if request_firmware() fails - Make firmware images explicitly const and little-endian and never swap them. - Remove offsets from firmware header so that we don't have to validate them; we know the offset should be the base of the corresponding memory bank. - Validate sizes against the memory bank size. - Change filename to cs46xx-old.fw as this is easier to associate with its use. We can use cs46xx-new.fw for the "new" DSP code. Ben. diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig index 17e03b9..124b3a0 100644 --- a/sound/pci/Kconfig +++ b/sound/pci/Kconfig @@ -229,7 +229,7 @@ config SND_CS4281 config SND_CS46XX tristate "Cirrus Logic (Sound Fusion) CS4280/CS461x/CS462x/CS463x" - depends on BROKEN + select FW_LOADER select SND_RAWMIDI select SND_AC97_CODEC help @@ -241,6 +241,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 1be96ea..b12b930 100644 --- a/sound/pci/cs46xx/cs46xx_lib.c +++ b/sound/pci/cs46xx/cs46xx_lib.c @@ -53,6 +53,7 @@ #include <linux/slab.h> #include <linux/gameport.h> #include <linux/mutex.h> +#include <linux/firmware.h> #include <sound/core.h> @@ -308,7 +309,7 @@ static void snd_cs46xx_ac97_write(struct snd_ac97 *ac97, */ int snd_cs46xx_download(struct snd_cs46xx *chip, - u32 *src, + const __le32 *src, unsigned long offset, unsigned long len) { @@ -321,9 +322,9 @@ int snd_cs46xx_download(struct snd_cs46xx *chip, dst = chip->region.idx[bank+1].remap_addr + offset; len /= sizeof(u32); - /* writel already converts 32-bit value to right endianess */ while (len-- > 0) { - writel(*src++, dst); + __raw_writel((__force u32)*src++, dst); + mmiowb(); dst += sizeof(u32); } return 0; @@ -360,23 +361,77 @@ int snd_cs46xx_clear_BA1(struct snd_cs46xx *chip, #else /* old DSP image */ -#include "cs46xx_image.h" +struct cs46xx_old_image { + __le32 size[BA1_MEMORY_COUNT]; + __le32 data[0]; +}; -int snd_cs46xx_download_image(struct snd_cs46xx *chip) +static int snd_cs46xx_check_image_size(const struct firmware *firmware) { - int idx, err; - unsigned long offset = 0; + const struct cs46xx_old_image *image = + (const struct cs46xx_old_image *)firmware->data; + size_t offset = sizeof(*image); + int idx; + + if (firmware->size < offset) { + snd_printk(KERN_ERR "cs46xx: firmware too small\n"); + 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; - } + size_t size = le32_to_cpu(image->size[idx]); + + if (size % sizeof(u32)) { + snd_printk(KERN_ERR "cs46xx: firmware hunk misaligned\n"); + return -EINVAL; + } + if (size > BA1_DWORD_SIZE * sizeof(u32)) { + snd_printk(KERN_ERR "cs46xx: firmware hunk out of range\n"); + return -EINVAL; + } + offset += size; + } + + if (firmware->size != offset) { + snd_printk(KERN_ERR "cs46xx: firmware size mismatch\n"); + return -EINVAL; + } + return 0; } + +static int snd_cs46xx_download_image(struct snd_cs46xx *chip) +{ + int idx, err; + const struct firmware *firmware = NULL; + const struct cs46xx_old_image *image; + const __le32 *data; + + err = request_firmware(&firmware, "cs46xx/cs46xx-old.fw", + &chip->pci->dev); + if (err < 0) { + snd_printk(KERN_ERR "cs46xx: no firmware\n"); + return err; + } + + err = snd_cs46xx_check_image_size(firmware); + if (err < 0) + goto end; + image = (const struct cs46xx_old_image *)firmware->data; + data = image->data; + + for (idx = 0; idx < BA1_MEMORY_COUNT; idx++) { + size_t size = le32_to_cpu(image->size[idx]); + + err = snd_cs46xx_download(chip, data, idx << 16, size); + if (err < 0) + goto end; + data += size / sizeof(u32); + } +end: + release_firmware(firmware); + return err; +} #endif /* CONFIG_SND_CS46XX_NEW_DSP */ /* @@ -3874,3 +3929,5 @@ int __devinit snd_cs46xx_create(struct snd_card *card, *rchip = chip; return 0; } + +MODULE_FIRMWARE("cs46xx/cs46xx-old.fw"); diff --git a/sound/pci/cs46xx/cs46xx_lib.h b/sound/pci/cs46xx/cs46xx_lib.h index 4eb55aa..85babb5 100644 --- a/sound/pci/cs46xx/cs46xx_lib.h +++ b/sound/pci/cs46xx/cs46xx_lib.h @@ -103,8 +103,8 @@ int cs46xx_dsp_proc_done (struct snd_cs46xx *chip); #define cs46xx_dsp_proc_done(chip) #endif int cs46xx_dsp_scb_and_task_init (struct snd_cs46xx *chip); -int snd_cs46xx_download (struct snd_cs46xx *chip, u32 *src, unsigned long offset, - unsigned long len); +int snd_cs46xx_download(struct snd_cs46xx *chip, const __le32 *src, unsigned long offset, + unsigned long len); int snd_cs46xx_clear_BA1(struct snd_cs46xx *chip, unsigned long offset, unsigned long len); int cs46xx_dsp_enable_spdif_out (struct snd_cs46xx *chip); int cs46xx_dsp_enable_spdif_hw (struct snd_cs46xx *chip); --- END --- -- Ben Hutchings No political challenge can be met by shopping. - George Monbiot
Attachment:
signature.asc
Description: This is a digitally signed message part