Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, Mar 19, 2014 at 08:32:06PM +0200, Jyri Sarha wrote:
> On 03/19/2014 03:46 PM, Mark Brown wrote:
> >It seems it'd be a bit more idiomatic to do that with a phandle rather
> >than with a string in order to allow extensions for things like TDM (the
> >I2S to mono speaker driver use case for example).
> You mean a like this:
> sound {
> bitclock-master = < 0>
> frame-master = < 0>;
> bitclock-inversion = <1>;
> simple-audio-card,cpu {
Possibly referring to the subnodes rather than the device but yeah.
signature.asc
Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On 03/19/2014 03:46 PM, Mark Brown wrote:
On Wed, Mar 19, 2014 at 12:08:55PM +0200, Jyri Sarha wrote:
While we are at it we could update the bitclock-master and frame-master
syntax to be like this:
bitclock-master = "cpu"
frame-master = "codec"
With the above explicit definition all the daifmt settings could be defined
in link level. For backwards compatibility we could still define that
omitting the value equals "codec" and omitting the property equals "cpu".
It seems it'd be a bit more idiomatic to do that with a phandle rather
than with a string in order to allow extensions for things like TDM (the
I2S to mono speaker driver use case for example).
You mean a like this:
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "Simple Audio";
simple-audio-card,widgets = ...
simple-audio-card,routing = ...
simple-audio-card,dai-link@0 {/* I2S - codec */
format = "i2s";
bitclock-master = < 0>
frame-master = < 0>;
bitclock-inversion = <1>;
simple-audio-card,cpu {
sound-dai = < 0>;
bitclock-inversion = <0>;
};
simple-audio-card,codec {
sound-dai = < 0>;
system-clock-frequency = <1200>;
};
};
...
Yep, that makes sense when considering tdm setups with multiple codecs
on the same wires.
Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, 19 Mar 2014 12:08:55 +0200
Jyri Sarha wrote:
> sound {
> compatible = "simple-audio-card";
> simple-audio-card,name = "Simple Audio";
> simple-audio-card,widgets = ...
> simple-audio-card,routing = ...
>
> simple-audio-card,dai-link@0 { /* I2S - codec */
> format = "i2s";
> bitclock-master = "codec";
> frame-master = "codec";
> bitclock-inversion = <1>;
> simple-audio-card,cpu {
> sound-dai = < 0>;
> bitclock-inversion = <0>;
> };
> simple-audio-card,codec {
> sound-dai = < 0>;
> system-clock-frequency = <1200>;
> };
> };
> ...
>
> I can participate in the implementation too.
Thanks. I will prepare the multi DAI links and send you the first
patches.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, Mar 19, 2014 at 12:08:55PM +0200, Jyri Sarha wrote: > While we are at it we could update the bitclock-master and frame-master > syntax to be like this: > bitclock-master = "cpu" > frame-master = "codec" > With the above explicit definition all the daifmt settings could be defined > in link level. For backwards compatibility we could still define that > omitting the value equals "codec" and omitting the property equals "cpu". It seems it'd be a bit more idiomatic to do that with a phandle rather than with a string in order to allow extensions for things like TDM (the I2S to mono speaker driver use case for example). signature.asc Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On 03/18/2014 10:17 AM, Jean-Francois Moine wrote:
On Mon, 17 Mar 2014 16:43:39 +
Mark Brown wrote:
On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
[snip]
...
I agree. I see two possible syntaxes:
1) keep the same definitions in the containers:
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "Cubox Audio";
simple-audio-card,dai-link@0 { /* I2S - HDMI */
simple-audio-card,cpu {
sound-dai = < 0>;
format = "i2s";
};
simple-audio-card,codec {
sound-dai = < 0>;
};
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
simple-audio-card,cpu {
sound-dai = < 1>;
};
simple-audio-card,codec {
sound-dai = < 1>;
}
};
...
I vote for the version above. As Mark said there is need for dai
specific properties.
While we are at it we could update the bitclock-master and frame-master
syntax to be like this:
bitclock-master = "cpu"
frame-master = "codec"
With the above explicit definition all the daifmt settings could be
defined in link level. For backwards compatibility we could still define
that omitting the value equals "codec" and omitting the property equals
"cpu".
It may sometimes be helpful to allow overwriting link level settings in
dai level. In order to do that it should be possible to write all daifmt
settings explicitly like this:
bitclock-inversion = <0>; /* <0> = no bitclock-inversion */
If backward compatibility is necessary we could recognize the syntax
version from the existence dai-link node.
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "Simple Audio";
simple-audio-card,widgets = ...
simple-audio-card,routing = ...
simple-audio-card,dai-link@0 { /* I2S - codec */
format = "i2s";
bitclock-master = "codec";
frame-master = "codec";
bitclock-inversion = <1>;
simple-audio-card,cpu {
sound-dai = < 0>;
bitclock-inversion = <0>;
};
simple-audio-card,codec {
sound-dai = < 0>;
system-clock-frequency = <1200>;
};
};
...
I can participate in the implementation too.
Best regards,
Jyro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On 03/18/2014 10:17 AM, Jean-Francois Moine wrote:
On Mon, 17 Mar 2014 16:43:39 +
Mark Brown broo...@kernel.org wrote:
On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
[snip]
...
I agree. I see two possible syntaxes:
1) keep the same definitions in the containers:
sound {
compatible = simple-audio-card;
simple-audio-card,name = Cubox Audio;
simple-audio-card,dai-link@0 { /* I2S - HDMI */
simple-audio-card,cpu {
sound-dai = audio1 0;
format = i2s;
};
simple-audio-card,codec {
sound-dai = tda998x 0;
};
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
simple-audio-card,cpu {
sound-dai = audio1 1;
};
simple-audio-card,codec {
sound-dai = tda998x 1;
}
};
...
I vote for the version above. As Mark said there is need for dai
specific properties.
While we are at it we could update the bitclock-master and frame-master
syntax to be like this:
bitclock-master = cpu
frame-master = codec
With the above explicit definition all the daifmt settings could be
defined in link level. For backwards compatibility we could still define
that omitting the value equals codec and omitting the property equals
cpu.
It may sometimes be helpful to allow overwriting link level settings in
dai level. In order to do that it should be possible to write all daifmt
settings explicitly like this:
bitclock-inversion = 0; /* 0 = no bitclock-inversion */
If backward compatibility is necessary we could recognize the syntax
version from the existence dai-link node.
sound {
compatible = simple-audio-card;
simple-audio-card,name = Simple Audio;
simple-audio-card,widgets = ...
simple-audio-card,routing = ...
simple-audio-card,dai-link@0 { /* I2S - codec */
format = i2s;
bitclock-master = codec;
frame-master = codec;
bitclock-inversion = 1;
simple-audio-card,cpu {
sound-dai = audio1 0;
bitclock-inversion = 0;
};
simple-audio-card,codec {
sound-dai = codec 0;
system-clock-frequency = 1200;
};
};
...
I can participate in the implementation too.
Best regards,
Jyro
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, Mar 19, 2014 at 12:08:55PM +0200, Jyri Sarha wrote: While we are at it we could update the bitclock-master and frame-master syntax to be like this: bitclock-master = cpu frame-master = codec With the above explicit definition all the daifmt settings could be defined in link level. For backwards compatibility we could still define that omitting the value equals codec and omitting the property equals cpu. It seems it'd be a bit more idiomatic to do that with a phandle rather than with a string in order to allow extensions for things like TDM (the I2S to mono speaker driver use case for example). signature.asc Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, 19 Mar 2014 12:08:55 +0200
Jyri Sarha jsa...@ti.com wrote:
sound {
compatible = simple-audio-card;
simple-audio-card,name = Simple Audio;
simple-audio-card,widgets = ...
simple-audio-card,routing = ...
simple-audio-card,dai-link@0 { /* I2S - codec */
format = i2s;
bitclock-master = codec;
frame-master = codec;
bitclock-inversion = 1;
simple-audio-card,cpu {
sound-dai = audio1 0;
bitclock-inversion = 0;
};
simple-audio-card,codec {
sound-dai = codec 0;
system-clock-frequency = 1200;
};
};
...
I can participate in the implementation too.
Thanks. I will prepare the multi DAI links and send you the first
patches.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On 03/19/2014 03:46 PM, Mark Brown wrote:
On Wed, Mar 19, 2014 at 12:08:55PM +0200, Jyri Sarha wrote:
While we are at it we could update the bitclock-master and frame-master
syntax to be like this:
bitclock-master = cpu
frame-master = codec
With the above explicit definition all the daifmt settings could be defined
in link level. For backwards compatibility we could still define that
omitting the value equals codec and omitting the property equals cpu.
It seems it'd be a bit more idiomatic to do that with a phandle rather
than with a string in order to allow extensions for things like TDM (the
I2S to mono speaker driver use case for example).
You mean a like this:
sound {
compatible = simple-audio-card;
simple-audio-card,name = Simple Audio;
simple-audio-card,widgets = ...
simple-audio-card,routing = ...
simple-audio-card,dai-link@0 {/* I2S - codec */
format = i2s;
bitclock-master = codec 0
frame-master = codec 0;
bitclock-inversion = 1;
simple-audio-card,cpu {
sound-dai = audio1 0;
bitclock-inversion = 0;
};
simple-audio-card,codec {
sound-dai = codec 0;
system-clock-frequency = 1200;
};
};
...
Yep, that makes sense when considering tdm setups with multiple codecs
on the same wires.
Best regards,
Jyri
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Wed, Mar 19, 2014 at 08:32:06PM +0200, Jyri Sarha wrote:
On 03/19/2014 03:46 PM, Mark Brown wrote:
It seems it'd be a bit more idiomatic to do that with a phandle rather
than with a string in order to allow extensions for things like TDM (the
I2S to mono speaker driver use case for example).
You mean a like this:
sound {
bitclock-master = codec 0
frame-master = codec 0;
bitclock-inversion = 1;
simple-audio-card,cpu {
Possibly referring to the subnodes rather than the device but yeah.
signature.asc
Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Tue, Mar 18, 2014 at 09:17:28AM +0100, Jean-Francois Moine wrote: > The 2nd syntax is simpler and clearer, but the properties of the CPU > DAI and of the CODEC DAI are the same in the container (format, clock > and PCM slots). Is this a problem? Yes, things like clocks might be different. We need a way to specify DAI specific properties. > BTW, there is a 'dai_fmt' in the DAI link, but this format is not used > in the simple-card. Why? Could you be more specific please? If you mean the dai_fmt feature of the generic dai_link struct then because of the confusion about what the DAI formats mean which you'll have seen on the list. signature.asc Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Mon, 17 Mar 2014 16:43:39 +
Mark Brown wrote:
> On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
[snip]
> > +sound {
> > + compatible = "simple-audio-card";
> > + simple-audio-card,name = "Cubox Audio";
> > +
> > + simple-audio-card,cpu@0 { /* I2S - HDMI */
> > + sound-dai = < 0>;
> > + format = "i2s";
> > + };
> > + simple-audio-card,codec@0 {
> > + sound-dai = < 0>;
> > + };
> > +
> > + simple-audio-card,cpu@1 { /* S/PDIF - HDMI */
> > + sound-dai = < 1>;
> > + };
> > + simple-audio-card,codec@1 {
> > + sound-dai = < 1>;
> > + };
>
> So, this is not exactly pretty as a binding. I would expect to see the
> links explicitly represented in the DT so you see the two DAIs in each
> link grouped into a container, the above isn't very easy to read and as
> Jyri says this lack of clarity also causes practical problems in that
> there's nowhere to place link specific parameters.
>
> I think what I'd expect to see here is that the simple card can either
> be a container with a link in it directly or a container of links.
I agree. I see two possible syntaxes:
1) keep the same definitions in the containers:
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "Cubox Audio";
simple-audio-card,dai-link@0 { /* I2S - HDMI */
simple-audio-card,cpu {
sound-dai = < 0>;
format = "i2s";
};
simple-audio-card,codec {
sound-dai = < 0>;
};
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
simple-audio-card,cpu {
sound-dai = < 1>;
};
simple-audio-card,codec {
sound-dai = < 1>;
}
};
...
2) new definitions in the container
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "Cubox Audio";
simple-audio-card,dai-link@0 { /* I2S - HDMI */
format = "i2s";
cpu-dai = < 0>;
codec-dai = < 0>;
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
cpu-dai = < 1>;
codec-dai = < 1>;
};
...
The 2nd syntax is simpler and clearer, but the properties of the CPU
DAI and of the CODEC DAI are the same in the container (format, clock
and PCM slots). Is this a problem?
BTW, there is a 'dai_fmt' in the DAI link, but this format is not used
in the simple-card. Why?
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Mon, 17 Mar 2014 16:43:39 +
Mark Brown broo...@kernel.org wrote:
On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
[snip]
+sound {
+ compatible = simple-audio-card;
+ simple-audio-card,name = Cubox Audio;
+
+ simple-audio-card,cpu@0 { /* I2S - HDMI */
+ sound-dai = audio1 0;
+ format = i2s;
+ };
+ simple-audio-card,codec@0 {
+ sound-dai = tda998x 0;
+ };
+
+ simple-audio-card,cpu@1 { /* S/PDIF - HDMI */
+ sound-dai = audio1 1;
+ };
+ simple-audio-card,codec@1 {
+ sound-dai = tda998x 1;
+ };
So, this is not exactly pretty as a binding. I would expect to see the
links explicitly represented in the DT so you see the two DAIs in each
link grouped into a container, the above isn't very easy to read and as
Jyri says this lack of clarity also causes practical problems in that
there's nowhere to place link specific parameters.
I think what I'd expect to see here is that the simple card can either
be a container with a link in it directly or a container of links.
I agree. I see two possible syntaxes:
1) keep the same definitions in the containers:
sound {
compatible = simple-audio-card;
simple-audio-card,name = Cubox Audio;
simple-audio-card,dai-link@0 { /* I2S - HDMI */
simple-audio-card,cpu {
sound-dai = audio1 0;
format = i2s;
};
simple-audio-card,codec {
sound-dai = tda998x 0;
};
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
simple-audio-card,cpu {
sound-dai = audio1 1;
};
simple-audio-card,codec {
sound-dai = tda998x 1;
}
};
...
2) new definitions in the container
sound {
compatible = simple-audio-card;
simple-audio-card,name = Cubox Audio;
simple-audio-card,dai-link@0 { /* I2S - HDMI */
format = i2s;
cpu-dai = audio1 0;
codec-dai = tda998x 0;
};
simple-audio-card,dai-link@1 { /* S/PDIF - HDMI */
cpu-dai = audio1 1;
codec-dai = tda998x 1;
};
...
The 2nd syntax is simpler and clearer, but the properties of the CPU
DAI and of the CODEC DAI are the same in the container (format, clock
and PCM slots). Is this a problem?
BTW, there is a 'dai_fmt' in the DAI link, but this format is not used
in the simple-card. Why?
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Tue, Mar 18, 2014 at 09:17:28AM +0100, Jean-Francois Moine wrote: The 2nd syntax is simpler and clearer, but the properties of the CPU DAI and of the CODEC DAI are the same in the container (format, clock and PCM slots). Is this a problem? Yes, things like clocks might be different. We need a way to specify DAI specific properties. BTW, there is a 'dai_fmt' in the DAI link, but this format is not used in the simple-card. Why? Could you be more specific please? If you mean the dai_fmt feature of the generic dai_link struct then because of the confusion about what the DAI formats mean which you'll have seen on the list. signature.asc Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
> There may be many couples of CPU/CODEC DAI links.
> The example 2 is extracted from the Cubox DT.
Oh, here's some documentation - please include the documentation
before the code, without the documentation the reader is going to have
no idea what the code is supposed to be implementing.
> + There may be one or many couples (simple-audio-card,cpu,
> simple-audio-card,codec)
> + (see example 2).
This doesn't mention how they're matched up.
> +sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "Cubox Audio";
> +
> + simple-audio-card,cpu@0 { /* I2S - HDMI */
> + sound-dai = < 0>;
> + format = "i2s";
> + };
> + simple-audio-card,codec@0 {
> + sound-dai = < 0>;
> + };
> +
> + simple-audio-card,cpu@1 { /* S/PDIF - HDMI */
> + sound-dai = < 1>;
> + };
> + simple-audio-card,codec@1 {
> + sound-dai = < 1>;
> + };
So, this is not exactly pretty as a binding. I would expect to see the
links explicitly represented in the DT so you see the two DAIs in each
link grouped into a container, the above isn't very easy to read and as
Jyri says this lack of clarity also causes practical problems in that
there's nowhere to place link specific parameters.
I think what I'd expect to see here is that the simple card can either
be a container with a link in it directly or a container of links.
signature.asc
Description: Digital signature
Re: [PATCH v3 4/4] ASoC: simple-card: Add DT documentation for multi-DAI links
On Sat, Mar 15, 2014 at 12:30:05PM +0100, Jean-Francois Moine wrote:
There may be many couples of CPU/CODEC DAI links.
The example 2 is extracted from the Cubox DT.
Oh, here's some documentation - please include the documentation
before the code, without the documentation the reader is going to have
no idea what the code is supposed to be implementing.
+ There may be one or many couples (simple-audio-card,cpu,
simple-audio-card,codec)
+ (see example 2).
This doesn't mention how they're matched up.
+sound {
+ compatible = simple-audio-card;
+ simple-audio-card,name = Cubox Audio;
+
+ simple-audio-card,cpu@0 { /* I2S - HDMI */
+ sound-dai = audio1 0;
+ format = i2s;
+ };
+ simple-audio-card,codec@0 {
+ sound-dai = tda998x 0;
+ };
+
+ simple-audio-card,cpu@1 { /* S/PDIF - HDMI */
+ sound-dai = audio1 1;
+ };
+ simple-audio-card,codec@1 {
+ sound-dai = tda998x 1;
+ };
So, this is not exactly pretty as a binding. I would expect to see the
links explicitly represented in the DT so you see the two DAIs in each
link grouped into a container, the above isn't very easy to read and as
Jyri says this lack of clarity also causes practical problems in that
there's nowhere to place link specific parameters.
I think what I'd expect to see here is that the simple card can either
be a container with a link in it directly or a container of links.
signature.asc
Description: Digital signature
