AACI broken with commit 29a4f2d3

[Russell King - ARM Linux]

On Thu, Mar 25, 2010 at 12:12:52PM +0100, Takashi Iwai wrote:
> At Wed, 24 Mar 2010 15:18:06 -0000,
> Will Deacon wrote:
> > 
> > Hi Catalin,
> > 
> > > aaci: Use writew() to the AC97_POWERDOWN 16-bit register
> > > 
> > > From: Catalin Marinas <catalin.marinas at arm.com>
> > > 
> > > The writel() introduced by commit 29a4f2d3 generates an alignment fault
> > > on ARM.
> > > 
> > > Signed-off-by: Catalin Marinas <catalin.marinas at arm.com>
> > > Cc: Philby John <pjohn at in.mvista.com>
> > > Cc: Takashi Iwai <tiwai at suse.de>
> > > ---
> > >  sound/arm/aaci.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/sound/arm/aaci.c b/sound/arm/aaci.c
> > > index 656e474..d66d4ff 100644
> > > --- a/sound/arm/aaci.c
> > > +++ b/sound/arm/aaci.c
> > > @@ -863,7 +863,7 @@ static int __devinit aaci_probe_ac97(struct aaci *aaci)
> > >  	struct snd_ac97 *ac97;
> > >  	int ret;
> > > 
> > > -	writel(0, aaci->base + AC97_POWERDOWN);
> > > +	writew(0, aaci->base + AC97_POWERDOWN);
> > >  	/*
> > >  	 * Assert AACIRESET for 2us
> > >  	 */
> > 
> > A writel() looks wrong anyway because even if it could succeed, it would
> > send half of its write to AC97_EXTENDED_ID, which sounds suspiciously read-only
> > to me.
> > 
> > Tested-by: Will Deacon <will.deacon at arm.com>
> 
> Looking back at the original patch again, I wonder now whether using
> AC97_* for the register offset here is really correct.  Usually AC97
> registers are accessed indirectly.
> 
> Is it just same 0x26, or is this intentional?

The original patch is total crap.

1. You can't access AC97 registers via writel.
2. If the offset is 0x26 (AACI_CSCH2 + AACI_IE + 2), this is writing to
   reserved bits in channel 2's interrupt enable register which has
   nothing to do with the slot 1/2 transmit registers.

The patch could never have been tested in the first place.