[Qemu-devel] Re: [PATCH 01/14] lm32: add Milkymist AC97 support

[Alexander Graf]

On 17.03.2011, at 00:02, Michael Walle <address@hidden> wrote:

> Am Mittwoch 16 März 2011, 19:12:44 schrieb malc:
>>>> diff --git a/hw/milkymist-ac97.c b/hw/milkymist-ac97.c
>>>> new file mode 100644
>>>> index 0000000..6c9e318
>>>> --- /dev/null
>>>> +++ b/hw/milkymist-ac97.c
>>>> @@ -0,0 +1,335 @@
>>>> +/*
>>>> + *  QEMU model of the Milkymist System Controller.
>>>> + *
>>>> + *  Copyright (c) 2010 Michael Walle<address@hidden>
>>>> + *
>>>> + * This library is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2 of the License, or (at your option) any later version.
>>>> + *
>>>> + * This library is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with this library; if not,
>>>> see<http://www.gnu.org/licenses/>.
>>>> + *
>>>> + *
>>>> + * Specification available at:
>>>> + *   http://www.milkymist.org/socdoc/ac97.pdf
>>>> + */
>>>> +
>>>> +#include "hw.h"
>>>> +#include "sysbus.h"
>>>> +#include "trace.h"
>>>> +#include "audio/audio.h"
>>>> +#include "qemu-error.h"
>>>> +
>>>> +enum {
>>>> +    R_AC97_CTRL = 0,
>> 
>> Unneeded..
> I wanted to point out, that the registers begin at offset 0. If i change 
> this, 
> i'll need to change all other models, too. Including the ones that have been 
> committed.
> 
>>>> +    R_AC97_ADDR,
>>>> +    R_AC97_DATAOUT,
>>>> +    R_AC97_DATAIN,
>>>> +    R_D_CTRL,
>>>> +    R_D_ADDR,
>>>> +    R_D_REMAINING,
>>>> +    R_RESERVED,
>>>> +    R_U_CTRL,
>>>> +    R_U_ADDR,
>>>> +    R_U_REMAINING,
>>>> +    R_MAX
>>>> +};
>>>> +
>>>> +enum {
>>>> +    AC97_CTRL_RQEN  = (1<<0),
>> 
>> Oookay..
>> 
>>>> +    AC97_CTRL_WRITE = (1<<1),
>> 
>> Incinsistent(with previous enum formatting) comma (and C99 only at that)
>> 
>>>> +};
>>>> +
>>>> +enum {
>>>> +    CTRL_EN = (1<<0),
>> 
>> Ditto x2
> Ok, but it has a hidden agenda. R_MAX must always be the last entry, 
> therefore 
> i omitted the comma. Whereas the bit flags could still be extended. Again if 
> this is a concern for you, i'll change that and all other models too.
> 
>>>> +};
>>>> +
>>>> +struct MilkymistAC97State {
>>>> +    SysBusDevice busdev;
>>>> +
>>>> +    QEMUSoundCard card;
>>>> +    SWVoiceIn *voice_in;
>>>> +    SWVoiceOut *voice_out;
>>>> +
>>>> +    uint32_t regs[R_MAX];
>>>> +
>>>> +    qemu_irq crrequest_irq;
>>>> +    qemu_irq crreply_irq;
>>>> +    qemu_irq dmar_irq;
>>>> +    qemu_irq dmaw_irq;
>>>> +};
>>>> +typedef struct MilkymistAC97State MilkymistAC97State;
>>>> +
>>>> +static void update_voices(MilkymistAC97State *s)
>>>> +{
>>>> +    if (s->regs[R_D_CTRL]&  CTRL_EN) {
>> 
>> Space before ampersand would be nice.
> mh i think Alex email client messed this up. At least for me, there is one 
> space before and one space after the ampersand in my original email.

Yes, thunderbird breaks those. Sorry :).

Alex

> 
> -- 
> Michael