Re: [PATCH v5 00/17] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Tue, Jan 16, 2018 at 10:51 PM, Nicolin Chen  wrote:
> [ Maciej, could you please send your Tested-by/Reviewed-by for AC97
>   once you confirm this series?
>
>   And Caleb, this version does not need a test for non-AC97 cases.
>
>   Thanks both! ]
>
> ==Change log==
> v5
>  * Reworked the series by taking suggestions from Maciej for AC97
>   + Fixed SSI lockup issue by changing cleanup sequence in PATCH-13
>   + Moved fsl_ssi_hw_clean() after unregistering the CODEC device
> in PATCH-13
>   + Set NULL as the parent of CODEC platform device to fix a NULL
> pointer dereference bug in PATCH-16
>  * Updated comments of three variables/pointers in struct fsl_ssi
>to describe them more accurately in PATCH-16
> v4
>  * Reworked the series by taking suggestions from Maciej
>   + Added TXBIT0 bit back to play safe in PATCH-14
>   + Made bool synchronous exclusive with AC97 mode in PATCH-16
> v3
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to make RX and TX more clearly defined
>   + Replaced "bool dir" with "int dir" in PATCH-04
>   + Replaced "!dir" with "int adir" in PATCH-05
>   + Put CBM_CFS behind the baudclk check to keep the same
> program flow in PATCH-14
>   + Removed all cpu_dai_drv changes in PATCH-15
> v2
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to keep all ssi->i2s_net updated
>   + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
>   + Moved all initial register configurations from dai probe() to
> platform probe() so as to let AC97 CODEC successfully probe.
>  * Added Tested-by from Caleb for TDM test cases.
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
>  - Playback via I2S Master and Slave mode
>  - Record via I2S Master and Slave mode
>  - Simultaneous playback and record via I2S Master and Slave mode
>  - Background playback with foreground record (starting at different
>time) via I2S Master and Slave mode
>  - Background record with foreground playback (starting at different
>time) via I2S Master and Slave mode
>  * All tests above by hacking offline_config to true in imx51.
>
> Caleb has tested v1-v4 with TDM lookback tests on i.MX6.
>
> Example of uncovered tests: AC97, PowerPC and FIQ.
>
> Nicolin Chen (17):
>   ASoC: fsl_ssi: Redefine RX and TX macros
>   ASoC: fsl_ssi: Keep ssi->i2s_net updated
>   ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
>   ASoC: fsl_ssi: Maintain a mask of active streams
>   ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
>   ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
>   ASoC: fsl_ssi: Clean up helper functions of trigger()
>   ASoC: fsl_ssi: Add DAIFMT define for AC97
>   ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
>   ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
>   ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
>   ASoC: fsl_ssi: Move one-time configurations to probe()
>   ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
>   ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
>   ASoC: fsl_ssi: Add bool synchronous to mark synchronous mode
>   ASoC: fsl_ssi: Move DT related code to a separate probe()
>   ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
>  sound/soc/fsl/fsl_ssi.c | 756 
> +---
>  sound/soc/fsl/fsl_ssi.h |   3 -
>  2 files changed, 395 insertions(+), 364 deletions(-)
>
> --
> 2.7.4
>
tested v5.

No problems,
 -Caleb

Re: [PATCH v4 00/17] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Mon, Jan 15, 2018 at 3:16 PM, Nicolin Chen <nicoleots...@gmail.com> wrote:
> ==Change log==
> v4
>  * Reworked the series by taking suggestions from Maciej
>   + Added TXBIT0 bit back to play safe in PATCH-14
>   + Made bool synchronous exclusive with AC97 mode in PATCH-16
> v3
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to make RX and TX more clearly defined
>   + Replaced "bool dir" with "int dir" in PATCH-04
>   + Replaced "!dir" with "int adir" in PATCH-05
>   + Put CBM_CFS behind the baudclk check to keep the same
> program flow in PATCH-14
>   + Removed all cpu_dai_drv changes in PATCH-15
> v2
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to keep all ssi->i2s_net updated
>   + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
>   + Moved all initial register configurations from dai probe() to
> platform probe() so as to let AC97 CODEC successfully probe.
>  * Added Tested-by from Caleb for TDM test cases.
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
>  - Playback via I2S Master and Slave mode
>  - Record via I2S Master and Slave mode
>  - Simultaneous playback and record via I2S Master and Slave mode
>  - Background playback with foreground record (starting at different
>time) via I2S Master and Slave mode
>  - Background record with foreground playback (starting at different
>time) via I2S Master and Slave mode
>  * All tests above by hacking offline_config to true in imx51.
>
> Caleb has tested v1 with TDM lookback tests on i.MX6.
>
> Example of uncovered tests: AC97, PowerPC and FIQ.
>
> Nicolin Chen (17):
>   ASoC: fsl_ssi: Redefine RX and TX macros
>   ASoC: fsl_ssi: Keep ssi->i2s_net updated
>   ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
>   ASoC: fsl_ssi: Maintain a mask of active streams
>   ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
>   ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
>   ASoC: fsl_ssi: Clean up helper functions of trigger()
>   ASoC: fsl_ssi: Add DAIFMT define for AC97
>   ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
>   ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
>   ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
>   ASoC: fsl_ssi: Move one-time configurations to probe()
>   ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
>   ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
>   ASoC: fsl_ssi: Add bool synchronous to mark synchronous mode
>   ASoC: fsl_ssi: Move DT related code to a separate probe()
>   ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
>  sound/soc/fsl/fsl_ssi.c | 741 
> +---
>  sound/soc/fsl/fsl_ssi.h |   3 -
>  2 files changed, 379 insertions(+), 365 deletions(-)
>
> --
> 2.7.4
>

Tested v4 with 8 channel TDM at 48kHz. No problems found.

Tested-by: Caleb Crome <ca...@crome.org>

Re: [PATCH v3 00/17] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Sun, Jan 14, 2018 at 8:21 PM, Nicolin Chen <nicoleots...@gmail.com> wrote:
> ==Change log==
> v3
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to make RX and TX more clearly defined
>   + Replaced "bool dir" with "int dir" in PATCH-04
>   + Replaced "!dir" with "int adir" in PATCH-05
>   + Put CBM_CFS behind the baudclk check to keep the same
> program flow in PATCH-14
>   + Removed all cpu_dai_drv changes in PATCH-15
> v2
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to keep all ssi->i2s_net updated
>   + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
>   + Moved all initial register configurations from dai probe() to
> platform probe() so as to let AC97 CODEC successfully probe.
>  * Added Tested-by from Caleb for TDM test cases.
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
>  - Playback via I2S Master and Slave mode
>  - Record via I2S Master and Slave mode
>  - Simultaneous playback and record via I2S Master and Slave mode
>  - Background playback with foreground record (starting at different
>time) via I2S Master and Slave mode
>  - Background record with foreground playback (starting at different
>time) via I2S Master and Slave mode
>  * All tests above by hacking offline_config to true in imx51.
>
> Caleb has tested v1 with TDM lookback tests on i.MX6.
>
> Example of uncovered tests: AC97, PowerPC and FIQ.
>
> Nicolin Chen (17):
>   ASoC: fsl_ssi: Redefine RX and TX macros
>   ASoC: fsl_ssi: Keep ssi->i2s_net updated
>   ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
>   ASoC: fsl_ssi: Maintain a mask of active streams
>   ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
>   ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
>   ASoC: fsl_ssi: Clean up helper functions of trigger()
>   ASoC: fsl_ssi: Add DAIFMT define for AC97
>   ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
>   ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
>   ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
>   ASoC: fsl_ssi: Move one-time configurations to probe()
>   ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
>   ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
>   ASoC: fsl_ssi: Add bool synchronous to mark synchronous mode
>   ASoC: fsl_ssi: Move DT related code to a separate probe()
>   ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
>  sound/soc/fsl/fsl_ssi.c | 733 
> 
>  sound/soc/fsl/fsl_ssi.h |   3 -
>  2 files changed, 370 insertions(+), 366 deletions(-)
>
> --
> 2.7.4
>
tested v3...

Tested-by: Caleb Crome <ca...@crome.org>

Re: [PATCH v2 00/16] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Wed, Jan 10, 2018 at 10:42 PM, Nicolin Chen <nicoleots...@gmail.com> wrote:
>
> ==Change log==
> v2
>  * Reworked the series by taking suggestions from Maciej
>   + Added PATCH-01 to keep all ssi->i2s_net updated
>   + Replaced bool tx with bool dir in PATCH-03 and PATCH-06
>   + Moved all initial register configurations from dai probe() to
> platform probe() so as to let AC97 CODEC successfully probe.
>  * Added Tested-by from Caleb for TDM test cases.
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
>  - Playback via I2S Master and Slave mode
>  - Record via I2S Master and Slave mode
>  - Simultaneous playback and record via I2S Master and Slave mode
>  - Background playback with foreground record (starting at different
>time) via I2S Master and Slave mode
>  - Background record with foreground playback (starting at different
>time) via I2S Master and Slave mode
>  * All tests above by hacking offline_config to true in imx51.
>
> Caleb has tested v1 with TDM lookback tests on i.MX6.
>
> Example of uncovered tests: AC97, PowerPC and FIQ.
>
> Nicolin Chen (16):
>   ASoC: fsl_ssi: Keep ssi->i2s_net updated
>   ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
>   ASoC: fsl_ssi: Maintain a mask of active streams
>   ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
>   ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
>   ASoC: fsl_ssi: Clean up helper functions of trigger()
>   ASoC: fsl_ssi: Add DAIFMT define for AC97
>   ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
>   ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
>   ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
>   ASoC: fsl_ssi: Move one-time configurations to probe()
>   ASoC: fsl_ssi: Setup AC97 in fsl_ssi_hw_init()
>   ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
>   ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
>   ASoC: fsl_ssi: Move DT related code to a separate probe()
>   ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
>  sound/soc/fsl/fsl_ssi.c | 740 
> 
>  1 file changed, 369 insertions(+), 371 deletions(-)
>
> --
> 2.7.4
>
Tested again, just to be sure...  All looks good.


Tested-by: Caleb Crome <ca...@crome.org>

Re: [PATCH v1 00/15] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Thu, Jan 4, 2018 at 11:48 AM, Nicolin Chen <nicoleots...@gmail.com> wrote:
> On Tue, Jan 02, 2018 at 03:28:11PM -0800, Caleb Crome wrote:
>
>> tested this patch set on MX6 SSI against broonie for-next (4.15-rc5),
>> no problems.
>> Do I send a separate Tested-by for each patch, or just the 00/15 one?
>
>> Tested-by: Caleb Crome <ca...@crome.org>
>
> I will include your Tested-by to each patch in v2 unless there is
> going to be a critical change suggested by the future reviews so
> that we need another round of full testing. Thank you

Great.

-Caleb

Re: [PATCH v1 00/15] ASoC: fsl_ssi: Clean up - program flow level

[Caleb Crome]

