| Message ID | 1461236675-10176-11-git-send-email-peter.griffin@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 21, 2016 at 12:04:27PM +0100, Peter Griffin wrote: > clocks = <&clk_s_d0_flexgen CLK_PCM_2>; > + assigned-clocks = <&clk_s_d0_flexgen CLK_PCM_2>; > + assigned-clock-parents = <&clk_s_d0_quadfs 2>; > + assigned-clock-rates = <50000000>; This may be true for the particular system you're looking at but isn't really relevant to the device. > @@ -133,6 +139,7 @@ Example of audio card declaration: > /* DAC */ > format = "i2s"; > dai-tdm-slot-width = <32>; > + mclk-fs = <256>; > cpu { > sound-dai = <&sti_uni_player2>; > }; This one is more relevant though I'm still a bit concerned that there's an expectation that the examples can just be pasted in...
Hi Mark, On Thu, 21 Apr 2016, Mark Brown wrote: > On Thu, Apr 21, 2016 at 12:04:27PM +0100, Peter Griffin wrote: > > > clocks = <&clk_s_d0_flexgen CLK_PCM_2>; > > + assigned-clocks = <&clk_s_d0_flexgen CLK_PCM_2>; > > + assigned-clock-parents = <&clk_s_d0_quadfs 2>; > > + assigned-clock-rates = <50000000>; > > This may be true for the particular system you're looking at but isn't > really relevant to the device. Ok I will drop this from v4 then. > > > @@ -133,6 +139,7 @@ Example of audio card declaration: > > /* DAC */ > > format = "i2s"; > > dai-tdm-slot-width = <32>; > > + mclk-fs = <256>; > > cpu { > > sound-dai = <&sti_uni_player2>; > > }; > > This one is more relevant though I'm still a bit concerned that there's > an expectation that the examples can just be pasted in... I agree with your point in a more general case (e.g. seperate audio codec chip connected via SPI), but the reality of the situation for STi arch is that this device is found in STiH407 family silicon, of which the two supported upstream devices (STiH407/410) are almost identical SoC's. Given the recent announcement of "no new SoC's" for STB market, there is unlikely to be support upstream for new variants and as there is no community board available new board variants upstream are also unlikely (so far only ST reference boards are supported upstreamed). So although I would only consider copying an example as the starting point for getting something working (and indeed it was the case here). On STi arch there *should* be a very high probability that copying the exanple is all that will be required. Peter.
On Tue, Apr 26, 2016 at 12:52:10PM +0100, Peter Griffin wrote: > On Thu, 21 Apr 2016, Mark Brown wrote: > > This one is more relevant though I'm still a bit concerned that there's > > an expectation that the examples can just be pasted in... > So although I would only consider copying an example as the starting > point for getting something working (and indeed it was the case here). On STi > arch there *should* be a very high probability that copying the exanple > is all that will be required. If there are never going to be any new users why bother at all, why would anyone paste it in when all the SoCs that ever use this are already upstream?
Hi Mark, On Tue, 26 Apr 2016, Mark Brown wrote: > On Tue, Apr 26, 2016 at 12:52:10PM +0100, Peter Griffin wrote: > > On Thu, 21 Apr 2016, Mark Brown wrote: > > > > This one is more relevant though I'm still a bit concerned that there's > > > an expectation that the examples can just be pasted in... > > > So although I would only consider copying an example as the starting > > point for getting something working (and indeed it was the case here). On STi > > arch there *should* be a very high probability that copying the exanple > > is all that will be required. > > If there are never going to be any new users why bother at all, why > would anyone paste it in when all the SoCs that ever use this are > already upstream? Generally if I come across documentation which is incorrect, I like to fix it. Currently the DT ASoC documentation doesn't match the driver and the example doesn't work, which is not ideal. I could well be the last person who needs to read it & use the example, but deliberately leaving it with mistakes in seemed like a bad idea. Peter.
On Tue, Apr 26, 2016 at 03:51:29PM +0100, Peter Griffin wrote: > On Tue, 26 Apr 2016, Mark Brown wrote: > > On Tue, Apr 26, 2016 at 12:52:10PM +0100, Peter Griffin wrote: > > If there are never going to be any new users why bother at all, why > > would anyone paste it in when all the SoCs that ever use this are > > already upstream? > Generally if I come across documentation which is incorrect, I like to fix it. A lot of this is details of the system integration for this SoC, not actual errors. > Currently the DT ASoC documentation doesn't match the driver and the example > doesn't work, which is not ideal. I could well be the last person who needs > to read it & use the example, but deliberately leaving it with mistakes in > seemed like a bad idea. People might end up trying to use it as an example, it's fairly routine to have to explain to people that just because some old driver did something that doesn't mean it's something we want in new drivers.
Hi Mark, On Tue, 26 Apr 2016, Mark Brown wrote: > On Tue, Apr 26, 2016 at 03:51:29PM +0100, Peter Griffin wrote: > > On Tue, 26 Apr 2016, Mark Brown wrote: > > > On Tue, Apr 26, 2016 at 12:52:10PM +0100, Peter Griffin wrote: > > > > If there are never going to be any new users why bother at all, why > > > would anyone paste it in when all the SoCs that ever use this are > > > already upstream? > > > Generally if I come across documentation which is incorrect, I like to fix it. > > A lot of this is details of the system integration for this SoC, not > actual errors. This particular clock patch yes, but the other ASoC dt doc update is fixing bindings which haven't progressed in lockstep with the driver code. Presumably this happened during the review process when they were changed to being st, prefixed in the driver, but the doc wasn't also updated. > > > Currently the DT ASoC documentation doesn't match the driver and the example > > doesn't work, which is not ideal. I could well be the last person who needs > > to read it & use the example, but deliberately leaving it with mistakes in > > seemed like a bad idea. > > People might end up trying to use it as an example, Yes I know..I did exactly that and it didn't work. That's why I submitted a patch :) > it's fairly routine > to have to explain to people that just because some old driver did > something that doesn't mean it's something we want in new drivers. I'm sure it is. Although I fail to see why leaving the documentation with mistakes in is helpful to anybody. As you can see by this set, and others on the mailing lists, the ST landing team are actively working on upstreaming and supporting these SoC's. Although we have come a long way we aren't finished. Lee is working on remoteproc rpmsg drivers for the video co-processors and we already have a DRM display, ASoC, LinuxDVB tsin. So soon it will be possible to have full end to end playback from a live feed tuner->demodulator->demux-> v4l2 decoder->render (DRM / ASoC) using upstream kernel drivers. Something I suspect not many SoC platforms can do. Peter.
On Tue, Apr 26, 2016 at 05:14:32PM +0100, Peter Griffin wrote: > On Tue, 26 Apr 2016, Mark Brown wrote: > > A lot of this is details of the system integration for this SoC, not > > actual errors. > This particular clock patch yes, but the other ASoC dt doc update is fixing > bindings which haven't progressed in lockstep with the driver code. Presumably > this happened during the review process when they were changed to being st, > prefixed in the driver, but the doc wasn't also updated. The bits where you are correcting the names of the properties are not details of the system integration and are therefore fine. The bits where you're documenting the particular clocking arrangements for the SoC you happen to be using less so. > > it's fairly routine > > to have to explain to people that just because some old driver did > > something that doesn't mean it's something we want in new drivers. > I'm sure it is. Although I fail to see why leaving the documentation with > mistakes in is helpful to anybody. Fixing actual mistakes is fine.
Hi Mark, On Tue, 26 Apr 2016, Mark Brown wrote: > On Tue, Apr 26, 2016 at 05:14:32PM +0100, Peter Griffin wrote: > > On Tue, 26 Apr 2016, Mark Brown wrote: > > > > A lot of this is details of the system integration for this SoC, not > > > actual errors. > > > This particular clock patch yes, but the other ASoC dt doc update is fixing > > bindings which haven't progressed in lockstep with the driver code. Presumably > > this happened during the review process when they were changed to being st, > > prefixed in the driver, but the doc wasn't also updated. > > The bits where you are correcting the names of the properties are not > details of the system integration and are therefore fine. The bits > where you're documenting the particular clocking arrangements for the > SoC you happen to be using less so. Ok sounds good. With that in mind I will drop this clocking patch in v4 and just leave the one which updates the ASoC bindings mismatch. > > > > it's fairly routine > > > to have to explain to people that just because some old driver did > > > something that doesn't mean it's something we want in new drivers. > > > I'm sure it is. Although I fail to see why leaving the documentation with > > mistakes in is helpful to anybody. > > Fixing actual mistakes is fine. Peter.
diff --git a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt index ef2e0c6..e6ef96a8 100644 --- a/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt +++ b/Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt @@ -62,6 +62,9 @@ Example: #sound-dai-cells = <0>; st,syscfg = <&syscfg_core>; clocks = <&clk_s_d0_flexgen CLK_PCM_2>; + assigned-clocks = <&clk_s_d0_flexgen CLK_PCM_2>; + assigned-clock-parents = <&clk_s_d0_quadfs 2>; + assigned-clock-rates = <50000000>; reg = <0x8D82000 0x158>; interrupts = <GIC_SPI 86 IRQ_TYPE_NONE>; dmas = <&fdma0 4 0 1>; @@ -78,6 +81,9 @@ Example: #sound-dai-cells = <0>; st,syscfg = <&syscfg_core>; clocks = <&clk_s_d0_flexgen CLK_SPDIFF>; + assigned-clocks = <&clk_s_d0_flexgen CLK_SPDIFF>; + assigned-clock-parents = <&clk_s_d0_quadfs 3>; + assigned-clock-rates = <50000000>; reg = <0x8D85000 0x158>; interrupts = <GIC_SPI 89 IRQ_TYPE_NONE>; dmas = <&fdma0 7 0 1>; @@ -133,6 +139,7 @@ Example of audio card declaration: /* DAC */ format = "i2s"; dai-tdm-slot-width = <32>; + mclk-fs = <256>; cpu { sound-dai = <&sti_uni_player2>; }; @@ -144,6 +151,7 @@ Example of audio card declaration: simple-audio-card,dai-link@1 { /* SPDIF */ format = "left_j"; + mclk-fs = <128>; cpu { sound-dai = <&sti_uni_player3>; };
Update the examples, as otherwise driver errors with an incorrect mclk ratio at runtime. Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Cc: arnaud.pouliquen@st.com --- Documentation/devicetree/bindings/sound/st,sti-asoc-card.txt | 8 ++++++++ 1 file changed, 8 insertions(+)