Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1384067 > unrolled thread

[PATCH 09/18] ASoC: sti: Update DT example to match the driver code

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 09/18] ASoC: sti: Update DT example to match the driver code Peter Griffin <peter.griffin@linaro.org> - 2016-04-21 13:10 +0200
    Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code Arnd Bergmann <arnd@arndb.de> - 2016-04-21 13:30 +0200
      Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver  code Peter Griffin <peter.griffin@linaro.org> - 2016-04-26 12:20 +0200
        Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code Arnd Bergmann <arnd@arndb.de> - 2016-04-26 13:00 +0200
          Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver  code Peter Griffin <peter.griffin@linaro.org> - 2016-04-26 13:20 +0200
            Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code Arnd Bergmann <arnd@arndb.de> - 2016-04-26 13:50 +0200
    Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver  code Mark Brown <broonie@kernel.org> - 2016-04-21 18:00 +0200
      Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver  code Peter Griffin <peter.griffin@linaro.org> - 2016-04-26 13:10 +0200

#1384067 — [PATCH 09/18] ASoC: sti: Update DT example to match the driver code

uniperiph-id, version and mode are ST specific bindings and
need the 'st,' prefix. Update the examples, as otherwise copying
them yields a runtime error parsing the DT node.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
Cc: arnaud.pouliquen@st.com
---
 .../devicetree/bindings/sound/st,sti-asoc-card.txt         | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
index 028fa1c..ef2e0c6 100644
--- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
+++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
@@ -67,9 +67,9 @@ Example:
 		dmas = <&fdma0 4 0 1>;
 		dai-name = "Uni Player #1 (DAC)";
 		dma-names = "tx";
-		uniperiph-id = <2>;
-		version = <5>;
-		mode = "PCM";
+		st,uniperiph-id = <2>;
+		st,version = <5>;
+		st,mode = "PCM";
 	};
 
 	sti_uni_player3: sti-uni-player@3 {
@@ -83,9 +83,9 @@ Example:
 		dmas = <&fdma0 7 0 1>;
 		dma-names = "tx";
 		dai-name = "Uni Player #1 (PIO)";
-		uniperiph-id = <3>;
-		version = <5>;
-		mode = "SPDIF";
+		st,uniperiph-id = <3>;
+		st,version = <5>;
+		st,mode = "SPDIF";
 	};
 
 	sti_uni_reader1: sti-uni-reader@1 {
@@ -98,7 +98,7 @@ Example:
 		dmas = <&fdma0 6 0 1>;
 		dma-names = "rx";
 		dai-name = "Uni Reader #1 (HDMI RX)";
-		version = <3>;
+		st,version = <3>;
 	};
 
 2) sti-sas-codec: internal audio codec IPs driver
-- 
1.9.1

[toc] | [next] | [standalone]

#1384097

On Thursday 21 April 2016 12:04:26 Peter Griffin wrote:
> uniperiph-id, version and mode are ST specific bindings and
> need the 'st,' prefix. Update the examples, as otherwise copying
> them yields a runtime error parsing the DT node.
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> Cc: arnaud.pouliquen@st.com
> ---
>  .../devicetree/bindings/sound/st,sti-asoc-card.txt         | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> index 028fa1c..ef2e0c6 100644
> --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> @@ -67,9 +67,9 @@ Example:
>                 dmas = <&fdma0 4 0 1>;
>                 dai-name = "Uni Player #1 (DAC)";
>                 dma-names = "tx";
> -               uniperiph-id = <2>;
> -               version = <5>;
> -               mode = "PCM";
> +               st,uniperiph-id = <2>;
> +               st,version = <5>;
> +               st,mode = "PCM";
>         };

You don't change the binding desciption here, only the example,
so they no longer match.

What is st,uniperiph-id needed for anyway? It's often an indication
that you are doing something wrong if you need this.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1387328 — Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code

Hi Arnd,

On Thu, 21 Apr 2016, Arnd Bergmann wrote:

> On Thursday 21 April 2016 12:04:26 Peter Griffin wrote:
> > uniperiph-id, version and mode are ST specific bindings and
> > need the 'st,' prefix. Update the examples, as otherwise copying
> > them yields a runtime error parsing the DT node.
> > 
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > Cc: arnaud.pouliquen@st.com
> > ---
> >  .../devicetree/bindings/sound/st,sti-asoc-card.txt         | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > index 028fa1c..ef2e0c6 100644
> > --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > @@ -67,9 +67,9 @@ Example:
> >                 dmas = <&fdma0 4 0 1>;
> >                 dai-name = "Uni Player #1 (DAC)";
> >                 dma-names = "tx";
> > -               uniperiph-id = <2>;
> > -               version = <5>;
> > -               mode = "PCM";
> > +               st,uniperiph-id = <2>;
> > +               st,version = <5>;
> > +               st,mode = "PCM";
> >         };
> 
> You don't change the binding desciption here, only the example,
> so they no longer match.

