[alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document

[Kuninori Morimoto]

Hi Sylwester

Thank you for your feedback

> > +Simple-Graph-Card:
>
> This binding is likely going to cover most of existing sound system
> configuration, at least it looks as it has a potential to be scalable
> enough.  So perhaps we could treat is as a more generic binding and
> could drop the "Simple" word now?  I'm not 100% sure it's a good idea,
> but maybe we could make it "Generic Sound Card DT Binding"?
(snip)
> > +Required properties:
> > +
> > +- compatible				: "asoc-simple-graph-card";
>
> "asoc" is Linux related, we shouldn't be putting this in the compatible string.
> Perhaps "generic-sound-card" ? Again, I'm sure if it is expected to have such
> a generic kind of DT binding.

This is OF-graph version of existing "simple-card"
(sound/soc/generic/simple-card.c).
As you know, ASoC sound card can have many features, but what this sound card
can do is just CPU-Codec connection and some small additional things.
You can use this card if you want to just connect CPU-Codec,
but if you want to use more advanced feature on your sound card,
then, you need to create your own sound card.
Of course we can expand simple card, but has some limitation.

For example, we have simple-scu-card.c which is for DPCM version of simple card.
As you can see, these are using almost same DT bindings, but using different
feature because normal sound card and DPCM sound card are totally different.
Thus, unfortunately, using "generic" in compatible is a little bit over-kill.
So I and Mark had named it as "simple" card.
Thus I want to keep this "simple" on this OF-graph version driver too.

> > +Example: Single DAI case
> > +
> > +	sound_card: sound {
> > +		compatible = "asoc-simple-graph-card";
> > +
> > +		ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			port at 0 {
> > +				sound0_in: endpoint {
> > +					remote-endpoint = <&rsnd_port0>;
> > +
> > +					simple-audio-card,format = "left_j";
> > +					simple-audio-card,bitclock-master = <&rsnd_port0>;
> > +					simple-audio-card,frame-master = <&rsnd_port0>;
> > +				};
> > +			};
> > +			port at 1 {
> > +				sound0_out: endpoint {
> > +					remote-endpoint = <&ak4613_port>;
> > +				};
> > +			};
> > +		};
> > +	};
>
> I wouldn't be making a separate case for single DAI. The 'ports' node can
> be omitted, port at 0, port at 1 nodes could be put under respective device nodes
> and in the 'sound' node we would have 'dais' property pointing to the CPU
> DAI port,  not the endpoint.  The endpoint is supposed to describe one of
> possible configurations of the port.  I think we want phandles to the 'port'
> nodes, not phandles to the 'endpoint' nodes in the 'sound' node.

Last 2 line was unfortunately not clear for me. do you mean
dais = <&cpu_port>; on card ?
This driver want to handle both single/multi DAI connection,
so, using same rule is more easy and simple.
If my understanding was correct, do you mean like below ?

	cpu: cpu {
		...
		cpu_port: port {
			cpu_out: endpoint {
				remote-endpoint = <&codec_in>;
			};
		};
	};

	codec: codec {
		...
		codec_port: port {
			codec_in: endpoint {
				remote-endpoint = <&cpu_out>;
			};
		};
	};

	card {
		compatible = "asoc-simple-graph-card";

		dais = <&cpu_port>;
	};

If so, I want Multi DAI like this

	cpu: cpu {
		...
		ports {
			cpu0_port: port {
				cpu0_out: endpoint {
					remote-endpoint = <&codec0_in>;
				};
			};
			cpu1_port: port {
				cpu1_out: endpoint {
					remote-endpoint = <&codec1_in>;
				};
			};
		};
	};

	codec0: codec at 0 {
		...
		codec0_port: port {
			codec0_in: endpoint {
				remote-endpoint = <&cpu0_out>;
			};
		};
	};

	codec1: codec at 1 {
		...
		codec1_port: port {
			codec1_in: endpoint {
				remote-endpoint = <&cpu1_out>;
			};
		};
	};

	card {
		compatible = "asoc-simple-graph-card";

		dais = <&cpu0_port
			&cpu1_port>;
	};


Best regards
---
Kuninori Morimoto