[alsa-devel] r8a7790 only has audio when clock is forced on
Hi all,
We have a problem where audio on r8a7790 is not working due to it not sending interrupts.
We've verified that the pins and the clocks are working and correctly configured. The codec is starting and producing correct bit and word clock signals
We've made a hack to force /mstp10_clks /on that makes the audio work. Further investigation showed that it's only needed to force SSI(ALL) on, but, as far as we know, the manual doesn't say anything about it having to be kept on.
We've also debugged this hack and saw that the registers have correct values. These are some of the debugging messages we have when the audio fails:
Playing WAVE '../audio/r.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo cpg_mstp_clock_endisable: index 5, enable 1 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffe0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, enable 1 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, value written in group->smstpcr 00027fc0 aplay: pcm_writecpg_mstp_clock_endisable: index 15, enable 0 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 0002ffc0 :1710: write errcpg_mstp_clock_endisable: index 5, enable 0 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffe0 or: Input/output error
This shows that the SSI (ALL) clock in bit 5 being enabled before the SSI0 and that it is also disabled in the reverse sequence as expected.
Not sure what to do next or what the problem could be. Any new ideas are welcome,
Thanks!
On Tue, Jun 03, 2014 at 06:23:19PM +0100, Violeta Menendez Gonzalez wrote:
Hi all,
We have a problem where audio on r8a7790 is not working due to it not sending interrupts.
Adding Morimoto-san who does most of the work on those drivers upstream - it's always best to CC relevant maintainers/developers on kernel stuff as things only on the lists are easily missed. Not deleting context so he sees it.
We've verified that the pins and the clocks are working and correctly configured. The codec is starting and producing correct bit and word clock signals
We've made a hack to force /mstp10_clks /on that makes the audio work. Further investigation showed that it's only needed to force SSI(ALL) on, but, as far as we know, the manual doesn't say anything about it having to be kept on.
We've also debugged this hack and saw that the registers have correct values. These are some of the debugging messages we have when the audio fails:
Playing WAVE '../audio/r.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo cpg_mstp_clock_endisable: index 5, enable 1 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffe0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, enable 1 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, value written in group->smstpcr 00027fc0 aplay: pcm_writecpg_mstp_clock_endisable: index 15, enable 0 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 0002ffc0 :1710: write errcpg_mstp_clock_endisable: index 5, enable 0 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffe0 or: Input/output error
This shows that the SSI (ALL) clock in bit 5 being enabled before the SSI0 and that it is also disabled in the reverse sequence as expected.
Not sure what to do next or what the problem could be. Any new ideas are welcome,
Thanks!
-- Violeta Menéndez González http://www.codethink.co.uk/ Software Engineer Codethink - Providing Genius
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Violeta, Mark
Adding Morimoto-san who does most of the work on those drivers upstream
- it's always best to CC relevant maintainers/developers on kernel stuff
as things only on the lists are easily missed. Not deleting context so he sees it.
Thank you for adding me. I want to help you
We have a problem where audio on r8a7790 is not working due to it not sending interrupts.
I need to ask you something
- Are you using lager board ? - Which kernel are you using ? - Does your kernel booted from legacy code (= using board-lager.c) or from DT code (= using board-lager-referenct.c) ?
We've made a hack to force /mstp10_clks /on that makes the audio work. Further investigation showed that it's only needed to force SSI(ALL) on, but, as far as we know, the manual doesn't say anything about it having to be kept on.
YES. all SSI port needs SSI(ALL). (Actually, all SRC needs SRC(ALL) too if you use it)
Basically, R-Car sound is very complex IP. The interrupt/data register/DMA selection are depends on your "data path"
This driver is producing many steps to use it.
At 1st, you can use "PIO transfer". this is not good performance, but, very easy settings. you can check your basic setting by this. (power/codec/pin/clock...)
The data path is..
PIO [MEM] ---> [SSI]
Next step is "using DMA". but, R-Car sound has 2 DMAs (Audio DMAC/Audio DMAC peri peri) And, these DMAC usage is complex too. So, you can use basic DMA transfer as DMA 1st step. The data path is..
Audio DMAC [MEM] ---> [SSI]
Next step is more complex. use SRC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] --->[SSI]
Final step is using DVC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] ---> [DVC] --->[SSI]
Actually, lagacy boot is supporting above all path. But, DT boot is supporting 1st/2nd step only at this point (On rsnd driver side, DMAEngine DT is not yet supporing) I want to support 3rd/4th step on DT, but I'm busy now...
I pushed my local patches which are based on LTSI v3.4 + our original patches. These are tring to use sound as legacy code (= not DT) for lager/koelsh. Maybe not good fit too you, but can help ?
https://dl.dropboxusercontent.com/u/2774973/lager-koelsh-sound.tar.bz2
On 04/06/14 01:26, Kuninori Morimoto wrote:
Hi Violeta, Mark
Adding Morimoto-san who does most of the work on those drivers upstream
- it's always best to CC relevant maintainers/developers on kernel stuff
as things only on the lists are easily missed. Not deleting context so he sees it.
Thank you for adding me. I want to help you
We have a problem where audio on r8a7790 is not working due to it not sending interrupts.
I need to ask you something
- Are you using lager board ?
- Which kernel are you using ?
- Does your kernel booted from legacy code (= using board-lager.c) or from DT code (= using board-lager-referenct.c) ?
We've made a hack to force /mstp10_clks /on that makes the audio work. Further investigation showed that it's only needed to force SSI(ALL) on, but, as far as we know, the manual doesn't say anything about it having to be kept on.
YES. all SSI port needs SSI(ALL). (Actually, all SRC needs SRC(ALL) too if you use it)
The result of our investigation is as follows:
- The clock code correctly enables SSI(ALL) but the audio does not work - If we force SSI(ALL) to be on all the time, we have working audio
Basically, R-Car sound is very complex IP. The interrupt/data register/DMA selection are depends on your "data path"
This driver is producing many steps to use it.
At 1st, you can use "PIO transfer". this is not good performance, but, very easy settings. you can check your basic setting by this. (power/codec/pin/clock...)
The data path is..
PIO [MEM] ---> [SSI]Next step is "using DMA". but, R-Car sound has 2 DMAs (Audio DMAC/Audio DMAC peri peri) And, these DMAC usage is complex too. So, you can use basic DMA transfer as DMA 1st step. The data path is..
Audio DMAC [MEM] ---> [SSI]Next step is more complex. use SRC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] --->[SSI]Final step is using DVC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] ---> [DVC] --->[SSI]Actually, lagacy boot is supporting above all path. But, DT boot is supporting 1st/2nd step only at this point (On rsnd driver side, DMAEngine DT is not yet supporing) I want to support 3rd/4th step on DT, but I'm busy now...
I pushed my local patches which are based on LTSI v3.4 + our original patches. These are tring to use sound as legacy code (= not DT) for lager/koelsh. Maybe not good fit too you, but can help ?
https://dl.dropboxusercontent.com/u/2774973/lager-koelsh-sound.tar.bz2
To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/04/14 01:26, Kuninori Morimoto wrote:
Hi Violeta, Mark
Adding Morimoto-san who does most of the work on those drivers upstream
- it's always best to CC relevant maintainers/developers on kernel stuff
as things only on the lists are easily missed. Not deleting context so he sees it.
Thank you for adding me. I want to help you
Sorry I didn't Cc'ed you in the first place
We have a problem where audio on r8a7790 is not working due to it not sending interrupts.
I need to ask you something
- Are you using lager board ?
Yes
- Which kernel are you using ?
3.15-rc1
- Does your kernel booted from legacy code (= using board-lager.c) or from DT code (= using board-lager-referenct.c) ?
DT code
We've made a hack to force /mstp10_clks /on that makes the audio work. Further investigation showed that it's only needed to force SSI(ALL) on, but, as far as we know, the manual doesn't say anything about it having to be kept on.
YES. all SSI port needs SSI(ALL). (Actually, all SRC needs SRC(ALL) too if you use it)
Basically, R-Car sound is very complex IP. The interrupt/data register/DMA selection are depends on your "data path"
This driver is producing many steps to use it.
At 1st, you can use "PIO transfer". this is not good performance, but, very easy settings. you can check your basic setting by this. (power/codec/pin/clock...)
The data path is..
PIO [MEM] ---> [SSI]Next step is "using DMA". but, R-Car sound has 2 DMAs (Audio DMAC/Audio DMAC peri peri) And, these DMAC usage is complex too. So, you can use basic DMA transfer as DMA 1st step. The data path is..
Audio DMAC [MEM] ---> [SSI]Next step is more complex. use SRC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] --->[SSI]Final step is using DVC
Audio DMAC Audio DMAC peri peri [MEM] ---> [SRC] ---> [DVC] --->[SSI]Actually, lagacy boot is supporting above all path. But, DT boot is supporting 1st/2nd step only at this point (On rsnd driver side, DMAEngine DT is not yet supporing) I want to support 3rd/4th step on DT, but I'm busy now...
I pushed my local patches which are based on LTSI v3.4 + our original patches. These are tring to use sound as legacy code (= not DT) for lager/koelsh. Maybe not good fit too you, but can help ?
https://dl.dropboxusercontent.com/u/2774973/lager-koelsh-sound.tar.bz2
To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Violeta,
On Wed, Jun 4, 2014 at 11:17 AM, Violeta Menendez Gonzalez violeta.menendez@codethink.co.uk wrote:
- Which kernel are you using ?
3.15-rc1
- Does your kernel booted from legacy code (= using board-lager.c) or from DT code (= using board-lager-referenct.c) ?
DT code
Does applying commit 3c90c55dcde745bed81f6447f24ba96bda43d984 ("drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI") help?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Jun 04, 2014 at 11:33:14AM +0200, Geert Uytterhoeven wrote:
Hi Violeta,
On Wed, Jun 4, 2014 at 11:17 AM, Violeta Menendez Gonzalez violeta.menendez@codethink.co.uk wrote:
- Which kernel are you using ?
3.15-rc1
- Does your kernel booted from legacy code (= using board-lager.c) or from DT code (= using board-lager-referenct.c) ?
DT code
Does applying commit 3c90c55dcde745bed81f6447f24ba96bda43d984 ("drivers: sh: compile drivers/sh/pm_runtime.c if ARCH_SHMOBILE_MULTI") help?
We should have that in our tree (other things don't work). As a note, we did include a debug output showing the clock driver is writing the clock enables for these bits.
The system is also not showing any signs of external aborts, which is indicative of trying to access an IP block when it is not clocked. I will sit down and see if we can debug through this once we've had a look at DMA.
Hi Ben
# I misunderstood your point...
The result of our investigation is as follows:
- The clock code correctly enables SSI(ALL) but the audio does not work
- If we force SSI(ALL) to be on all the time, we have working audio
SSI(ALL) bit mean "this bit is always required for all SSI" If you want to use SSI0, then, you need to enable "SSI(ALL) + SSI0" bit
Hi!
On 06/05/14 07:05, Kuninori Morimoto wrote:
Hi Ben
# I misunderstood your point...
The result of our investigation is as follows:
- The clock code correctly enables SSI(ALL) but the audio does not work
- If we force SSI(ALL) to be on all the time, we have working audio
SSI(ALL) bit mean "this bit is always required for all SSI" If you want to use SSI0, then, you need to enable "SSI(ALL) + SSI0" bit
We have to force SSI(ALL) to stay always on on the DT with a special property, like this: always-on = <0x00000020>;
we don't need to force SSI0 on, although it is necessary for audio that this clock is on too.
The code for mstp10 correctly enables SSI0 *and* SSI(ALL). But it only works when SSI(ALL) is forced on the DT.
When we do on the DT: always-on = <0x00000020>; we have: cpg_mstp_clock_endisable: index 5, enable 0 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, enable 1 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, enable 1 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, enable 0 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, enable 0 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 and audio is working correctly.
If we do: always-on = <0x00000000>; (or not having the property at all) then we have: cpg_mstp_clock_endisable: index 5, enable 1 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffe0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, enable 1 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, enable 0 cpg_mstp_clock_endisable: index 15, value read from group->smstpcr 00027fc0 cpg_mstp_clock_endisable: index 15, value write in group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, enable 0 cpg_mstp_clock_endisable: index 5, value read from group->smstpcr 0002ffc0 cpg_mstp_clock_endisable: index 5, value write in group->smstpcr 0002ffe0 and we don't have audio at all.
Where index 5 is SSI(ALL) and index 15 is SSI0. We can observe that when we force SSI(ALL) on on the DT, then clock_disable is called for SSI(ALL) before it enables again. But when we don't force SSI(ALL) to be on, then it only calls clock_enable, but the register data is the same.
This looks like the correct behaviour, but in one case we have audio, and in the other we don't.
I hope this clarifies the problem a bit more.
Thanks
Hi Violeta
SSI(ALL) bit mean "this bit is always required for all SSI" If you want to use SSI0, then, you need to enable "SSI(ALL) + SSI0" bit
We have to force SSI(ALL) to stay always on on the DT with a special property, like this: always-on = <0x00000020>;
we don't need to force SSI0 on, although it is necessary for audio that this clock is on too.
The code for mstp10 correctly enables SSI0 *and* SSI(ALL). But it only works when SSI(ALL) is forced on the DT.
(snip)
Where index 5 is SSI(ALL) and index 15 is SSI0. We can observe that when we force SSI(ALL) on on the DT, then clock_disable is called for SSI(ALL) before it enables again. But when we don't force SSI(ALL) to be on, then it only calls clock_enable, but the register data is the same.
This looks like the correct behaviour, but in one case we have audio, and in the other we don't.
I hope this clarifies the problem a bit more.
I understand your point on DT with SSI(ALL). I didn't send R-Car sound DT support patch yet for some reasons. But, R-Car sound clock needs trick.
- SSI(ALL) is called via pm_runtime_xxx() from sound driver - SRC(ALL) is called as parent clock of SRCn clocks
This is implemented on legacy clock-r8a7790.c too. So, I think "always-on" is not needed for sound driver.
My local DTS file is below. (not enough maintenance though...)
It is very nice if you can wait for my DT patches, since R-Car sound driver is very complex / needs some trick etc...
mstp10_clks: mstp10_clks@e6150998 { compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks"; reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>; clocks = <&p_clk>, /* parent of SCU */ <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>; #clock-cells = <1>; clock-indices = < R8A7790_CLK_SCU R8A7790_CLK_SSI R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5 R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0 R8A7790_CLK_DVC1 R8A7790_CLK_DVC0 R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5 R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0 >; clock-output-names = "scu", "ssi", "ssi9", "ssi8", "ssi7", "ssi6", "ssi5", "ssi4", "ssi3", "ssi2", "ssi1", "ssi0", "dvc1", "dvc0", "src9", "src8", "src7", "src6", "src5", "src4", "src3", "src2", "src1", "src0"; };
...
rcar_sound: rcar_sound@0xec500000 { #sound-dai-cells = <1>; compatible = "renesas,rcar_sound-r8a7790", "renesas,rcar_sound-gen2", "renesas,rcar_sound"; interrupt-parent = <&gic>; reg = <0 0xec500000 0 0x1000>, /* SCU */ <0 0xec5a0000 0 0x100>, /* ADG */ <0 0xec540000 0 0x1000>, /* SSIU */ <0 0xec541000 0 0x1280>; /* SSI */ clocks = <&mstp10_clks R8A7790_CLK_SSI>, <&mstp10_clks R8A7790_CLK_SSI9>, <&mstp10_clks R8A7790_CLK_SSI8>, <&mstp10_clks R8A7790_CLK_SSI7>, <&mstp10_clks R8A7790_CLK_SSI6>, <&mstp10_clks R8A7790_CLK_SSI5>, <&mstp10_clks R8A7790_CLK_SSI4>, <&mstp10_clks R8A7790_CLK_SSI3>, <&mstp10_clks R8A7790_CLK_SSI2>, <&mstp10_clks R8A7790_CLK_SSI1>, <&mstp10_clks R8A7790_CLK_SSI0>, <&mstp10_clks R8A7790_CLK_SRC9>, <&mstp10_clks R8A7790_CLK_SRC8>, <&mstp10_clks R8A7790_CLK_SRC7>, <&mstp10_clks R8A7790_CLK_SRC6>, <&mstp10_clks R8A7790_CLK_SRC5>, <&mstp10_clks R8A7790_CLK_SRC4>, <&mstp10_clks R8A7790_CLK_SRC3>, <&mstp10_clks R8A7790_CLK_SRC2>, <&mstp10_clks R8A7790_CLK_SRC1>, <&mstp10_clks R8A7790_CLK_SRC0>, <&audio_clk_a>, <&audio_clk_b>, <&audio_clk_c>, <&m2_clk>; clock-names = "ssi", "ssi.9", "ssi.8", "ssi.7", "ssi.6", "ssi.5", "ssi.4", "ssi.3", "ssi.2", "ssi.1", "ssi.0", "src.9", "src.8", "src.7", "src.6", "src.5", "src.4", "src.3", "src.2", "src.1", "src.0", "clk_a", "clk_b", "clk_c", "clk_i"; ...
On 05/06/14 10:35, Kuninori Morimoto wrote:
Hi Violeta
SSI(ALL) bit mean "this bit is always required for all SSI" If you want to use SSI0, then, you need to enable "SSI(ALL) + SSI0" bit
We have to force SSI(ALL) to stay always on on the DT with a special property, like this: always-on = <0x00000020>;
we don't need to force SSI0 on, although it is necessary for audio that this clock is on too.
The code for mstp10 correctly enables SSI0 *and* SSI(ALL). But it only works when SSI(ALL) is forced on the DT.
(snip)
Where index 5 is SSI(ALL) and index 15 is SSI0. We can observe that when we force SSI(ALL) on on the DT, then clock_disable is called for SSI(ALL) before it enables again. But when we don't force SSI(ALL) to be on, then it only calls clock_enable, but the register data is the same.
This looks like the correct behaviour, but in one case we have audio, and in the other we don't.
I hope this clarifies the problem a bit more.
I understand your point on DT with SSI(ALL). I didn't send R-Car sound DT support patch yet for some reasons. But, R-Car sound clock needs trick.
- SSI(ALL) is called via pm_runtime_xxx() from sound driver
- SRC(ALL) is called as parent clock of SRCn clocks
See, this is why I /hate/ the implicit clock management. This is another case where we're getting bitten by it.
This is implemented on legacy clock-r8a7790.c too. So, I think "always-on" is not needed for sound driver.
My local DTS file is below. (not enough maintenance though...)
It is very nice if you can wait for my DT patches, since R-Car sound driver is very complex / needs some trick etc...
mstp10_clks: mstp10_clks@e6150998 { compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks"; reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>; clocks = <&p_clk>, /* parent of SCU */ <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,
we have these as <&mstp10_clks R8A7790_CLK_SSI>
<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>; #clock-cells = <1>; clock-indices = < R8A7790_CLK_SCU R8A7790_CLK_SSI R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5 R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0 R8A7790_CLK_DVC1 R8A7790_CLK_DVC0 R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5 R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0 >; clock-output-names = "scu", "ssi", "ssi9", "ssi8", "ssi7", "ssi6", "ssi5", "ssi4", "ssi3", "ssi2", "ssi1", "ssi0", "dvc1", "dvc0", "src9", "src8", "src7", "src6", "src5", "src4", "src3", "src2", "src1", "src0"; };...
rcar_sound: rcar_sound@0xec500000 { #sound-dai-cells = <1>; compatible = "renesas,rcar_sound-r8a7790", "renesas,rcar_sound-gen2", "renesas,rcar_sound"; interrupt-parent = <&gic>; reg = <0 0xec500000 0 0x1000>, /* SCU */ <0 0xec5a0000 0 0x100>, /* ADG */ <0 0xec540000 0 0x1000>, /* SSIU */ <0 0xec541000 0 0x1280>; /* SSI */ clocks = <&mstp10_clks R8A7790_CLK_SSI>,
I don't think we have this one here.
<&mstp10_clks R8A7790_CLK_SSI9>, <&mstp10_clks R8A7790_CLK_SSI8>, <&mstp10_clks R8A7790_CLK_SSI7>, <&mstp10_clks R8A7790_CLK_SSI6>, <&mstp10_clks R8A7790_CLK_SSI5>, <&mstp10_clks R8A7790_CLK_SSI4>, <&mstp10_clks R8A7790_CLK_SSI3>, <&mstp10_clks R8A7790_CLK_SSI2>, <&mstp10_clks R8A7790_CLK_SSI1>, <&mstp10_clks R8A7790_CLK_SSI0>, <&mstp10_clks R8A7790_CLK_SRC9>, <&mstp10_clks R8A7790_CLK_SRC8>, <&mstp10_clks R8A7790_CLK_SRC7>, <&mstp10_clks R8A7790_CLK_SRC6>, <&mstp10_clks R8A7790_CLK_SRC5>, <&mstp10_clks R8A7790_CLK_SRC4>, <&mstp10_clks R8A7790_CLK_SRC3>, <&mstp10_clks R8A7790_CLK_SRC2>, <&mstp10_clks R8A7790_CLK_SRC1>, <&mstp10_clks R8A7790_CLK_SRC0>, <&audio_clk_a>, <&audio_clk_b>, <&audio_clk_c>, <&m2_clk>; clock-names = "ssi", "ssi.9", "ssi.8", "ssi.7", "ssi.6", "ssi.5", "ssi.4", "ssi.3", "ssi.2", "ssi.1", "ssi.0", "src.9", "src.8", "src.7", "src.6", "src.5", "src.4", "src.3", "src.2", "src.1", "src.0", "clk_a", "clk_b", "clk_c", "clk_i"; ...
On 06/05/14 10:52, Ben Dooks wrote:
On 05/06/14 10:35, Kuninori Morimoto wrote:
Hi Violeta
SSI(ALL) bit mean "this bit is always required for all SSI" If you want to use SSI0, then, you need to enable "SSI(ALL) + SSI0" bit
We have to force SSI(ALL) to stay always on on the DT with a special property, like this: always-on = <0x00000020>;
we don't need to force SSI0 on, although it is necessary for audio that this clock is on too.
The code for mstp10 correctly enables SSI0 *and* SSI(ALL). But it only works when SSI(ALL) is forced on the DT.
(snip)
Where index 5 is SSI(ALL) and index 15 is SSI0. We can observe that when we force SSI(ALL) on on the DT, then clock_disable is called for SSI(ALL) before it enables again. But when we don't force SSI(ALL) to be on, then it only calls clock_enable, but the register data is the same.
This looks like the correct behaviour, but in one case we have audio, and in the other we don't.
I hope this clarifies the problem a bit more.
I understand your point on DT with SSI(ALL). I didn't send R-Car sound DT support patch yet for some reasons. But, R-Car sound clock needs trick.
- SSI(ALL) is called via pm_runtime_xxx() from sound driver
- SRC(ALL) is called as parent clock of SRCn clocks
See, this is why I /hate/ the implicit clock management. This is another case where we're getting bitten by it.
This is implemented on legacy clock-r8a7790.c too. So, I think "always-on" is not needed for sound driver.
My local DTS file is below. (not enough maintenance though...)
It is very nice if you can wait for my DT patches, since R-Car sound driver is very complex / needs some trick etc...
mstp10_clks: mstp10_clks@e6150998 { compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks"; reg = <0 0xe6150998 0 4>, <0 0xe61509a8 0 4>; clocks = <&p_clk>, /* parent of SCU */ <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>, <&p_clk>,we have these as <&mstp10_clks R8A7790_CLK_SSI>
<&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>, <&mstp10_clks R8A7790_CLK_SCU>; #clock-cells = <1>; clock-indices = < R8A7790_CLK_SCU R8A7790_CLK_SSI R8A7790_CLK_SSI9 R8A7790_CLK_SSI8 R8A7790_CLK_SSI7 R8A7790_CLK_SSI6 R8A7790_CLK_SSI5 R8A7790_CLK_SSI4 R8A7790_CLK_SSI3 R8A7790_CLK_SSI2 R8A7790_CLK_SSI1 R8A7790_CLK_SSI0 R8A7790_CLK_DVC1 R8A7790_CLK_DVC0 R8A7790_CLK_SRC9 R8A7790_CLK_SRC8 R8A7790_CLK_SRC7 R8A7790_CLK_SRC6 R8A7790_CLK_SRC5 R8A7790_CLK_SRC4 R8A7790_CLK_SRC3 R8A7790_CLK_SRC2 R8A7790_CLK_SRC1 R8A7790_CLK_SRC0 >; clock-output-names = "scu", "ssi", "ssi9", "ssi8", "ssi7", "ssi6", "ssi5", "ssi4", "ssi3", "ssi2", "ssi1", "ssi0", "dvc1", "dvc0", "src9", "src8", "src7", "src6", "src5", "src4", "src3", "src2", "src1", "src0"; };...
rcar_sound: rcar_sound@0xec500000 { #sound-dai-cells = <1>; compatible = "renesas,rcar_sound-r8a7790", "renesas,rcar_sound-gen2", "renesas,rcar_sound"; interrupt-parent = <&gic>; reg = <0 0xec500000 0 0x1000>, /* SCU */ <0 0xec5a0000 0 0x100>, /* ADG */ <0 0xec540000 0 0x1000>, /* SSIU */ <0 0xec541000 0 0x1280>; /* SSI */ clocks = <&mstp10_clks R8A7790_CLK_SSI>,
I don't think we have this one here.
Indeed. Adding it fixed our issue.
Thanks all.
<&mstp10_clks R8A7790_CLK_SSI9>, <&mstp10_clks R8A7790_CLK_SSI8>, <&mstp10_clks R8A7790_CLK_SSI7>, <&mstp10_clks R8A7790_CLK_SSI6>, <&mstp10_clks R8A7790_CLK_SSI5>, <&mstp10_clks R8A7790_CLK_SSI4>, <&mstp10_clks R8A7790_CLK_SSI3>, <&mstp10_clks R8A7790_CLK_SSI2>, <&mstp10_clks R8A7790_CLK_SSI1>, <&mstp10_clks R8A7790_CLK_SSI0>, <&mstp10_clks R8A7790_CLK_SRC9>, <&mstp10_clks R8A7790_CLK_SRC8>, <&mstp10_clks R8A7790_CLK_SRC7>, <&mstp10_clks R8A7790_CLK_SRC6>, <&mstp10_clks R8A7790_CLK_SRC5>, <&mstp10_clks R8A7790_CLK_SRC4>, <&mstp10_clks R8A7790_CLK_SRC3>, <&mstp10_clks R8A7790_CLK_SRC2>, <&mstp10_clks R8A7790_CLK_SRC1>, <&mstp10_clks R8A7790_CLK_SRC0>, <&audio_clk_a>, <&audio_clk_b>, <&audio_clk_c>, <&m2_clk>; clock-names = "ssi", "ssi.9", "ssi.8", "ssi.7", "ssi.6", "ssi.5", "ssi.4", "ssi.3", "ssi.2", "ssi.1", "ssi.0", "src.9", "src.8", "src.7", "src.6", "src.5", "src.4", "src.3", "src.2", "src.1", "src.0", "clk_a", "clk_b", "clk_c", "clk_i"; ...
Hi Ben, Violeta
reg = <0 0xec500000 0 0x1000>, /* SCU */ <0 0xec5a0000 0 0x100>, /* ADG */ <0 0xec540000 0 0x1000>, /* SSIU */ <0 0xec541000 0 0x1280>; /* SSI */ clocks = <&mstp10_clks R8A7790_CLK_SSI>,I don't think we have this one here.
Indeed. Adding it fixed our issue.
Phew, nice ! Sorry for my poor English which causes delay solution.
- SSI(ALL) is called via pm_runtime_xxx() from sound driver
- SRC(ALL) is called as parent clock of SRCn clocks
See, this is why I /hate/ the implicit clock management. This is another case where we're getting bitten by it.
I troubled about this ALL bit too, then, it become current style.
One reason was that this driver is supporting Gen1 sound too. But, this SRC(ALL) didn't exist on Gen1..
Hi Morimoto-san,
On Fri, Jun 6, 2014 at 7:54 AM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
- SSI(ALL) is called via pm_runtime_xxx() from sound driver
- SRC(ALL) is called as parent clock of SRCn clocks
See, this is why I /hate/ the implicit clock management. This is another case where we're getting bitten by it.
I troubled about this ALL bit too, then, it become current style.
One reason was that this driver is supporting Gen1 sound too. But, this SRC(ALL) didn't exist on Gen1..
This is not the only module that has a clock in R-Car Gen2, but not in Gen1. The same is true for GPIO. There may be others...
When adding a device node to the DTS, always make sure to double check if it has a clock, and add it.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
participants (6)
-
Ben Dooks -
Ben Dooks -
Geert Uytterhoeven -
Kuninori Morimoto -
Mark Brown -
Violeta Menendez Gonzalez