Whoops. Will fix that in v4.

> 
> What is st,uniperiph-id needed for anyway? It's often an indication
> that you are doing something wrong if you need this.

From looking at the code in sound/soc/sti/uniperif_player.c, there is
one sysconf register called "Audio glue config" which is shared by all
of the uniperif IP instances. This binding is being used to generate a
bitoffset into this shared register based on the instance of the IP.

I guess the alternative is to have an explosion of compatibles?

st,sti-uni-player-1
st,sti-uni-player-2
st,sti-uni-player-3

If not what would you recommend instead? :-)

I don't currently have access to the functional spec for this
IP block, but I have asked ST to send it to me to see if there is any
other way to derive this information (although I suspect there won't
be).

FYI I didn't actually write or upstream that driver. However fdma is
a depedency of the ASoC driver and it is required to get working audio
upstream. It's also worth pointing out that this ASoC driver has been
merged for a while, so I'm not sure what your opinion is of now changing
the DT bindings? Obviously it is currently not used by anyone upstream
due to the missing fdma depedency.

regards,

Peter.

[toc] | [prev] | [next] | [standalone]

#1387366

On Tuesday 26 April 2016 11:11:36 Peter Griffin wrote:
> Hi Arnd,
> 
> On Thu, 21 Apr 2016, Arnd Bergmann wrote:
> 
> > On Thursday 21 April 2016 12:04:26 Peter Griffin wrote:
> > > uniperiph-id, version and mode are ST specific bindings and
> > > need the 'st,' prefix. Update the examples, as otherwise copying
> > > them yields a runtime error parsing the DT node.
> > > 
> > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > Cc: arnaud.pouliquen@st.com
> > > ---
> > >  .../devicetree/bindings/sound/st,sti-asoc-card.txt         | 14 +++++++-------
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > index 028fa1c..ef2e0c6 100644
> > > --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > @@ -67,9 +67,9 @@ Example:
> > >                 dmas = <&fdma0 4 0 1>;
> > >                 dai-name = "Uni Player #1 (DAC)";
> > >                 dma-names = "tx";
> > > -               uniperiph-id = <2>;
> > > -               version = <5>;
> > > -               mode = "PCM";
> > > +               st,uniperiph-id = <2>;
> > > +               st,version = <5>;
> > > +               st,mode = "PCM";
> > >         };
> > 
> > You don't change the binding desciption here, only the example,
> > so they no longer match.
> 
> Whoops. Will fix that in v4.
> 
> > 
> > What is st,uniperiph-id needed for anyway? It's often an indication
> > that you are doing something wrong if you need this.
> 
> From looking at the code in sound/soc/sti/uniperif_player.c, there is
> one sysconf register called "Audio glue config" which is shared by all
> of the uniperif IP instances. This binding is being used to generate a
> bitoffset into this shared register based on the instance of the IP.
> 
> I guess the alternative is to have an explosion of compatibles?
> 
> st,sti-uni-player-1
> st,sti-uni-player-2
> st,sti-uni-player-3
> 

That would certainly be worse: if the devices are actually identical,
they should have the same compatible string.

> If not what would you recommend instead? :-)

It's still not clear to me what that bit in the syscfg register
is for. Given the error message about "sti-audio-clk-glue",
I suspect that this is actually a clock controller and that
it should be using the clock binding with a separate driver
instead of manipulating the regmap directly from the audio driver.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1387382 — Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code

Hi Arnd,

On Tue, 26 Apr 2016, Arnd Bergmann wrote:

