diff mbox series

[1/2] ALSA: emu10k1: fix SNDRV_EMU10K1_IOCTL_SINGLE_STEP

Message ID 20230422161021.1144004-1-oswald.buddenhagen@gmx.de (mailing list archive)
State Accepted
Commit 375052892172c741b93e8e9303ec186cca7169ab
Headers show
Series [1/2] ALSA: emu10k1: fix SNDRV_EMU10K1_IOCTL_SINGLE_STEP | expand

Commit Message

Oswald Buddenhagen April 22, 2023, 4:10 p.m. UTC 
- 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(-)

Comments

kernel test robot April 22, 2023, 8:25 p.m. UTC | #1 
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-emu10k1-don-t-stop-DSP-in-_snd_emu10k1_-audigy_-init_efx/20230423-002213
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.de
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@intel.com/config)
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/dfbd3fc49e3b9ec81160caee29b1d41476d5a6aa
        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
Takashi Iwai April 23, 2023, 7:34 a.m. UTC | #2 
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
Oswald Buddenhagen April 23, 2023, 7:48 a.m. UTC | #3 
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
diff mbox series

Patch

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)