On Fri, Jan 27, 2012 at 18:45, Stephen Warren <swarren@xxxxxxxxxx> wrote: > 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. Sorry for the misunderstanding, This is what i wanted to say. > >> >> + * 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. Sure > >> >> + 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 > -- Leon Romanovsky | Independent Linux Consultant www.leon.nu | leon@xxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html