Leon Romanovsky wrote at Friday, January 27, 2012 9:02 AM: > On Thu, Jan 26, 2012 at 00:21, Stephen Warren <swarren@xxxxxxxxxx> wrote: > > Leon Romanovsky wrote at Wednesday, January 25, 2012 11:49 AM: > >> This patch adds initial device tree support of ALC5632 sound codec and > >> machine driver for PAZ00 board. The implementation is based on the WM8903 codec. > > > >> +++ b/Documentation/devicetree/bindings/sound/tegra-audio-alc5632.txt > >> @@ -0,0 +1,55 @@ > >> +NVIDIA Tegra audio complex > >> + > >> +Required properties: > >> +- compatible : "nvidia,tegra-audio-alc5632" > >> +- nvidia,model : The user-visible name of this sound complex. > >> +- nvidia,audio-routing : A list of the connections between audio components. > >> + Each entry is a pair of strings, the first being the connection's sink, > >> + the second being the connection's source. Valid names for sources and > >> + sinks are the ALC5632's pins: > >> + > >> + ALC5632 pins: > >> + > >> + * SPKOUT > >> + * SPKOUTN > >> + * HPL > >> + * HPR > >> + * AUXOUT > > > > My copy of the ALC5632 datasheet indicates there are both AUX_OUT_P and > > AUX_OUTN pins. Are they always used together such that it makes sense to > > group them together in the device tree binding? > > You are right, it must be the same as SPKOUT I think the opposite; AUXOUT and SPKOUT aren't the same pin. I meant that AUX_OUT_P and AUX_OUT_N are different pins, so I'm asking why they're a single pin in the above pin list (and in the ASoC codec driver). In contrast, SPKOUT is represented explicitly as two pins which seems to make more sense. > >> + * LINEINL > >> + * LINEINR > >> + * PHONEP > >> + * PHONEP > > > > PHONEN > > > >> + * MIC1 > >> + * MIC2 > > > > Same as above; the datasheet lists MIC1_P, MIC1_N, MIC2_P, MIC2_N. > > > >> + * MICBIAS1 > >> + * MICBIAS2 > > > > I only see MICBIAS1 in the datasheet, not MICBIAS2. > > You are right if you are looking on function block only (page 3), but > in register description you can find reference to MICBIAS2 (reg 22h, > microphone control). I do see that register bit, but the pinout doesn't include it in the codec's own datasheet nor the AC100 schematics. I suspect that there really is no MICBIAS2 feature and the register documentation is simply incorrect, perhaps a carry-over from some other codec? Still, I suppose that just in case the signal really is routed to one of the pins labeled "NC" in the datasheet, there isn't too much harm including it in the driver, and this binding. > >> + Board connectors: ... > > Don't you need "Headset Mic" in the list too? ... > >> + nvidia,audio-routing = ... > >> + "MIC1", "MICBIAS1", > >> + "MICBIAS1", "Headset Mic", > > > > I think those last two lines should read: > > > > "Headset Mic", "MICBIAS1", > > "MIC1", " Headset Mic", > > > > The DAPM route table in the driver probably needs updating to say the > > same thing too. > > Microphone is not tested at all, so you probably right. I prefer to be > close as possible to previous board implementation and provide > followup patches to clean the microphone path. I guess that doesn't actually affect the structure of the binding, just the data contained in the properties, so it's not a big deal if it goes in like this. Mark might have a different opinion though, since it's a simple fix... -- nvpublic ��.n��������+%������w��{.n�����{����*jg��������ݢj����G�������j:+v���w�m������w�������h�����٥