On Thu, Dec 2, 2010 at 10:11 AM, Dimitris Papastamos dp@opensource.wolfsonmicro.com wrote:
Make sure to use codec->reg_def_copy instead of codec_drv->reg_cache_default wherever necessary. This change is necessary because in the next patch we move the cache initialization code outside snd_soc_register_codec() and by that time any data marked as __devinitconst such as the original reg_cache_default array might have already been freed by the kernel.
This commit breaks cs4270.c:
Cirrus Logic CS4270 ALSA SoC Codec Driver cs4270-codec 0-004f: found device at i2c address 4F cs4270-codec 0-004f: hardware revision 3 Unable to handle kernel paging request for data at address 0x00000000 Faulting instruction address: 0xc001646c Oops: Kernel access of bad area, sig: 11 [#1] MPC86xx HPCD last sysfs file: Modules linked in: NIP: c001646c LR: c006cef8 CTR: 00000001 REGS: ef03dd20 TRAP: 0300 Not tainted (2.6.37-rc3-02080-g8e7bd55) MSR: 00009032 <EE,ME,IR,DR> CR: 22044028 XER: 20000000 DAR: 00000000, DSISR: 40000000 TASK = ef038000[1] 'swapper' THREAD: ef03c000 GPR00: 00000000 ef03ddd0 ef038000 ef2bb9e0 fffffffc 00000008 ef2bb9dc 00000001 GPR08: 2e302d30 00000000 fffffffe 00000006 22044082 100a38d4 00000000 3ffe7388 GPR16: 00000000 3f9a6378 00000000 3ffe6210 3ffe6210 3ffe6210 00000000 00000000 GPR24: c03c0560 ef03de20 c043c968 ef03de00 ef3da220 00000000 ef2bb9e0 00000008 NIP [c001646c] memcpy+0x1c/0x9c LR [c006cef8] kmemdup+0x3c/0x54 Call Trace: [ef03ddd0] [c006cee4] kmemdup+0x28/0x54 (unreliable) [ef03ddf0] [c0250664] snd_soc_register_codec+0x278/0x390 [ef03de70] [c0256514] cs4270_i2c_probe+0xb0/0x178 [ef03de90] [c021c8a8] i2c_device_probe+0xec/0x114 [ef03deb0] [c01af400] driver_probe_device+0xa0/0x1a8 [ef03ded0] [c01af5c4] __driver_attach+0xbc/0xc0 [ef03def0] [c01aea90] bus_for_each_dev+0x70/0xac [ef03df20] [c01af20c] driver_attach+0x24/0x34 [ef03df30] [c01ae314] bus_add_driver+0x1b8/0x298 [ef03df60] [c01af928] driver_register+0x88/0x158 [ef03df80] [c021cd38] i2c_register_driver+0x4c/0xa4 [ef03dfa0] [c040fffc] cs4270_init+0x2c/0x3c [ef03dfb0] [c0003f6c] do_one_initcall+0x15c/0x1c4 [ef03dfe0] [c03f31e0] kernel_init+0xc0/0x164 [ef03dff0] [c001086c] kernel_thread+0x4c/0x68 Instruction dump: 38c60001 4200fff0 4e800020 7c032040 418100a0 54a7e8ff 38c3fffc 3884fffc 41820028 70c00003 7ce903a6 40820054 <80e40004> 85040008 90e60004 95060008 ---[ end trace 40d01772bfab46f2 ]--- Kernel panic - not syncing: Attempted to kill init!
Which is this line:
@@ -3463,6 +3464,19 @@ int snd_soc_register_codec(struct device *dev,
/* allocate CODEC register cache */ if (codec_drv->reg_cache_size && codec_drv->reg_word_size) {
- reg_size = codec_drv->reg_cache_size * codec_drv->reg_word_size;
- /* it is necessary to make a copy of the default register cache
- * because in the case of using a compression type that requires
- * the default register cache to be marked as __devinitconst the
- * kernel might have freed the array by the time we initialize
- * the cache.
- */
- codec->reg_def_copy = kmemdup(codec_drv->reg_cache_default,
- reg_size, GFP_KERNEL);
Unlike all the other codec drivers, the CS4270 driver does not initialize codec_drv->reg_cache_default, so the pointer is NULL during the kmemdup() call.
What is the criteria for defining reg_cache_default? Many drivers don't define it:
$ grep -c reg_cache_default *.c | grep :0 88pm860x-codec.c:0 ac97.c:0 ad1836.c:0 ad73311.c:0 ads117x.c:0 ak4104.c:0 alc5623.c:0 cq93vc.c:0 cs4270.c:0 cs42l51.c:0 cx20442.c:0 l3.c:0 max9877.c:0 pcm3008.c:0 spdif_transciever.c:0 tlv320aic26.c:0 tpa6130a2.c:0 wl1273.c:0 wm2000.c:0 wm8350.c:0 wm8400.c:0 wm8727.c:0 wm8994-tables.c:0 wm_hubs.c:0