ALSA: intel8x: Why incorrect codec index used setting ICH_DI2L SDIN

[BUG] ALSA: intel8x: Why incorrect codec index used setting ICH_DI2L SDIN

Posted by Alok Tiwari 3 months, 3 weeks ago

Correct the index to use codec[i] to match the loop iteration, not codec[1].

Is this a mistake or intentional?

Thanks,
Alok

propose changes
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
 sound/pci/intel8x0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
index 51e7f1f1a48e..b521cec20333 100644
--- a/sound/pci/intel8x0.c
+++ b/sound/pci/intel8x0.c
@@ -2249,7 +2249,7 @@ static int snd_intel8x0_mixer(struct intel8x0 *chip, int ac97_clock,
 			tmp |= chip->ac97_sdin[0] << ICH_DI1L_SHIFT;
 			for (i = 1; i < 4; i++) {
 				if (pcm->r[0].codec[i]) {
-					tmp |= chip->ac97_sdin[pcm->r[0].codec[1]->num] << ICH_DI2L_SHIFT;
+					tmp |= chip->ac97_sdin[pcm->r[0].codec[i]->num] << ICH_DI2L_SHIFT;
 					break;
 				}
 			}
-- 
2.47.1

Re: [BUG] ALSA: intel8x: Why incorrect codec index used setting ICH_DI2L SDIN

Posted by Andy Shevchenko 3 months, 3 weeks ago

On Wed, Jun 18, 2025 at 12:19:21PM -0700, Alok Tiwari wrote:
> Correct the index to use codec[i] to match the loop iteration, not codec[1].
> 
> Is this a mistake or intentional?

You marked it as a BUG in the Subject, can you elaborate more, please?

-- 
With Best Regards,
Andy Shevchenko

Re: [External] : Re: [BUG] ALSA: intel8x: Why incorrect codec index used setting ICH_DI2L SDIN

Posted by ALOK TIWARI 3 months, 3 weeks ago


On 6/19/2025 9:08 PM, Andy Shevchenko wrote:
> On Wed, Jun 18, 2025 at 12:19:21PM -0700, Alok Tiwari wrote:
>> Correct the index to use codec[i] to match the loop iteration, not codec[1].
>>
>> Is this a mistake or intentional?
> 
> You marked it as a BUG in the Subject, can you elaborate more, please?
> 

I should have mark this as a query.
Why is the value 1 hardcoded here in the codec?

tmp |= chip->ac97_sdin[pcm->r[0].codec[1]->num] << ICH_DI2L_SHIFT;

https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/pci/intel8x0.c#n2252


Thanks,
Alok

Re: [External] : Re: [BUG] ALSA: intel8x: Why incorrect codec index used setting ICH_DI2L SDIN

Posted by Takashi Iwai 3 months, 2 weeks ago

On Thu, 19 Jun 2025 21:26:53 +0200,
ALOK TIWARI wrote:
> 
> 
> 
> On 6/19/2025 9:08 PM, Andy Shevchenko wrote:
> > On Wed, Jun 18, 2025 at 12:19:21PM -0700, Alok Tiwari wrote:
> >> Correct the index to use codec[i] to match the loop iteration, not codec[1].
> >> 
> >> Is this a mistake or intentional?
> > 
> > You marked it as a BUG in the Subject, can you elaborate more, please?
> > 
> 
> I should have mark this as a query.
> Why is the value 1 hardcoded here in the codec?
> 
> tmp |= chip->ac97_sdin[pcm->r[0].codec[1]->num] << ICH_DI2L_SHIFT;
> 
> https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/sound/pci/intel8x0.c#n2252

I believe this is a long-standing bug, but practically seen, it never
hits on real devices, because there have been little devices with
multiple AC97 codecs on Intel ICH, and they have at most only two
codecs, AFAIK.

That said, I'd happily take the fix patch when you submit a properly
formatted one.  It'll likely change the behavior of existing devices,
but it's always good to fix something.

thanks,

Takashi