AACI broken with commit 29a4f2d3

[Catalin Marinas]

On Thu, 2010-03-25 at 11:50 +0000, Philby John wrote:
> On 03/25/2010 05:06 PM, Takashi Iwai wrote:
> > At Thu, 25 Mar 2010 11:30:19 +0000,
> > Russell King - ARM Linux wrote:
> >>
> >> 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.
> >
> > Thanks for checking.  Now I wonder why this fixed the original issue
> > at all.  Obviously something wrong in either testing or problem
> > analysis.
> >
> > Anyway, it's better to revert the patch first...
> 
> This was tested throughly but not for alignment and to verify the fix is
> easy. All you need to do is revert the commit and then type reboot at
> the command prompt. On reboot you will get "aaci-pl041 fpga:04: ac97
> read back fail" failures, rendering audio non-functional on an ARM1176.

I can confirm that it solves the reset issue on other RealView boards as
well. Whether it's the correct fix I can't tell.

-- 
Catalin