Re: [PATCH 10/19] dma: imx-sdma: Add multi fifo support

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Mar 18, 2022 at 01:42:51PM +0800, Shengjiu Wang wrote:
>    Hi
>    On Thu, Mar 17, 2022 at 4:28 PM Sascha Hauer <[1]s.hauer@pengutronix.de>
>    wrote:
> 
>      Signed-off-by: Sascha Hauer <[2]s.hauer@pengutronix.de>
>      ---
>       drivers/dma/imx-sdma.c                | 54 +++++++++++++++++++++++++++
>       include/linux/platform_data/dma-imx.h |  7 ++++
>       2 files changed, 61 insertions(+)
> 
>      diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
>      index 1038f6bc7f846..21e1cec2ffde9 100644
>      --- a/drivers/dma/imx-sdma.c
>      +++ b/drivers/dma/imx-sdma.c
>      @@ -14,6 +14,7 @@
>       #include <linux/iopoll.h>
>       #include <linux/module.h>
>       #include <linux/types.h>
>      +#include <linux/bitfield.h>
>       #include <linux/bitops.h>
>       #include <linux/mm.h>
>       #include <linux/interrupt.h>
>      @@ -73,6 +74,7 @@
>       #define SDMA_CHNENBL0_IMX35    0x200
>       #define SDMA_CHNENBL0_IMX31    0x080
>       #define SDMA_CHNPRI_0          0x100
>      +#define SDMA_DONE0_CONFIG      0x1000
> 
>       /*
>        * Buffer descriptor status values.
>      @@ -180,6 +182,12 @@
>                                       BIT(DMA_MEM_TO_DEV) | \
>                                       BIT(DMA_DEV_TO_DEV))
> 
>      +#define SDMA_WATERMARK_LEVEL_N_FIFOS   GENMASK(15, 12)
>      +#define SDMA_WATERMARK_LEVEL_SW_DONE   BIT(23)
>      +
>      +#define SDMA_DONE0_CONFIG_DONE_SEL     BIT(7)
>      +#define SDMA_DONE0_CONFIG_DONE_DIS     BIT(6)
>      +
>       /**
>        * struct sdma_script_start_addrs - SDMA script start pointers
>        *
>      @@ -441,6 +449,11 @@ struct sdma_channel {
>              struct work_struct              terminate_worker;
>              struct list_head                terminated;
>              bool                            is_ram_script;
>      +       unsigned int                    n_fifos;
>      +       unsigned int                    n_fifos_src;
>      +       unsigned int                    n_fifos_dst;
>      +       bool                            sw_done;
>      +       u32                             sw_done_sel;
> 
>    "sw_done_sel" is not used, and may not be needed.

Ok, will drop.

>    And can we just add 'struct sdma_peripheral_config *pconfig'
>    to replace each item here ('n_fifos_src', 'n_fifos_dst',
>    'sw_done')? 

I rather do not access the pointer to the peripheral_config outside of
sdma_config because I know nothing about the lifetime of that structure.

>    the pconfig can point to the struct in dma_slave_config.
>    And 'n_fifos' can be moved to locally function in 
>    sdma_set_watermarklevel_for_sais(), then use sdmac->direction
>    to select 'n_fifos_dst' or 'n_fifos_src'.

Ok.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |