Re: [alsa-devel] r8a7790 only has audio when clock is forced on

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

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
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Kuninori Morimoto]

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

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Ben Dooks]

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
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Kuninori Morimoto]

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

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Violeta Menendez Gonzalez]

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

-- 
Violeta Menéndez González	http://www.codethink.co.uk/
Software Engineer		Codethink - Providing Genius

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Kuninori Morimoto]

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";
       ...

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Ben Dooks]

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";
>        ...
> 


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Violeta Menendez Gonzalez]

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";
>>         ...
>>
>
>

-- 
Violeta Menéndez González	http://www.codethink.co.uk/
Software Engineer		Codethink - Providing Genius

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Kuninori Morimoto]

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..

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Geert Uytterhoeven]

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

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Violeta Menendez Gonzalez]

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
>

-- 
Violeta Menéndez González	http://www.codethink.co.uk/
Software Engineer		Codethink - Providing Genius

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Geert Uytterhoeven]

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

Re: [alsa-devel] r8a7790 only has audio when clock is forced on

[Ben Dooks]

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.

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.