On Tue, Dec 19, 2017 at 9:00 AM, Nicolin Chen <nicoleots...@gmail.com> wrote:
>
> ==Background==
> The fsl_ssi driver was designed for PPC originally and then it has
> been updated to support different modes for i.MX Series, including
> SDMA, I2S Master mode, AC97 and older i.MXs with FIQ, by different
> contributors for different use cases in different coding styles.
>
> Additionally, in order to fix/work-around hardware bugs and design
> flaws, the driver made a lot of compromise so now its program flow
> looks very complicated and it's getting hard to maintain or update.
>
> So I am going to clean up the driver on both coding style level and
> program flow level.
>
> ==Introduction==
> This series of patches is the second set to clean up fsl_ssi driver
> in the program flow level. Any patch here may impact a fundamental
> test case like playback or record.
>
> ==Verification==
> This series of patches require fully tested. I have done such tests
> on i.MX6SoloX with WM8962 using imx_v6_v7_defconfig as:
>  - Playback via I2S Master and Slave mode
>  - Record via I2S Master and Slave mode
>  - Simultaneous playback and record via I2S Master and Slave mode
>  - Background playback with foreground record (starting at different
>time) via I2S Master and Slave mode
>  - Background record with foreground playback (starting at different
>time) via I2S Master and Slave mode
>  * All tests above by hacking offline_config to true in imx51.
>
> Example of uncovered tests: TDM, AC97, PowerPC and FIQ.
>
> Nicolin Chen (15):
>   ASoC: fsl_ssi: Clean up set_dai_tdm_slot()
>   ASoC: fsl_ssi: Maintain a mask of active streams
>   ASoC: fsl_ssi: Rename fsl_ssi_disable_val macro
>   ASoC: fsl_ssi: Clear FIFO directly in fsl_ssi_config()
>   ASoC: fsl_ssi: Clean up helper functions of trigger()
>   ASoC: fsl_ssi: Add DAIFMT define for AC97
>   ASoC: fsl_ssi: Clean up fsl_ssi_setup_regvals()
>   ASoC: fsl_ssi: Set xFEN0 and xFEN1 together
>   ASoC: fsl_ssi: Use snd_soc_init_dma_data instead
>   ASoC: fsl_ssi: Move one-time configurations to dai_probe()
>   ASoC: fsl_ssi: Setup AC97 in dai_probe()
>   ASoC: fsl_ssi: Clean up _fsl_ssi_set_dai_fmt()
>   ASoC: fsl_ssi: Remove cpu_dai_drv from fsl_ssi structure
>   ASoC: fsl_ssi: Move DT related code to a separate probe()
>   ASoC: fsl_ssi: Use ssi->streams instead of reading register
>
>  sound/soc/fsl/fsl_ssi.c | 710 
> 
>  1 file changed, 348 insertions(+), 362 deletions(-)
>
> --
> 2.7.4
>

tested this patch set on MX6 SSI against broonie for-next (4.15-rc5),
no problems.
Do I send a separate Tested-by for each patch, or just the 00/15 one?



Tested-by: Caleb Crome <ca...@crome.org>

ASoC: fsl_ssi: Bringing up the SSI port in multi-channel TDM mode

[Caleb Crome]

Hi,
   I just posted a little write up for helping people get started with
the Freescale SSI port in TDM mode.

https://medium.com/@caleb_22836/how-to-get-the-mx6-ssi-port-up-and-running-in-tdm-mode-dbce02a15e81

I'm just posting here in case anybody is searching the google for this
information and finds this post.

(Also, it assumes that v4.15 will be released by the time anybody
actually uses it...)

-Caleb

