[PATCH 1/2] ALSA: emu10k1: fix SNDRV_EMU10K1_IOCTL_SINGLE_STEP
- Use correct address limit for Audigy - Use the right constant to actually make a step on Audigy - Don't store *_DBG_STEP and the address in emu->fx8010.dbg, as otherwise unrelated operations would make steps, too
This is untested. as10k1 was never ported to Audigy anyway.
Signed-off-by: Oswald Buddenhagen oswald.buddenhagen@gmx.de --- sound/pci/emu10k1/emufx.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c index 6a51aed59238..70ec2cb9efe8 100644 --- a/sound/pci/emu10k1/emufx.c +++ b/sound/pci/emu10k1/emufx.c @@ -2646,17 +2646,19 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un return -EPERM; if (get_user(addr, (unsigned int __user *)argp)) return -EFAULT; - if (addr > 0x1ff) - return -EINVAL; - if (emu->audigy) - snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg |= A_DBG_SINGLE_STEP | addr); - else - snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg |= EMU10K1_DBG_SINGLE_STEP | addr); - udelay(10); - if (emu->audigy) - snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg |= A_DBG_SINGLE_STEP | A_DBG_STEP_ADDR | addr); - else - snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg |= EMU10K1_DBG_SINGLE_STEP | EMU10K1_DBG_STEP | addr); + if (emu->audigy) { + if (addr > A_DBG_STEP_ADDR) + return -EINVAL; + snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg |= A_DBG_SINGLE_STEP); + udelay(10); + snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg | A_DBG_STEP | addr); + } else { + if (addr > EMU10K1_DBG_SINGLE_STEP_ADDR) + return -EINVAL; + snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg |= EMU10K1_DBG_SINGLE_STEP); + udelay(10); + snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg | EMU10K1_DBG_STEP | addr); + } return 0; case SNDRV_EMU10K1_IOCTL_DBG_READ: if (emu->audigy)
These functions don't actually touch the DSP until they poke the code into it, at which point it's temporarily stopped anyway. And fx8010.dbg is already zero anyway.
Signed-off-by: Oswald Buddenhagen oswald.buddenhagen@gmx.de --- sound/pci/emu10k1/emufx.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c index 70ec2cb9efe8..b43acf0611a2 100644 --- a/sound/pci/emu10k1/emufx.c +++ b/sound/pci/emu10k1/emufx.c @@ -1259,9 +1259,6 @@ static int _snd_emu10k1_audigy_init_efx(struct snd_emu10k1 *emu) gpr_map[gpr++] = 0x0000ffff; bit_shifter16 = gpr;
- /* stop FX processor */ - snd_emu10k1_ptr_write(emu, A_DBG, 0, (emu->fx8010.dbg = 0) | A_DBG_SINGLE_STEP); - #if 1 /* PCM front Playback Volume (independent from stereo mix) * playback = 0 + ( gpr * FXBUS_PCM_LEFT_FRONT >> 31) @@ -1899,9 +1896,6 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu) tmp = 0x88; /* we need 4 temporary GPR */ /* from 0x8c to 0xff is the area for tone control */
- /* stop FX processor */ - snd_emu10k1_ptr_write(emu, DBG, 0, (emu->fx8010.dbg = 0) | EMU10K1_DBG_SINGLE_STEP); - /* * Process FX Buses */
Hi Oswald,
kernel test robot noticed the following build errors:
[auto build test ERROR on tiwai-sound/for-next] [also build test ERROR on tiwai-sound/for-linus linus/master v6.3-rc7 next-20230421] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Oswald-Buddenhagen/ALSA-emu10... base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next patch link: https://lore.kernel.org/r/20230422161021.1144004-1-oswald.buddenhagen%40gmx.... patch subject: [PATCH 1/2] ALSA: emu10k1: fix SNDRV_EMU10K1_IOCTL_SINGLE_STEP config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230423/202304230435.xkjl2DgF-lkp@i...) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/dfbd3fc49e3b9ec81160caee29b1d4... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Oswald-Buddenhagen/ALSA-emu10k1-don-t-stop-DSP-in-_snd_emu10k1_-audigy_-init_efx/20230423-002213 git checkout dfbd3fc49e3b9ec81160caee29b1d41476d5a6aa # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=i386 olddefconfig make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304230435.xkjl2DgF-lkp@intel.com/
All errors (new ones prefixed by >>):
sound/pci/emu10k1/emufx.c: In function 'snd_emu10k1_fx8010_ioctl':
sound/pci/emu10k1/emufx.c:2654:80: error: 'A_DBG_STEP' undeclared (first use in this function); did you mean 'A_DBG_ZC'?
2654 | snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg | A_DBG_STEP | addr); | ^~~~~~~~~~ | A_DBG_ZC sound/pci/emu10k1/emufx.c:2654:80: note: each undeclared identifier is reported only once for each function it appears in
vim +2654 sound/pci/emu10k1/emufx.c
2541 2542 static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, unsigned int cmd, unsigned long arg) 2543 { 2544 struct snd_emu10k1 *emu = hw->private_data; 2545 struct snd_emu10k1_fx8010_info *info; 2546 struct snd_emu10k1_fx8010_code *icode; 2547 struct snd_emu10k1_fx8010_pcm_rec *ipcm; 2548 unsigned int addr; 2549 void __user *argp = (void __user *)arg; 2550 int res; 2551 2552 switch (cmd) { 2553 case SNDRV_EMU10K1_IOCTL_PVERSION: 2554 emu->support_tlv = 1; 2555 return put_user(SNDRV_EMU10K1_VERSION, (int __user *)argp); 2556 case SNDRV_EMU10K1_IOCTL_INFO: 2557 info = kzalloc(sizeof(*info), GFP_KERNEL); 2558 if (!info) 2559 return -ENOMEM; 2560 snd_emu10k1_fx8010_info(emu, info); 2561 if (copy_to_user(argp, info, sizeof(*info))) { 2562 kfree(info); 2563 return -EFAULT; 2564 } 2565 kfree(info); 2566 return 0; 2567 case SNDRV_EMU10K1_IOCTL_CODE_POKE: 2568 if (!capable(CAP_SYS_ADMIN)) 2569 return -EPERM; 2570 2571 icode = memdup_user(argp, sizeof(*icode)); 2572 if (IS_ERR(icode)) 2573 return PTR_ERR(icode); 2574 res = snd_emu10k1_icode_poke(emu, icode, false); 2575 kfree(icode); 2576 return res; 2577 case SNDRV_EMU10K1_IOCTL_CODE_PEEK: 2578 icode = memdup_user(argp, sizeof(*icode)); 2579 if (IS_ERR(icode)) 2580 return PTR_ERR(icode); 2581 res = snd_emu10k1_icode_peek(emu, icode); 2582 if (res == 0 && copy_to_user(argp, icode, sizeof(*icode))) { 2583 kfree(icode); 2584 return -EFAULT; 2585 } 2586 kfree(icode); 2587 return res; 2588 case SNDRV_EMU10K1_IOCTL_PCM_POKE: 2589 ipcm = memdup_user(argp, sizeof(*ipcm)); 2590 if (IS_ERR(ipcm)) 2591 return PTR_ERR(ipcm); 2592 res = snd_emu10k1_ipcm_poke(emu, ipcm); 2593 kfree(ipcm); 2594 return res; 2595 case SNDRV_EMU10K1_IOCTL_PCM_PEEK: 2596 ipcm = memdup_user(argp, sizeof(*ipcm)); 2597 if (IS_ERR(ipcm)) 2598 return PTR_ERR(ipcm); 2599 res = snd_emu10k1_ipcm_peek(emu, ipcm); 2600 if (res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm))) { 2601 kfree(ipcm); 2602 return -EFAULT; 2603 } 2604 kfree(ipcm); 2605 return res; 2606 case SNDRV_EMU10K1_IOCTL_TRAM_SETUP: 2607 if (!capable(CAP_SYS_ADMIN)) 2608 return -EPERM; 2609 if (get_user(addr, (unsigned int __user *)argp)) 2610 return -EFAULT; 2611 mutex_lock(&emu->fx8010.lock); 2612 res = snd_emu10k1_fx8010_tram_setup(emu, addr); 2613 mutex_unlock(&emu->fx8010.lock); 2614 return res; 2615 case SNDRV_EMU10K1_IOCTL_STOP: 2616 if (!capable(CAP_SYS_ADMIN)) 2617 return -EPERM; 2618 if (emu->audigy) 2619 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg |= A_DBG_SINGLE_STEP); 2620 else 2621 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg |= EMU10K1_DBG_SINGLE_STEP); 2622 return 0; 2623 case SNDRV_EMU10K1_IOCTL_CONTINUE: 2624 if (!capable(CAP_SYS_ADMIN)) 2625 return -EPERM; 2626 if (emu->audigy) 2627 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg = 0); 2628 else 2629 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg = 0); 2630 return 0; 2631 case SNDRV_EMU10K1_IOCTL_ZERO_TRAM_COUNTER: 2632 if (!capable(CAP_SYS_ADMIN)) 2633 return -EPERM; 2634 if (emu->audigy) 2635 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg | A_DBG_ZC); 2636 else 2637 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg | EMU10K1_DBG_ZC); 2638 udelay(10); 2639 if (emu->audigy) 2640 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg); 2641 else 2642 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg); 2643 return 0; 2644 case SNDRV_EMU10K1_IOCTL_SINGLE_STEP: 2645 if (!capable(CAP_SYS_ADMIN)) 2646 return -EPERM; 2647 if (get_user(addr, (unsigned int __user *)argp)) 2648 return -EFAULT; 2649 if (emu->audigy) { 2650 if (addr > A_DBG_STEP_ADDR) 2651 return -EINVAL; 2652 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg |= A_DBG_SINGLE_STEP); 2653 udelay(10);
2654 snd_emu10k1_ptr_write(emu, A_DBG, 0, emu->fx8010.dbg | A_DBG_STEP | addr);
2655 } else { 2656 if (addr > EMU10K1_DBG_SINGLE_STEP_ADDR) 2657 return -EINVAL; 2658 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg |= EMU10K1_DBG_SINGLE_STEP); 2659 udelay(10); 2660 snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg | EMU10K1_DBG_STEP | addr); 2661 } 2662 return 0; 2663 case SNDRV_EMU10K1_IOCTL_DBG_READ: 2664 if (emu->audigy) 2665 addr = snd_emu10k1_ptr_read(emu, A_DBG, 0); 2666 else 2667 addr = snd_emu10k1_ptr_read(emu, DBG, 0); 2668 if (put_user(addr, (unsigned int __user *)argp)) 2669 return -EFAULT; 2670 return 0; 2671 } 2672 return -ENOTTY; 2673 } 2674
On Sat, 22 Apr 2023 18:10:20 +0200, Oswald Buddenhagen wrote:
- Use correct address limit for Audigy
- Use the right constant to actually make a step on Audigy
- Don't store *_DBG_STEP and the address in emu->fx8010.dbg, as otherwise unrelated operations would make steps, too
This is untested. as10k1 was never ported to Audigy anyway.
Signed-off-by: Oswald Buddenhagen oswald.buddenhagen@gmx.de
Applied both patches now. Although 0-day bot caught some build error, I couldn't reproduce it, so took as is. Let's see whether it's broken at some config combo.
thanks,
Takashi
On Sun, Apr 23, 2023 at 09:34:55AM +0200, Takashi Iwai wrote:
Although 0-day bot caught some build error, I couldn't reproduce it, so took as is. Let's see whether it's broken at some config combo.
undeclared logical dependency on the "fixup DSP defines" patch, i think.
but even without that mistake the bot would croak, because i made my tooling describe deps in a human-readable way rather than sha1 + patch-ids. i guess i'll have to include both to make it bot-friendly ...
regards
participants (3)
-
kernel test robot -
Oswald Buddenhagen -
Takashi Iwai