On 08/05/2013 06:59 PM, Mark Brown wrote:

On Mon, Aug 05, 2013 at 05:04:35PM +0200, Sebastian Hesselbarth wrote:

quoted

I cannot follow you on the clocking arrangements. The binding
proposal was for audio controller <-> CODEC connections. For clocks
there are common properties ("clocks", "clock-names", "clock-frequency")
to pass them to drivers - for "sound cards" in general there are not.

The clocking arrangements are an example of why the boards can get
interesting enough to describe for themselves.

I understand there are complex arrangements, I still don't think that
you need a global super-node to describe them. Anyway, I am not voting
against a super-node.

quoted

So, having a look at the node in question:

quoted

sound {
   compatible = "nvidia,tegra-audio-rt5640-beaver",
                "nvidia,tegra-audio-rt5640";
   nvidia,model = "NVIDIA Tegra Beaver";

   nvidia,audio-routing = "Headphones", "HPOR",
                          "Headphones", "HPOL";

   nvidia,i2s-controller = <&tegra_i2s1>;
   nvidia,audio-codec = <&rt5640>;

   nvidia,hp-det-gpios = <&gpio TEGRA_GPIO(W, 2) GPIO_ACTIVE_HIGH>;

   clocks = <&tegra_car TEGRA30_CLK_PLL_A>,
            <&tegra_car TEGRA30_CLK_PLL_A_OUT0>,
            <&tegra_car TEGRA30_CLK_EXTERN1>;
   clock-names = "pll_a", "pll_a_out0", "mclk";
};

all those "nvidia," prefixed properties are not nvidia-specific at all.

This is all because you have to add a prefix for DT.

Right, like we have on all the other non-standard properties like
"pinctrl-0", "bla-gpios", "clocks", ... come on, just make them generic
enough and you can omit the vendor prefix.

quoted

Also, all those "nvidia," properties would have fit very well into the
i2s controller node

No, almost none of them have any place there.  For example, the audio
routing contains only connections to the CODEC so the I2S controller
isn't in play, the headphone detection is connected to the AP but isn't
connected to the I2S port.

 From a quick look in tegra30_hub.h, I can see configuration registers
i2s formatting. I assume that i2s controller on Tegra can therefore
directly connected to a I2S codec, can't it? Then, with an existing i2s
node and an existing codec node - why is there no place to link both?

I can think of use cases that are hard to describe in a link-to-link
way, but none of them really requires a super-node nor special
board-specific compatible strings. With that we can just have the root
DT node be compatible with Beaver above and register all the old
platform_device way.

[...]

quoted

I learned to never match "device nodes" with "drivers" as there
is simply no relation between them.

That's clearly a thinko for device node.

I assume with "thinko" you mean "thought wrong" - IMHO the above 
statement is very true. If it wouldn't, why not just have a node for
kirkwood-dma and kirkwood-i2s instead of merging the drivers?
You think that will also be accepted by DT maintainers?

quoted

So, back to my original node proposal, can you tell me what is so
different, except that I am not using "marvell,audio-codec(s)" but
"audio-codecs"; and hook the one available codec output by using
phandle/args instead of a new compatible string?

 From my point of view I'd rather not be shoving vendor prefixes on all
the properties, this is coming from DT convention requiring vendor
prefixes on bindings.

I understand that those vendor prefixes are part of the helper
functions of ASoC. But no other subsystem has a similar approach but
though of properties generic enough to help drivers find what they
need to know. Either ASoC is mis-interpreting the vendor-prefix
requirement - or all other subsystems are.

Sebastian