Re: [PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level

[Caleb Crome]

On Wed, Dec 20, 2017 at 3:40 AM, Arnaud Mouiche <arnaud.moui...@invoxia.com>
wrote:

>
>
> On 19/12/2017 01:25, Caleb Crome wrote:
>
>> On Mon, Dec 18, 2017 at 3:02 PM, Nicolin Chen <nicoleots...@gmail.com>
>> wrote:
>>
>>> On Mon, Dec 18, 2017 at 02:19:08PM -0800, Caleb Crome wrote:
>>>
>>> Acked-by: Timur Tabi <ti...@tabi.org>
>>>>>
>>>> --- To Mark ---
>>>
>>> Mark, can you still take these changes first? Since this failed
>>> test that Caleb reported here is already existing on the top of
>>> the mainline tree, I would like to treat this mail as a separate
>>> bug report and fix it with a separate patch.
>>>
>>> Besides, this series of changes don't change any function flow.
>>>
>>> Thank you
>>>
>>> Sorry!  I should have created a separate thread for this subject.  My
>> comments have *nothing* to do with this patch set, except they are
>> about the same source files.
>>
>> --- To Caleb ---
>>>
>>> I'm re-setting up my loopback test to try to verify these most recent
>>>> changes.
>>>>
>>> I really appreciate your verification and help.
>>>
>> Of course!  I have this wandboard permanently set up for this
>> verification test, so that I can easily repeat whenever I touch our
>> kernel.
>>
>> It's a dead-simple hardware mod just to connect TX to RX.
>>
>> warn:   11a0 11a1 1160 11a3 11a4 11a5 11a6 11a7
>>>> warn: Valid frame after 1 invalid frames
>>>> warn:   11c0 11c1 11c2 11c3 11c4 11c5 11c6 11c7
>>>> warn: first invalid frame while expecting frame 0x00a0
>>>> warn:   13e7 1400 1401 1402 1403 1404 1405 1404
>>>> warn:   1407 1420 1421 1422 1423 1424 1425 1426
>>>> warn:   1427 1440 1441 1442 1443 1444 1445 1484
>>>> warn:   1447 1460 1461 1462 1463 1464 1465 1466
>>>>
>>>> Those last 4 lines are the channel slips -- the least significant
>>>> nibble should be the channel number:  i.e. should go 0, 1, 2, 3, 4, 5,
>>>> 6, 7.
>>>>
>>>> Ugh, so it's basically quite broken again -- before these patches.
>>>>
>>> I remember Arnaud reviewed one of my changes back to September.
>>> So I suppose the test should be fine at that time -- so a change
>>> being merged recently might have impacted the test result.
>>>
>>
>> It's certainly possible that I'm doing something wrong again -- it
>> wouldn't be the first time :-)
>>
>
> Hi All,
>
> Sorry but I will be busy until mid January, I could help testing and
> fixing broken multi channel after.
> Anyway, I don't see specific issues with Nicolin patches.
> We can take time to fix what was broken before this patch set... after.
>
> Arnaud
>
>
>
>> I guess I need to go backwards in time and see what rev re-broke it.
>>>> I don't really have time to dig too deep on this again.
>>>>
>>>> I'd be happy to provide the hardware to anybody that can diagnose and
>>>> debug this more quickly than I can.  I'm very inefficient at kernel
>>>> drivers I think.   My day job is acoustical and electrical
>>>> engineering.
>>>>
>>>> Here's what the hardware looks like for anybody that's interested.
>>>> Just a single wire loopback on the wandboard header.
>>>>
>>> I would definitely like to take the hardware to debug it as long
>>> as you are willing to provide me. Can you send me a private mail
>>> to discuss about it?
>>>
>> Absolutely.
>> -Caleb
>>
>>
>> Thanks
>>> Nicolin
>>>
>>
>
Okay, operator error on my part.  There was an old clock setting in my ssi3
dtsi file that (falsely) modified the ssi baud clock frequency.  Nicolin's
patch

ASoC: fsl_ssi: Caculate bit clock rate using slot number and width

now properly computes the master clock, and the old dtsi settings that were
necessary to fake things into the right speed are now obsolete.

So... basically, everything is back to working properly.  it wasn't broken
at all -- just my oversight on a ssi clock setting in the dtb.

-Caleb

Re: [PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level

[Caleb Crome]

On Thu, Dec 21, 2017 at 8:08 AM, Caleb Crome <ca...@crome.org> wrote:


On Wed, Dec 20, 2017 at 3:40 AM, Arnaud Mouiche
<arnaud.moui...@invoxia.com> wrote:
>
>
>
> On 19/12/2017 01:25, Caleb Crome wrote:
>
>> On Mon, Dec 18, 2017 at 3:02 PM, Nicolin Chen <nicoleots...@gmail.com> wrote:
>>>
>>> On Mon, Dec 18, 2017 at 02:19:08PM -0800, Caleb Crome wrote:
>>>
>>>
>>>>> Acked-by: Timur Tabi <ti...@tabi.org>
>>>
>>> --- To Mark ---
>>>
>>>
>>> Mark, can you still take these changes first? Since this failed
>>>
>>> test that Caleb reported here is already existing on the top of
>>>
>>> the mainline tree, I would like to treat this mail as a separate
>>>
>>> bug report and fix it with a separate patch.
>>>
>>>
>>> Besides, this series of changes don't change any function flow.
>>>
>>>
>>> Thank you
>>>
>> Sorry!  I should have created a separate thread for this subject.  My
>>
>> comments have *nothing* to do with this patch set, except they are
>>
>> about the same source files.
>>
>>
>>> --- To Caleb ---
>>>
>>>
>>>> I'm re-setting up my loopback test to try to verify these most recent 
>>>> changes.
>>>
>>> I really appreciate your verification and help.
>>
>> Of course!  I have this wandboard permanently set up for this
>>
>> verification test, so that I can easily repeat whenever I touch our
>>
>> kernel.
>>
>>
>> It's a dead-simple hardware mod just to connect TX to RX.
>>
>>
>>>> warn:   11a0 11a1 1160 11a3 11a4 11a5 11a6 11a7
>>>>
>>>> warn: Valid frame after 1 invalid frames
>>>>
>>>> warn:   11c0 11c1 11c2 11c3 11c4 11c5 11c6 11c7
>>>>
>>>> warn: first invalid frame while expecting frame 0x00a0
>>>>
>>>> warn:   13e7 1400 1401 1402 1403 1404 1405 1404
>>>>
>>>> warn:   1407 1420 1421 1422 1423 1424 1425 1426
>>>>
>>>> warn:   1427 1440 1441 1442 1443 1444 1445 1484
>>>>
>>>> warn:   1447 1460 1461 1462 1463 1464 1465 1466
>>>>
>>>>
>>>> Those last 4 lines are the channel slips -- the least significant
>>>>
>>>> nibble should be the channel number:  i.e. should go 0, 1, 2, 3, 4, 5,
>>>>
>>>> 6, 7.
>>>>
>>>>
>>>> Ugh, so it's basically quite broken again -- before these patches.
>>>
>>> I remember Arnaud reviewed one of my changes back to September.
>>>
>>> So I suppose the test should be fine at that time -- so a change
>>> being merged recently might have impacted the test result.
>>
>>
>> It's certainly possible that I'm doing something wrong again -- it
>> wouldn't be the first time :-)
>
[resend -- previous wasn't in plain text mode]

Okay, operator error on my part.  There was an old clock setting in my
ssi3 dtsi file that (falsely) modified the ssi baud clock frequency.
Nicolin's patch

ASoC: fsl_ssi: Caculate bit clock rate using slot number and width

now properly computes the master clock, and the old dtsi settings that
were necessary to fake things into the right speed are now obsolete.

So... basically, everything is back to working properly.  it wasn't
broken at all -- just my oversight on a ssi clock setting in the dtb.

-Caleb

Re: [PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level

[Caleb Crome]

On Mon, Dec 18, 2017 at 3:02 PM, Nicolin Chen <nicoleots...@gmail.com> wrote:
> On Mon, Dec 18, 2017 at 02:19:08PM -0800, Caleb Crome wrote:
>
>> > Acked-by: Timur Tabi <ti...@tabi.org>
>
> --- To Mark ---
>
> Mark, can you still take these changes first? Since this failed
> test that Caleb reported here is already existing on the top of
> the mainline tree, I would like to treat this mail as a separate
> bug report and fix it with a separate patch.
>
> Besides, this series of changes don't change any function flow.
>
> Thank you
>

Sorry!  I should have created a separate thread for this subject.  My
comments have *nothing* to do with this patch set, except they are
about the same source files.

>
> --- To Caleb ---
>
>> I'm re-setting up my loopback test to try to verify these most recent 
>> changes.
>
> I really appreciate your verification and help.

Of course!  I have this wandboard permanently set up for this
verification test, so that I can easily repeat whenever I touch our
kernel.

It's a dead-simple hardware mod just to connect TX to RX.

>
>> warn:   11a0 11a1 1160 11a3 11a4 11a5 11a6 11a7
>> warn: Valid frame after 1 invalid frames
>> warn:   11c0 11c1 11c2 11c3 11c4 11c5 11c6 11c7
>> warn: first invalid frame while expecting frame 0x00a0
>> warn:   13e7 1400 1401 1402 1403 1404 1405 1404
>> warn:   1407 1420 1421 1422 1423 1424 1425 1426
>> warn:   1427 1440 1441 1442 1443 1444 1445 1484
>> warn:   1447 1460 1461 1462 1463 1464 1465 1466
>>
>> Those last 4 lines are the channel slips -- the least significant
>> nibble should be the channel number:  i.e. should go 0, 1, 2, 3, 4, 5,
>> 6, 7.
>>
>> Ugh, so it's basically quite broken again -- before these patches.
>
> I remember Arnaud reviewed one of my changes back to September.
> So I suppose the test should be fine at that time -- so a change
> being merged recently might have impacted the test result.


It's certainly possible that I'm doing something wrong again -- it
wouldn't be the first time :-)

>
>> I guess I need to go backwards in time and see what rev re-broke it.
>> I don't really have time to dig too deep on this again.
>>
>> I'd be happy to provide the hardware to anybody that can diagnose and
>> debug this more quickly than I can.  I'm very inefficient at kernel
>> drivers I think.   My day job is acoustical and electrical
>> engineering.
>>
>> Here's what the hardware looks like for anybody that's interested.
>> Just a single wire loopback on the wandboard header.
>
> I would definitely like to take the hardware to debug it as long
> as you are willing to provide me. Can you send me a private mail
> to discuss about it?

Absolutely.
-Caleb


>
> Thanks
> Nicolin

Re: [PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level

[Caleb Crome]

On Sun, Dec 17, 2017 at 7:13 PM, Timur Tabi  wrote:
>
> On 12/17/17 8:51 PM, Nicolin Chen wrote:
>>
>> Nicolin Chen (11):
>>ASoC: fsl_ssi: Rename fsl_ssi_private to fsl_ssi
>>ASoC: fsl_ssi: Cache pdev->dev pointer
>>ASoC: fsl_ssi: Refine all comments
>>ASoC: fsl_ssi: Rename registers and fields macros
>>ASoC: fsl_ssi: Refine indentations and wrappings
>>ASoC: fsl_ssi: Refine printk outputs
>>ASoC: fsl_ssi: Rename cpu_dai parameter to dai
>>ASoC: fsl_ssi: Rename scr_val to scr
>>ASoC: fsl_ssi: Replace fsl_ssi_rxtx_reg_val with fsl_ssi_regvals
>>ASoC: fsl_ssi: Rename i2smode to i2s_net
>>ASoC: fsl_ssi: Define ternary macros to simplify code
>
>
> Acked-by: Timur Tabi 


I'm re-setting up my loopback test to try to verify these most recent changes.

I'm starting with the for-next branch at
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git (which
is a 4.15 numbered kernel).

The basic setup is using a wandboard with a TX pin tied directly to an
RX pin do test if what is sent is what is received, using Arnaud's
tester here: https://github.com/amouiche/atest

Unfortunately, it doesn't pass the loopback test, even before these
patches.  I'm not actively developing on this platform at the moment,
and my brain has been out of kernel development for quite a while.

I'm getting channel slips again.

output from atest:

warn:   11a0 11a1 1160 11a3 11a4 11a5 11a6 11a7
warn: Valid frame after 1 invalid frames
warn:   11c0 11c1 11c2 11c3 11c4 11c5 11c6 11c7
warn: first invalid frame while expecting frame 0x00a0
warn:   13e7 1400 1401 1402 1403 1404 1405 1404
warn:   1407 1420 1421 1422 1423 1424 1425 1426
warn:   1427 1440 1441 1442 1443 1444 1445 1484
warn:   1447 1460 1461 1462 1463 1464 1465 1466

Those last 4 lines are the channel slips -- the least significant
nibble should be the channel number:  i.e. should go 0, 1, 2, 3, 4, 5,
6, 7.

Ugh, so it's basically quite broken again -- before these patches.
I guess I need to go backwards in time and see what rev re-broke it.
I don't really have time to dig too deep on this again.

I'd be happy to provide the hardware to anybody that can diagnose and
debug this more quickly than I can.  I'm very inefficient at kernel
drivers I think.   My day job is acoustical and electrical
engineering.

Here's what the hardware looks like for anybody that's interested.
Just a single wire loopback on the wandboard header.

https://ibb.co/d2rfzm

-Caleb

Re: [PATCH v3 03/11] ASoC: fsl_ssi: Refine all comments

[Caleb Crome]

On Sat, Dec 16, 2017 at 9:15 AM, Timur Tabi  wrote:
>
> On 12/13/17 5:18 PM, Nicolin Chen wrote:
>
>> -   /* Used when using fsl-ssi as sound-card. This is only used by ppc 
>> and
>> -* should be replaced with simple-sound-card. */
>> struct platform_device *pdev;
>
>
> Is this comment no longer true?
>
>> + * 1) SSI in earlier SoCS has crtical bits in control registers that
>
>
> critical
>
>> -/**
>> - * fsl_ssi_isr: SSI interrupt handler
>> - *
>> - * Although it's possible to use the interrupt handler to send and receive
>> - * data to/from the SSI, we use the DMA instead.  Programming is more
>> - * complicated, but the performance is much better.
>> - *
>> - * This interrupt handler is used only to gather statistics.
>> - *
>> - * @irq: IRQ of the SSI device
>> - * @dev_id: pointer to the fsl_ssi structure for this SSI device
>> - */
>
>
> What's wrong with this comment?
>
>> -/*
>> - * Clear RX or TX FIFO to remove samples from the previous
>> - * stream session which may be still present in the FIFO and
>> - * may introduce bad samples and/or channel slipping.
>> - *
>> - * Note: The SOR is not documented in recent IMX datasheet, but
>> - * is described in IMX51 reference manual at section 56.3.3.15.
>> +/**
>> + * Clear remaining data in the FIFO to avoid dirty data or channel slipping
>
>
> I think the original is better, unless there's something untrue about it.
>
>> -* We are running on a SoC which does not support online SSI
>> -* reconfiguration, so we have to enable all necessary flags at once
>> -* even if we do not use them later (capture and playback 
>> configuration)
>> +* Online configuration is not supported
>> +* Enable or Disable all necessary bits at once
>
>
> Ditto
>
>> -   /*
>> -* Configure single direction units while the SSI unit is running
>> -* (online configuration)
>> -*/
>> +   /* Online configure single direction while SSI is running */
>
>
> Ditto
>
>> -   /*
>> -* Disabling the necessary flags for one of rx/tx while the
>> -* other stream is active is a little bit more difficult. We
>> -* have to disable only those flags that differ between both
>> -* streams (rx XOR tx) and that are set in the stream that is
>> -* disabled now. Otherwise we could alter flags of the other
>> -* stream
>> -*/
>> -
>> -   /* These assignments are simply vals without bits set in 
>> avals*/
>> +   /* Exclude necessary bits for the opposite stream */
>
>
> Ditto
>
>> -   /*
>> -* Be sure the Tx FIFO is filled when TE is set.
>> -* Otherwise, there are some chances to start the
>> -* playback with some void samples inserted first,
>> -* generating a channel slip.
>> -*
>> -* First, SSIEN must be set, to let the FIFO be 
>> filled.
>> -*
>> -* Notes:
>> -* - Limit this fix to the DMA case until FIQ cases 
>> can
>> -*   be tested.
>> -* - Limit the length of the busy loop to not lock 
>> the
>> -*   system too long, even if 1-2 loops are 
>> sufficient
>> -*   in general.
>> -*/
>
>
> What's wrong with this comment?
>
>> -   /*
>> -* Note that these below aren't just normal registers.
>> -* They are a way to disable or enable bits in SACCST
>> -* register:
>> -* - writing a '1' bit at some position in SACCEN sets the
>> -* relevant bit in SACCST,
>> -* - writing a '1' bit at some position in SACCDIS unsets
>> -* the relevant bit in SACCST register.
>> -*
>> -* The two writes below first disable all channels slots,
>> -* then enable just slots 3 & 4 ("PCM Playback Left Channel"
>> -* and "PCM Playback Right Channel").
>> -*/
>> +   /* Disable all channel slots */
>
>
> Ditto.
>
>
>> -* Why are we setting up SACCST everytime we are starting a
>> -* playback?
>> -* Some CODECs (like VT1613 CODEC on UDOO board) like to
>> -* (sometimes) set extra bits in their SLOTREQ requests.
>> -* When a bit is set in a SLOTREQ request then SSI sets the
>> -* relevant bit in SACCST automatically (it is enough if a bit was
>> -* set in a SLOTREQ just once, bits in SACCST are 'sticky').
>> -* If an extra slot gets enabled that's a disaster for playback
>> -* because some of normal left or right channel samples are
>> -

[PATCH] fsl_ssi: set fifo watermark to more reliable value

[Caleb Crome]

From: Caleb Crome <ca...@crome.org>

The fsl_ssi fifo watermark is by default set to 2 free spaces (i.e.
activate DMA on FIFO when only 2 spaces are left.)  This means the
DMA must service the fifo within 2 audio samples, which is just not
enough time  for many use cases with high data rate.  In many
configurations the audio channel slips (causing l/r swap in stereo
configurations, or channel slipping in multi-channel configurations).

This patch gives more breathing room and allows the SSI to operate
reliably by changing the fifio refill watermark to 8.

There is no change in behavior for older chips (with an 8-deep fifo).
Only the newer chips with a 15-deep fifo get the new behavior. I
suspect a new fifo depth setting could be optimized on the older
chips too, but I have not tested.

Signed-off-by: Caleb Crome <ca...@crome.org>
---
 sound/soc/fsl/fsl_ssi.c | 74 +++--
 1 file changed, 53 insertions(+), 21 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index bedec4a..56245e6 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -224,6 +224,12 @@ struct fsl_ssi_soc_data {
  * @dbg_stats: Debugging statistics
  *
  * @soc: SoC specific data
+ *
+ * @fifo_watermark: the FIFO watermark setting.  Notifies DMA when
+ * there are @fifo_watermark or fewer words in TX fifo or
+ * @fifo_watermark or more empty words in RX fifo.
+ * @dma_maxburst: max number of words to transfer in one go.  So far,
+ * this is always the same as fifo_watermark.
  */
 struct fsl_ssi_private {
struct regmap *regs;
@@ -263,6 +269,9 @@ struct fsl_ssi_private {
 
const struct fsl_ssi_soc_data *soc;
struct device *dev;
+
+   u32 fifo_watermark;
+   u32 dma_maxburst;
 };
 
 /*
@@ -1051,21 +1060,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
regmap_write(regs, CCSR_SSI_SRCR, srcr);
regmap_write(regs, CCSR_SSI_SCR, scr);
 
-   /*
-* Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't
-* use FIFO 1. We program the transmit water to signal a DMA transfer
-* if there are only two (or fewer) elements left in the FIFO. Two
-* elements equals one frame (left channel, right channel). This value,
-* however, depends on the depth of the transmit buffer.
-*
-* We set the watermark on the same level as the DMA burstsize.  For
-* fiq it is probably better to use the biggest possible watermark
-* size.
-*/
-   if (ssi_private->use_dma)
-   wm = ssi_private->fifo_depth - 2;
-   else
-   wm = ssi_private->fifo_depth;
+   wm = ssi_private->fifo_watermark;
 
regmap_write(regs, CCSR_SSI_SFCSR,
CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) |
@@ -1373,12 +1368,8 @@ static int fsl_ssi_imx_probe(struct platform_device 
*pdev,
dev_dbg(>dev, "could not get baud clock: %ld\n",
 PTR_ERR(ssi_private->baudclk));
 
-   /*
-* We have burstsize be "fifo_depth - 2" to match the SSI
-* watermark setting in fsl_ssi_startup().
-*/
-   ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2;
-   ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2;
+   ssi_private->dma_params_tx.maxburst = ssi_private->dma_maxburst;
+   ssi_private->dma_params_rx.maxburst = ssi_private->dma_maxburst;
ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0;
ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0;
 
@@ -1543,6 +1534,47 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 /* Older 8610 DTs didn't have the fifo-depth property */
ssi_private->fifo_depth = 8;
 
+   /*
+* Set the watermark for transmit FIFO 0 and receive FIFO 0. We don't
+* use FIFO 1 but set the watermark appropriately nontheless.
+* We program the transmit water to signal a DMA transfer
+* if there are N elements left in the FIFO. For chips with 15-deep
+* FIFOs, set watermark to 8.  This allows the SSI to operate at a
+* high data rate without channel slipping. Behavior is unchanged
+* for the older chips with a fifo depth of only 8.  A value of 4
+* might be appropriate for the older chips, but is left at
+* fifo_depth-2 until sombody has a chance to test.
+*
+* We set the watermark on the same level as the DMA burstsize.  For
+* fiq it is probably better to use the biggest possible watermark
+* size.
+*/
+   switch (ssi_private->fifo_depth) {
+   case 15:
+   /*
+* 2 samples is not enough when running at high data
+* 

[PATCH v2 1/1] ASoC: fsl_ssi: add CCSR_SSI_SOR to volatile register list

[Caleb Crome]

The CCSR_SSI_SOR is a register that clears the TX and/or the RX fifo
on the i.MX SSI port.  The fsl_ssi_trigger writes this register in
order to clear the fifo at trigger time.

However, since the CCSR_SSI_SOR register is not in the volatile list,
the caching mechanism prevented the register write in the trigger
function.  This caused the fifo to not be cleared (because the value
was unchanged from the last time the register was written), and thus
causes the channels in both TDM or simple I2S mode to slip and be in
the wrong time slots on SSI restart.

This has gone unnoticed for so long because with simple stereo mode,
the consequence is that left and right are swapped, which isn't that
noticeable.  However, it's catestrophic in some systems that
require the channels to be in the right slots.

Signed-off-by: Caleb Crome <ca...@crome.org>
Suggested-by: Arnaud Mouiche <arnaud.moui...@invoxia.com>

---
 sound/soc/fsl/fsl_ssi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 216e3cb..2f3bf9c 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -151,6 +151,7 @@ static bool fsl_ssi_volatile_reg(struct device *dev, 
unsigned int reg)
case CCSR_SSI_SACDAT:
case CCSR_SSI_SATAG:
case CCSR_SSI_SACCST:
+   case CCSR_SSI_SOR:
return true;
default:
return false;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1] ASoC: fsl_ssi: add CCSR_SSI_SOR to volatile register list

[Caleb Crome]

On Mon, Apr 25, 2016 at 11:06 AM, Mark Brown <broo...@kernel.org> wrote:
> On Mon, Apr 25, 2016 at 10:50:24AM -0700, Caleb Crome wrote:
>
>> Due to caching, SOR wasn't written when it should have been.  This
>> patch simply adds SOR to the volatile list.
>
> Could you expand on when it wasn't written and why it needed to be
> please?

Yes, sorry.

The CCSR_SSI_SOR is a register that clears the TX and/or the RX fifo
on the i.MX6 SSI port.  The fsl_ssi_trigger writes this register in
order to clear the fifo at trigger time.

However, since the CCSR_SSI_SOR register is not in the volatile list,
the caching mechanism prevented the register write in the trigger
function.  This caused the fifo to not be cleared (because the value
was unchanged from the last time the register was written), and thus
causes the channels in both TDM or simple I2S mode to slip and be in
the wrong time slots on SSI restart.

By adding CCSR_SSI_SOR to the volatile list, along with arnaud's
patches that I just tested (and sent tested-by slugs), fix most of the
problems  with the SSI port drivers for multi-channel operation (there
is one more to come that I think really fixes the last bit).

Most people never noticed the problem because with simple stereo mode,
the consequence is that left and right are swapped, which isn't that
noticeable.

I can re-submit the patch if you like with this more descriptive comment.

Thanks,
 -Caleb
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/1] ASoC: fsl_ssi: add CCSR_SSI_SOR to volatile register list

[Caleb Crome]

Due to caching, SOR wasn't written when it should have been.  This
patch simply adds SOR to the volatile list.

Signed-off-by: Caleb Crome <ca...@crome.org>
---
 sound/soc/fsl/fsl_ssi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 216e3cb..2f3bf9c 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -151,6 +151,7 @@ static bool fsl_ssi_volatile_reg(struct device *dev, 
unsigned int reg)
case CCSR_SSI_SACDAT:
case CCSR_SSI_SATAG:
case CCSR_SSI_SACCST:
+   case CCSR_SSI_SOR:
return true;
default:
return false;
-- 
1.9.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] ASoC: fsl_ssi: Set watermark and maxburst settings to eliminate DMA xruns on imx processors

[Caleb Crome]

On Mon, Jan 18, 2016 at 10:59 AM, kbuild test robot <l...@intel.com> wrote:
> Hi Caleb,
>
> [auto build test ERROR on asoc/for-next]
> [also build test ERROR on v4.4 next-20160118]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Caleb-Crome/ASoC-fsl_ssi-Set-watermark-and-maxburst-settings-to-eliminate-DMA-xruns-on-imx-processors/20160119-024143
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 
> for-next
> config: x86_64-randconfig-x019-01180513 (attached as .config)
> reproduce:


Doh, sorry sent the wrong patch file.  Will re-submit shortly.

-Caleb
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] ASoC: fsl_ssi: Set watermark and maxburst settings to eliminate DMA xruns on imx processors

[Caleb Crome]

Change watermark settings for imx processors.  The previous setting
was fifo_depth-2, which causes silent but deadly xruns when using more
than 2 channels.  This patch sets the watermark & maxburst
settings to

DMA, imx processors: 4
DMA, mpc8610   : fifo_depth - 2
FIQ, all   : fifo_depth

This leaves mpc8610 with identically the same settings as before this
patch, and changes the IMX processors to have a watermark setting of
4, meaning when there are 4 or more empty spaces in the TX FIFO, to
start a DMA burst. And similarly, when there are 4 or more
filled spaces in the RX fifo, to start a DMA burst.

The tradeoff with this patch is that on IMX processors, the DMA will
be more reliable (not have DMA slips), epseically when running more
than 2 channels, at the expense of more DMA transactions.  Interrupt
frequency is unaffected.

Signed-off-by: Caleb Crome <ca...@crome.org>
---
 sound/soc/fsl/fsl_ssi.c | 54 ++---
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 95d2392..5080c29 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -220,6 +220,10 @@ struct fsl_ssi_soc_data {
  * @dbg_stats: Debugging statistics
  *
  * @soc: SoC specific data
+ *
+ * @fifo_watermark:  the fifo level to signal the DMA for more words
+ * @dma_maxburst:the maximum number of words to send to the fifo
+ *   in a DMA burst
  */
 struct fsl_ssi_private {
struct regmap *regs;
@@ -257,6 +261,9 @@ struct fsl_ssi_private {
struct fsl_ssi_dbg dbg_stats;
 
const struct fsl_ssi_soc_data *soc;
+
+   u32 fifo_watermark;
+   u32 dma_maxburst;
 };
 
 /*
@@ -985,21 +992,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
regmap_write(regs, CCSR_SSI_SRCR, srcr);
regmap_write(regs, CCSR_SSI_SCR, scr);
 
-   /*
-* Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't
-* use FIFO 1. We program the transmit water to signal a DMA transfer
-* if there are only two (or fewer) elements left in the FIFO. Two
-* elements equals one frame (left channel, right channel). This value,
-* however, depends on the depth of the transmit buffer.
-*
-* We set the watermark on the same level as the DMA burstsize.  For
-* fiq it is probably better to use the biggest possible watermark
-* size.
-*/
-   if (ssi_private->use_dma)
-   wm = ssi_private->fifo_depth - 2;
-   else
-   wm = ssi_private->fifo_depth;
+   wm = ssi_private->soc.fifo_watermark;
 
regmap_write(regs, CCSR_SSI_SFCSR,
CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) |
@@ -1307,12 +1300,8 @@ static int fsl_ssi_imx_probe(struct platform_device 
*pdev,
dev_dbg(>dev, "could not get baud clock: %ld\n",
 PTR_ERR(ssi_private->baudclk));
 
-   /*
-* We have burstsize be "fifo_depth - 2" to match the SSI
-* watermark setting in fsl_ssi_startup().
-*/
-   ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2;
-   ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2;
+   ssi_private->dma_params_tx.maxburst = ssi_private->dma_maxburst;
+   ssi_private->dma_params_rx.maxburst = ssi_private->dma_maxburst;
ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0;
ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0;
 
@@ -1465,6 +1454,29 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 /* Older 8610 DTs didn't have the fifo-depth property */
ssi_private->fifo_depth = 8;
 
+   if (ssi_private->use_dma) {
+   /*
+* using DMA, set both watermark & maxburst
+* imx parts need a value of 4 to keep up with the
+* fastest data rates.
+* older non-imx parts keep the old values of
+* fifo_depth-2.
+* maxburst must be <= fifo_watermark;
+* and must be even if dual fifo is used.
+*/
+   if (ssi_private->soc->imx) {
+   ssi_private->fifo_watermark = 4;
+   ssi_private->dma_maxburst = 4;
+   } else {
+   ssi_private->fifo_watermark =
+   ssi_private->fifo_depth - 2;
+   ssi_private->dma_maxburst =
+   ssi_private->fifo_depth - 2;
+   }
+   } else
+   /* using FIQ.  Keep settings what they were originally */
+   ssi_private->fifo_watermark = ssi_private->fifo_depth;
+
dev_set_drvdata