> On Tuesday 26 April 2016 11:11:36 Peter Griffin wrote:
> > Hi Arnd,
> > 
> > On Thu, 21 Apr 2016, Arnd Bergmann wrote:
> > 
> > > On Thursday 21 April 2016 12:04:26 Peter Griffin wrote:
> > > > uniperiph-id, version and mode are ST specific bindings and
> > > > need the 'st,' prefix. Update the examples, as otherwise copying
> > > > them yields a runtime error parsing the DT node.
> > > > 
> > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> > > > Cc: arnaud.pouliquen@st.com
> > > > ---
> > > >  .../devicetree/bindings/sound/st,sti-asoc-card.txt         | 14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > > index 028fa1c..ef2e0c6 100644
> > > > --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > > +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt
> > > > @@ -67,9 +67,9 @@ Example:
> > > >                 dmas = <&fdma0 4 0 1>;
> > > >                 dai-name = "Uni Player #1 (DAC)";
> > > >                 dma-names = "tx";
> > > > -               uniperiph-id = <2>;
> > > > -               version = <5>;
> > > > -               mode = "PCM";
> > > > +               st,uniperiph-id = <2>;
> > > > +               st,version = <5>;
> > > > +               st,mode = "PCM";
> > > >         };
> > > 
> > > You don't change the binding desciption here, only the example,
> > > so they no longer match.
> > 
> > Whoops. Will fix that in v4.
> > 
> > > 
> > > What is st,uniperiph-id needed for anyway? It's often an indication
> > > that you are doing something wrong if you need this.
> > 
> > From looking at the code in sound/soc/sti/uniperif_player.c, there is
> > one sysconf register called "Audio glue config" which is shared by all
> > of the uniperif IP instances. This binding is being used to generate a
> > bitoffset into this shared register based on the instance of the IP.
> > 
> > I guess the alternative is to have an explosion of compatibles?
> > 
> > st,sti-uni-player-1
> > st,sti-uni-player-2
> > st,sti-uni-player-3
> > 
> 
> That would certainly be worse: if the devices are actually identical,
> they should have the same compatible string.
> 
> > If not what would you recommend instead? :-)
> 
> It's still not clear to me what that bit in the syscfg register
> is for. Given the error message about "sti-audio-clk-glue",
> I suspect that this is actually a clock controller and that
> it should be using the clock binding with a separate driver
> instead of manipulating the regmap directly from the audio driver.

Luckily I do have the datasheet for the audio-glue sysconf register.

It says: -

[11:8] PCM_CLK_SEL: Selects the frequency synthesizer clock or the external
PCM clock for each channel.

The driver only ever sets this to 1 which selects the frequency synthesizer
clock. So the bitfield of the register which the driver is using (PCM_CLK_SEL)
is a clock mux.

Peter.

[toc] | [prev] | [next] | [standalone]

#1387399

On Tuesday 26 April 2016 12:15:32 Peter Griffin wrote:
> > 
> > > If not what would you recommend instead? 
> > 
> > It's still not clear to me what that bit in the syscfg register
> > is for. Given the error message about "sti-audio-clk-glue",
> > I suspect that this is actually a clock controller and that
> > it should be using the clock binding with a separate driver
> > instead of manipulating the regmap directly from the audio driver.
> 
> Luckily I do have the datasheet for the audio-glue sysconf register.
> 
> It says: -
> 
> [11:8] PCM_CLK_SEL: Selects the frequency synthesizer clock or the external
> PCM clock for each channel.
> 
> The driver only ever sets this to 1 which selects the frequency synthesizer
> clock. So the bitfield of the register which the driver is using (PCM_CLK_SEL)
> is a clock mux.

Ok, that sounds like it could be either a really simple clock driver
with just a few lines, or integrated into an existing clock driver
if you already have one for this syscon node.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1384389 — Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code

[Multipart message — attachments visible in raw view] — view raw

On Thu, Apr 21, 2016 at 12:04:26PM +0100, Peter Griffin wrote:
> uniperiph-id, version and mode are ST specific bindings and
> need the 'st,' prefix. Update the examples, as otherwise copying
> them yields a runtime error parsing the DT node.

I'm not sure what connection this or the other ASoC documentation update
have to the rest of the series?

[toc] | [prev] | [next] | [standalone]

#1387373 — Re: [PATCH 09/18] ASoC: sti: Update DT example to match the driver code

Hi Mark,

On Thu, 21 Apr 2016, Mark Brown wrote:

> On Thu, Apr 21, 2016 at 12:04:26PM +0100, Peter Griffin wrote:
> > uniperiph-id, version and mode are ST specific bindings and
> > need the 'st,' prefix. Update the examples, as otherwise copying
> > them yields a runtime error parsing the DT node.
> 
> I'm not sure what connection this or the other ASoC documentation update
> have to the rest of the series?

The ASoC DT bindings are missing upstream as the ASoC driver depends on the
fdma driver. So this series adds the fdma driver, fdma dt bindings,
and also the missing ASoC dt bindings.

Whilst doing this and getting it working I noticed a few discrepencies
in the ASoC dt binding documentation versus the driver so also fixed that.

Once this whole series is applied the end result is working
audio upstream for STi platforms (as ASoC is the only upstream driver
using fdma currently).

regards,

Peter.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web