Subject: Re: autoconf(9) tree in an odd hardware arrangement
To: Marco Trillo <marcotrillo@gmail.com>
From: Michael Lorenz <macallan@netbsd.org>
List: tech-kern
Date: 11/25/2007 11:53:46
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Nov 25, 2007, at 11:31, Marco Trillo wrote:

> On 11/25/07, Michael Lorenz <macallan@netbsd.org> wrote:
>> On Nov 23, 2007, at 16:23, Marco Trillo wrote:
>>
>>> For an experiment, I ran into some sort of, perhaps stupid, autoconf
>>> (9)-
>>> -related doubt.
>>>
>>> The hardware arrangement looks like this (please excuse the
>>> poor ASCII drawing):
>>>
>>> The 'doubt' is what is the most clean way to attach an audio(4) bus
>>> in such an arrangement. Some functions of the audio setup need to
>>> configure the CODEC via the I2C bus, some functions need to  
>>> configure
>>> the 'Audio controller', and some need to configure both devices.
>>
>> Yeah, the problem is that the codec would need to attach to two
>> devices - the i2c bus and the audio controller ( or rather, the DMA
>> engine ) and it needs to deal with them attaching in different order.
>> Is there any audio hardware out there that needs this kind of
>> arrangement and isn't some sort of Apple onboard audio?
>
> Indeed, the experiment was factoring out the I2S controller code of  
> snapper(4)
> and move it to a i2s(4) driver, and then write individual machine- 
> independent
> codec drivers in sys/dev/i2c/ similar to the sgsmix(4) driver; so  
> it's easy
> to write drivers for the other I2S codecs (daca, pcm3052, etc.)  
> without making
> a mess of the snapper driver.
>
> Currently I have this working with the approach A, so it looks like  
> this:
>
> i2s0 at obio0 offset X: Apple I2S controller
> iic1 at ki2c1: I2C bus
> texasdsp0 at iic1 addr Y: [...]
> codec0 at texasdsp0
> codec0: [supported formats]
> [...]
> i2s0: connecting to codec0
> audio0 at i2s0: full duplex
>
> So the question was if this could be done another way that didn't
> involve deferred configuration.

Hard to avoid - on some machines the codec's found first ( the beige  
G3 IIRC, sgsmix hangs on the PMU's i2c bus ) on others it's the other  
way around so whoever is first needs to defer to whoever is last or  
something later.
Not sure if there's a sane way to extend config() to allow devices to  
have multiple parents and attach() only when all parents have been  
found.

>> I have an experimental awacs driver that attaches like this:
>> audiodma* at obio?
>> cawacs* at audiodma?
>> audio* at cawacs?
>>
>> The intention was to factor out all the DBDMA goo that's more or less
>> the same in snapper and awacs so we can also have:
>> snapper* at audiodma?
>> daca* at audiodma?
>> dumbcodec* at audiodma?
>> g3audio* at audiodma?
>
> Then the i2s driver could be attached to audiodma too... so the i2s
> driver would only need to do things like setting up the I2S clocks  
> and configuring or
> dealing with the gpio stuff (headphones, lineouts, etc.).

Something like that - I'm not sure they all use the same gpios. I  
also didn't get around to write snapper at audiodma either so I don't  
have a sane opinion on that problem yet. On awacs clock selection is  
part of the codec's interface, maybe it would be easiest to do the  
same on i2s ( maybe put the i2s clock and gpio stuff - if it really  
is the same on all i2s - into a separate source file and just pull it  
in with those codecs that need it so the DMA engine driver can stay  
nice and clean )

I'm not quite sure what to do about the beige G3's audio - it's a  
bastardized awacs with an i2c mixer on its auxillary output and the  
regular outputs going nowhere. Right now it's a bunch of #if NSGSMIX  
and similar ugliness in awacs.c which I'd like to get rid of.


>> For now there are two drivers that attach as i2c codecs - deq and
>> sgsmix. Deq is a simple marker with no real code while sgsmix exports
>> a bunch of functions to control volume, bass/treble etc. - not sure
>> which approach is better.
>
> In the experiment I did the i2c drivers (currently 'texasdsp')  
> exported
> a 'codec' interface that contains a bunch of methods ('configure',
> 'reset', etc.).
> Then i2s(4) calls the methods as appropriate.

Sounds like the right thing to do. Do you think it would make sense  
to have the codec driver fill in the mixer-related methods in  
audio_attach_data instead? I did something like that in my experiment  
but awacs is rather simplistic, not sure if I'd run into trouble with  
that later on.

>>> B) Similar to A), but inverse: make the 'codec' look for the
>>> controller
>>>    and attach the 'audio' bus to the 'codec'.
>>>    However, the most logic thing seems to be for the 'audio' bus to
>>>    be attached to the audio controller.
>>
>> The more time critical methods in the audio driver API need to talk
>> to the DMA engine, the mixer stuff is much more relaxed. I'd find it
>> more logical to have audio at something as the last piece of the
>> chain instead of having it fork.
>
> Yeah, I do not like approach B either.

Thought so :)

>>> What do you think?
>>
>> I think for the time being we can't really get around the way awacs
>> and snapper find their mixers - use a marker that attaches to i2cbus,
>> identifies if possible and grabs the mixer device, then have the
>> audio driver ( or the real codec driver which attaches to the DMA
>> engine ) look for the marker, All it needs from the marker is a
>> reference to the i2cbus and a device address.
>
> Or in this case, a codec driver that exports an interface, so the
> controller driver (i2s) does not need to use any I2C functions at all.

Yeah, where you put the actual code is a matter of further debate I  
guess.

have fun
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iQEVAwUBR0mom8pnzkX8Yg2nAQIF4gf9HERuWCxHMrBXdCfzAmtxlZuZVrQIe0g6
LlUpUycZuCqwvQIvIW2fu/pB2Z9Ty39ZqSCGOKt+xPMr32ZFbqojf5sGKizxaTkG
Y60Ui/0PbPIlCgm7krD6gBDCEli0HfsdlvA+Vay2NCTwRBV6qzCGZIv88UGcpUF9
i9KWwrVJNj1TDanxZmhEZj4qiW8ORywrvCDFd3HitBzELJCWKPwIk5D3yTb478+F
qIvI1WtdhxpF81zY5ye9+0rQ9ARh+2h1McwXfVk8AotWuP+i69ZR+JuHITUYqELG
z4mYTe1NLr0X1YNu7582o9sOGJMjJ6tmRqNjIYWBrwO63HKpTZMDog==
=Yc1Z
-----END PGP SIGNATURE-----