Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

2024-05-07 Thread Frank Li
On Tue, May 07, 2024 at 01:33:31PM +0200, Amelie Delaunay wrote:
> Hi Vinod,
> 
> Thanks for the review.
> 
> On 5/4/24 14:40, Vinod Koul wrote:
> > On 23-04-24, 14:32, Amelie Delaunay wrote:
> > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> > > controller:
> > > - LPDMA (Low Power): 4 channels, no FIFO
> > > - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> > > - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> > > Hardware configuration of the channels is retrieved from the hardware
> > > configuration registers.
> > > The client can specify its channel requirements through device tree.
> > > STM32 DMA3 channels can be individually reserved either because they are
> > > secure, or dedicated to another CPU.
> > > Indeed, channels availability depends on Resource Isolation Framework
> > > (RIF) configuration. RIF grants access to buses with Compartiment ID
> > 
> > Compartiment? typo...?
> > 
> 
> Sorry, indeed, Compartment instead.
> 
> > > (CIF) filtering, secure and privilege level. It also assigns DMA channels
> > > to one or several processors.
> > > DMA channels used by Linux should be CID-filtered and statically assigned
> > > to CID1 or shared with other CPUs but using semaphore. In case CID
> > > filtering is not configured, dma-channel-mask property can be used to
> > > specify available DMA channels to the kernel, otherwise such channels
> > > will be marked as reserved and can't be used by Linux.
> > > 
> > > Signed-off-by: Amelie Delaunay 
> > > ---
> > >   drivers/dma/stm32/Kconfig  |   10 +
> > >   drivers/dma/stm32/Makefile |1 +
> > >   drivers/dma/stm32/stm32-dma3.c | 1431 
> > >   3 files changed, 1442 insertions(+)
> > >   create mode 100644 drivers/dma/stm32/stm32-dma3.c
> > > 
> > > diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> > > index b72ae1a4502f..4d8d8063133b 100644
> > > --- a/drivers/dma/stm32/Kconfig
> > > +++ b/drivers/dma/stm32/Kconfig
> > > @@ -34,4 +34,14 @@ config STM32_MDMA
> > > If you have a board based on STM32 SoC with such DMA 
> > > controller
> > > and want to use MDMA say Y here.
> > > +config STM32_DMA3
> > > + tristate "STMicroelectronics STM32 DMA3 support"
> > > + select DMA_ENGINE
> > > + select DMA_VIRTUAL_CHANNELS
> > > + help
> > > +   Enable support for the on-chip DMA3 controller on STMicroelectronics
> > > +   STM32 platforms.
> > > +   If you have a board based on STM32 SoC with such DMA3 controller
> > > +   and want to use DMA3, say Y here.
> > > +
> > >   endif
> > > diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> > > index 663a3896a881..5082db4b4c1c 100644
> > > --- a/drivers/dma/stm32/Makefile
> > > +++ b/drivers/dma/stm32/Makefile
> > > @@ -2,3 +2,4 @@
> > >   obj-$(CONFIG_STM32_DMA) += stm32-dma.o
> > >   obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
> > >   obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> > > +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> > 
> > are there any similarities in mdma/dma and dma3..?
> > can anything be reused...?
> > 
> 
> DMA/MDMA were originally intended for STM32 MCUs and have been used in
> STM32MP1 MPUs.
> New MPUs (STM32MP2, ...) and STM32 MCUs (STM32H5, STM32N6, ...) use DMA3.
> Unlike DMA/MDMA, DMA3 can be declined in multiple configurations, LPDMA,
> GPDMA, HPDMA, and among these global configurations, there are possible
> sub-configurations (e.g. channel fifo size). stm32-dma3 uses the hardware
> configuration registers to discover the controller/channels capabilities.
> Reuse stm32-dma or stm32-mdma would lead to complicating the driver and
> making future stm32-dma3 evolutions for next STM32 MPUs intricate and very
> difficult.

I think your reason still not enough to create new driver instead try to
reuse old one.

Does register layout or dma descriptor is totally difference? 

If dma descriptor format is the same, at least you can reuse prepare DMA
descriptor part. 

Choose channel is independt part of DMA channel. You can create sperate
one for difference DMA engine.

Frank

> 
> > > diff --git a/drivers/dma/stm32/stm32-dma3.c 
> > > b/drivers/dma/stm32/stm32-dma3.c
> > > new file mode 100644
> > > index ..b5493f497d06
> > > --- /dev/null
> > > +++ b/drivers/dma/stm32/stm32-dma3.c
> > > @@ -0,0 +1,1431 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * STM32 DMA3 controller driver
> > > + *
> > > + * Copyright (C) STMicroelectronics 2024
> > > + * Author(s): Amelie Delaunay 
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include "../virt-dma.h"
> > > +
> > > +#define STM32_DMA3_SECCFGR   0x00
> > > +#define STM32_DMA3_PRIVCFGR  0x04
> > > +#define STM32_DMA

Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

2024-05-15 Thread Frank Li
On Mon, May 13, 2024 at 11:21:18AM +0200, Amelie Delaunay wrote:
> Hi Frank,
> 
> On 5/7/24 22:26, Frank Li wrote:
> > On Tue, May 07, 2024 at 01:33:31PM +0200, Amelie Delaunay wrote:
> > > Hi Vinod,
> > > 
> > > Thanks for the review.
> > > 
> > > On 5/4/24 14:40, Vinod Koul wrote:
> > > > On 23-04-24, 14:32, Amelie Delaunay wrote:
> > > > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 
> > > > > DMA3
> > > > > controller:
> > > > > - LPDMA (Low Power): 4 channels, no FIFO
> > > > > - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> > > > > - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> > > > > Hardware configuration of the channels is retrieved from the hardware
> > > > > configuration registers.
> > > > > The client can specify its channel requirements through device tree.
> > > > > STM32 DMA3 channels can be individually reserved either because they 
> > > > > are
> > > > > secure, or dedicated to another CPU.
> > > > > Indeed, channels availability depends on Resource Isolation Framework
> > > > > (RIF) configuration. RIF grants access to buses with Compartiment ID
> > > > 
> > > > Compartiment? typo...?
> > > > 
> > > 
> > > Sorry, indeed, Compartment instead.
> > > 
> > > > > (CIF) filtering, secure and privilege level. It also assigns DMA 
> > > > > channels
> > > > > to one or several processors.
> > > > > DMA channels used by Linux should be CID-filtered and statically 
> > > > > assigned
> > > > > to CID1 or shared with other CPUs but using semaphore. In case CID
> > > > > filtering is not configured, dma-channel-mask property can be used to
> > > > > specify available DMA channels to the kernel, otherwise such channels
> > > > > will be marked as reserved and can't be used by Linux.
> > > > > 
> > > > > Signed-off-by: Amelie Delaunay 
> > > > > ---
> > > > >drivers/dma/stm32/Kconfig  |   10 +
> > > > >drivers/dma/stm32/Makefile |1 +
> > > > >drivers/dma/stm32/stm32-dma3.c | 1431 
> > > > > 
> > > > >3 files changed, 1442 insertions(+)
> > > > >create mode 100644 drivers/dma/stm32/stm32-dma3.c
> > > > > 
> > > > > diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> > > > > index b72ae1a4502f..4d8d8063133b 100644
> > > > > --- a/drivers/dma/stm32/Kconfig
> > > > > +++ b/drivers/dma/stm32/Kconfig
> > > > > @@ -34,4 +34,14 @@ config STM32_MDMA
> > > > > If you have a board based on STM32 SoC with such DMA 
> > > > > controller
> > > > > and want to use MDMA say Y here.
> > > > > +config STM32_DMA3
> > > > > + tristate "STMicroelectronics STM32 DMA3 support"
> > > > > + select DMA_ENGINE
> > > > > + select DMA_VIRTUAL_CHANNELS
> > > > > + help
> > > > > +   Enable support for the on-chip DMA3 controller on 
> > > > > STMicroelectronics
> > > > > +   STM32 platforms.
> > > > > +   If you have a board based on STM32 SoC with such DMA3 
> > > > > controller
> > > > > +   and want to use DMA3, say Y here.
> > > > > +
> > > > >endif
> > > > > diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> > > > > index 663a3896a881..5082db4b4c1c 100644
> > > > > --- a/drivers/dma/stm32/Makefile
> > > > > +++ b/drivers/dma/stm32/Makefile
> > > > > @@ -2,3 +2,4 @@
> > > > >obj-$(CONFIG_STM32_DMA) += stm32-dma.o
> > > > >obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
> > > > >obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> > > > > +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> > > > 
> > > > are there any similarities in mdma/dma and dma3..?
> > > > can anything be reused...?
> > > > 
> > > 
> > > DMA/MDMA were originally intended for STM32 MCUs and have been used in
> > > STM32MP1 MPUs.
> > > New MPUs (STM32MP2, ...) and STM32 MCUs (STM32H5, STM

Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

2024-05-15 Thread Frank Li
On Tue, Apr 23, 2024 at 02:32:55PM +0200, Amelie Delaunay wrote:
> STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> controller:
> - LPDMA (Low Power): 4 channels, no FIFO
> - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> Hardware configuration of the channels is retrieved from the hardware
> configuration registers.
> The client can specify its channel requirements through device tree.
> STM32 DMA3 channels can be individually reserved either because they are
> secure, or dedicated to another CPU.
> Indeed, channels availability depends on Resource Isolation Framework
> (RIF) configuration. RIF grants access to buses with Compartiment ID
> (CIF) filtering, secure and privilege level. It also assigns DMA channels
> to one or several processors.
> DMA channels used by Linux should be CID-filtered and statically assigned
> to CID1 or shared with other CPUs but using semaphore. In case CID
> filtering is not configured, dma-channel-mask property can be used to
> specify available DMA channels to the kernel, otherwise such channels
> will be marked as reserved and can't be used by Linux.
> 
> Signed-off-by: Amelie Delaunay 
> ---
>  drivers/dma/stm32/Kconfig  |   10 +
>  drivers/dma/stm32/Makefile |1 +
>  drivers/dma/stm32/stm32-dma3.c | 1431 
>  3 files changed, 1442 insertions(+)
>  create mode 100644 drivers/dma/stm32/stm32-dma3.c
> 
> diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> index b72ae1a4502f..4d8d8063133b 100644
> --- a/drivers/dma/stm32/Kconfig
> +++ b/drivers/dma/stm32/Kconfig
> @@ -34,4 +34,14 @@ config STM32_MDMA
> If you have a board based on STM32 SoC with such DMA controller
> and want to use MDMA say Y here.
>  
> +config STM32_DMA3
> + tristate "STMicroelectronics STM32 DMA3 support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> +   Enable support for the on-chip DMA3 controller on STMicroelectronics
> +   STM32 platforms.
> +   If you have a board based on STM32 SoC with such DMA3 controller
> +   and want to use DMA3, say Y here.
> +
>  endif
> diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> index 663a3896a881..5082db4b4c1c 100644
> --- a/drivers/dma/stm32/Makefile
> +++ b/drivers/dma/stm32/Makefile
> @@ -2,3 +2,4 @@
>  obj-$(CONFIG_STM32_DMA) += stm32-dma.o
>  obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
>  obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> new file mode 100644
> index ..b5493f497d06
> --- /dev/null
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -0,0 +1,1431 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * STM32 DMA3 controller driver
> + *
> + * Copyright (C) STMicroelectronics 2024
> + * Author(s): Amelie Delaunay 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../virt-dma.h"
> +
> +#define STM32_DMA3_SECCFGR   0x00
> +#define STM32_DMA3_PRIVCFGR  0x04
> +#define STM32_DMA3_RCFGLOCKR 0x08
> +#define STM32_DMA3_MISR  0x0C
> +#define STM32_DMA3_SMISR 0x10
> +
> +#define STM32_DMA3_CLBAR(x)  (0x50 + 0x80 * (x))
> +#define STM32_DMA3_CCIDCFGR(x)   (0x54 + 0x80 * (x))
> +#define STM32_DMA3_CSEMCR(x) (0x58 + 0x80 * (x))
> +#define STM32_DMA3_CFCR(x)   (0x5C + 0x80 * (x))
> +#define STM32_DMA3_CSR(x)(0x60 + 0x80 * (x))
> +#define STM32_DMA3_CCR(x)(0x64 + 0x80 * (x))
> +#define STM32_DMA3_CTR1(x)   (0x90 + 0x80 * (x))
> +#define STM32_DMA3_CTR2(x)   (0x94 + 0x80 * (x))
> +#define STM32_DMA3_CBR1(x)   (0x98 + 0x80 * (x))
> +#define STM32_DMA3_CSAR(x)   (0x9C + 0x80 * (x))
> +#define STM32_DMA3_CDAR(x)   (0xA0 + 0x80 * (x))
> +#define STM32_DMA3_CLLR(x)   (0xCC + 0x80 * (x))
> +
> +#define STM32_DMA3_HWCFGR13  0xFC0 /* G_PER_CTRL(X) x=8..15 */
> +#define STM32_DMA3_HWCFGR12  0xFC4 /* G_PER_CTRL(X) x=0..7 */
> +#define STM32_DMA3_HWCFGR4   0xFE4 /* G_FIFO_SIZE(X) x=8..15 */
> +#define STM32_DMA3_HWCFGR3   0xFE8 /* G_FIFO_SIZE(X) x=0..7 */
> +#define STM32_DMA3_HWCFGR2   0xFEC /* G_MAX_REQ_ID */
> +#define STM32_DMA3_HWCFGR1   0xFF0 /* G_MASTER_PORTS, 
> G_NUM_CHANNELS, G_Mx_DATA_WIDTH */
> +#define STM32_DMA3_VERR  0xFF4
> +
> +/* SECCFGR DMA secure configuration register */
> +#define SECCFGR_SEC(x)   BIT(x)
> +
> +/* MISR DMA non-secure/secure masked interrupt status register */
> +#define MISR_MIS(x)  BIT(x)
> +
> +/* CxLBAR DMA channel x linked_list base ad

Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

2024-05-16 Thread Frank Li
On Thu, May 16, 2024 at 05:25:58PM +0200, Amelie Delaunay wrote:
> On 5/15/24 20:56, Frank Li wrote:
> > On Tue, Apr 23, 2024 at 02:32:55PM +0200, Amelie Delaunay wrote:
> > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> > > controller:
...
> > > + writel_relaxed(hwdesc->cdar, ddata->base + STM32_DMA3_CDAR(id));
> > > + writel_relaxed(hwdesc->cllr, ddata->base + STM32_DMA3_CLLR(id));
> > > +
> > > + /* Clear any pending interrupts */
> > > + csr = readl_relaxed(ddata->base + STM32_DMA3_CSR(id));
> > > + if (csr & CSR_ALL_F)
> > > + writel_relaxed(csr, ddata->base + STM32_DMA3_CFCR(id));
> > > +
> > > + stm32_dma3_chan_dump_reg(chan);
> > > +
> > > + ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(id));
> > > + writel_relaxed(ccr | CCR_EN, ddata->base + STM32_DMA3_CCR(id));
> > 
> > This one should use writel instead of writel_relaxed because it need
> > dma_wmb() as barrier for preious write complete.
> > 
> > Frank
> > 
> 
> ddata->base is Device memory type thanks to ioremap() use, so it is strongly
> ordered and non-cacheable.
> DMA3 is outside CPU cluster, its registers are accessible through AHB bus.
> dma_wmb() (in case of writel instead of writel_relaxed) is useless in that
> case: it won't ensure the propagation on the bus is complete, and it will
> have impacts on the system.
> That's why CCR register is written once,  then it is read before CCR_EN is
> set and being written again, with _relaxed(), because registers are behind a
> bus, and ioremapped with Device memory type which ensures it is strongly
> ordered and non-cacheable.

regardless memory map, writel_relaxed() just make sure io write and read is
orderred, not necessary order with other memory access. only readl and
writel make sure order with other memory read/write.

1. Write src_addr to descriptor
2. dma_wmb()
3. Write "ready" to descriptor
4. enable channel or doorbell by write a register.

if 4 use writel_relaxe(). because 3 write to DDR, which difference place of
mmio, 4 may happen before 3.  Your can refer axi order model.

4 have to use ONE writel(), to make sure 3 already write to DDR.

You need use at least one writel() to make sure all nornmal memory finish.

> 
> > > +
> > > + chan->dma_status = DMA_IN_PROGRESS;
> > > +
> > > + dev_dbg(chan2dev(chan), "vchan %pK: started\n", &chan->vchan);
> > > +}
> > > +
> > > +static int stm32_dma3_chan_suspend(struct stm32_dma3_chan *chan, bool 
> > > susp)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 csr, ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id)) & 
> > > ~CCR_EN;
> > > + int ret = 0;
> > > +
> > > + if (susp)
> > > + ccr |= CCR_SUSP;
> > > + else
> > > + ccr &= ~CCR_SUSP;
> > > +
> > > + writel_relaxed(ccr, ddata->base + STM32_DMA3_CCR(chan->id));
> > > +
> > > + if (susp) {
> > > + ret = readl_relaxed_poll_timeout_atomic(ddata->base + 
> > > STM32_DMA3_CSR(chan->id), csr,
> > > + csr & CSR_SUSPF, 1, 10);
> > > + if (!ret)
> > > + writel_relaxed(CFCR_SUSPF, ddata->base + 
> > > STM32_DMA3_CFCR(chan->id));
> > > +
> > > + stm32_dma3_chan_dump_reg(chan);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void stm32_dma3_chan_reset(struct stm32_dma3_chan *chan)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id)) & 
> > > ~CCR_EN;
> > > +
> > > + writel_relaxed(ccr |= CCR_RESET, ddata->base + 
> > > STM32_DMA3_CCR(chan->id));
> > > +}
> > > +
> > > +static int stm32_dma3_chan_stop(struct stm32_dma3_chan *chan)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 ccr;
> > > + int ret = 0;
> > > +
> > > + chan->dma_status = DMA_COMPLETE;
> > > +
> > > + /* Disable interrupts */
> > > + ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id));
> > > + writel_relaxed(ccr & ~(CCR_ALLIE | CCR_EN), ddata->base + 
> > > STM32_DMA3_CCR(chan->id));
> > > +
> > > + if (!(ccr &am

Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support

2024-05-17 Thread Frank Li
On Fri, May 17, 2024 at 11:42:17AM +0200, Amelie Delaunay wrote:
> On 5/16/24 19:09, Frank Li wrote:
> > On Thu, May 16, 2024 at 05:25:58PM +0200, Amelie Delaunay wrote:
> > > On 5/15/24 20:56, Frank Li wrote:
> > > > On Tue, Apr 23, 2024 at 02:32:55PM +0200, Amelie Delaunay wrote:
> > > > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 
> > > > > DMA3
> > > > > controller:
> > ...
> > > > > + writel_relaxed(hwdesc->cdar, ddata->base + STM32_DMA3_CDAR(id));
> > > > > + writel_relaxed(hwdesc->cllr, ddata->base + STM32_DMA3_CLLR(id));
> > > > > +
> > > > > + /* Clear any pending interrupts */
> > > > > + csr = readl_relaxed(ddata->base + STM32_DMA3_CSR(id));
> > > > > + if (csr & CSR_ALL_F)
> > > > > + writel_relaxed(csr, ddata->base + STM32_DMA3_CFCR(id));
> > > > > +
> > > > > + stm32_dma3_chan_dump_reg(chan);
> > > > > +
> > > > > + ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(id));
> > > > > + writel_relaxed(ccr | CCR_EN, ddata->base + STM32_DMA3_CCR(id));
> > > > 
> > > > This one should use writel instead of writel_relaxed because it need
> > > > dma_wmb() as barrier for preious write complete.
> > > > 
> > > > Frank
> > > > 
> > > 
> > > ddata->base is Device memory type thanks to ioremap() use, so it is 
> > > strongly
> > > ordered and non-cacheable.
> > > DMA3 is outside CPU cluster, its registers are accessible through AHB bus.
> > > dma_wmb() (in case of writel instead of writel_relaxed) is useless in that
> > > case: it won't ensure the propagation on the bus is complete, and it will
> > > have impacts on the system.
> > > That's why CCR register is written once,  then it is read before CCR_EN is
> > > set and being written again, with _relaxed(), because registers are 
> > > behind a
> > > bus, and ioremapped with Device memory type which ensures it is strongly
> > > ordered and non-cacheable.
> > 
> > regardless memory map, writel_relaxed() just make sure io write and read is
> > orderred, not necessary order with other memory access. only readl and
> > writel make sure order with other memory read/write.
> > 
> > 1. Write src_addr to descriptor
> > 2. dma_wmb()
> > 3. Write "ready" to descriptor
> > 4. enable channel or doorbell by write a register.
> > 
> > if 4 use writel_relaxe(). because 3 write to DDR, which difference place of
> > mmio, 4 may happen before 3.  Your can refer axi order model.
> > 
> > 4 have to use ONE writel(), to make sure 3 already write to DDR.
> > 
> > You need use at least one writel() to make sure all nornmal memory finish.
> > 
> 
> +writel_relaxed(chan->swdesc->ccr, ddata->base + STM32_DMA3_CCR(id));
> +writel_relaxed(hwdesc->ctr1, ddata->base + STM32_DMA3_CTR1(id));
> +writel_relaxed(hwdesc->ctr2, ddata->base + STM32_DMA3_CTR2(id));
> +writel_relaxed(hwdesc->cbr1, ddata->base + STM32_DMA3_CBR1(id));
> +writel_relaxed(hwdesc->csar, ddata->base + STM32_DMA3_CSAR(id));
> +writel_relaxed(hwdesc->cdar, ddata->base + STM32_DMA3_CDAR(id));
> +writel_relaxed(hwdesc->cllr, ddata->base + STM32_DMA3_CLLR(id));
> 
> These writel_relaxed() are from descriptors to DMA3 registers (descriptors
> being prepared "a long time ago" during _prep_).

You can't depend on "a long time ago" during _prep_. If later your driver
run at fast CPU. The execute time will be short.

All dma_map_sg and dma_alloc_coherence ... need at least one writel() to
make sure previous write actually reach to DDR. 

Some data may not really reach DDR, when DMA already start transfer

Please ref linux kernel document: 
Documentation/memory-barriers.txt, line 1948.

In your issue_pending(), call this function to enable channel. So need
at least one writel().

> As I said previously, DMA3 registers are outside CPU cluster, accessible
> through AHB bus, and ddata->base to address registers is ioremapped as
> Device memory type, non-cacheable and strongly ordered.
> 
> arch/arm/include/asm/io.h:
> /*
> * ioremap() and friends.
> *
> * ioremap() takes a resource address, and size.  Due to the ARM memory
> * types, it is important to use the correct ioremap() function as each
> * mapping has specific properties.
> *
> * FunctionMemory type Cacheability  

Re: [PATCH v3 05/12] dmaengine: Add STM32 DMA3 support

2024-05-20 Thread Frank Li
On Mon, May 20, 2024 at 05:49:41PM +0200, Amelie Delaunay wrote:
> STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> controller:
> - LPDMA (Low Power): 4 channels, no FIFO
> - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> Hardware configuration of the channels is retrieved from the hardware
> configuration registers.

Can you add empty line between segment?

> The client can specify its channel requirements through device tree.
> STM32 DMA3 channels can be individually reserved either because they are
> secure, or dedicated to another CPU.
> Indeed, channels availability depends on Resource Isolation Framework
> (RIF) configuration. RIF grants access to buses with Compartment ID (CID)
> filtering, secure and privilege level. It also assigns DMA channels to one
> or several processors.
> DMA channels used by Linux should be CID-filtered and statically assigned
> to CID1 or shared with other CPUs but using semaphore. In case CID
> filtering is not configured, dma-channel-mask property can be used to
> specify available DMA channels to the kernel, otherwise such channels
> will be marked as reserved and can't be used by Linux.
> 
> STM32 DMA3 is a new STM32 DMA controller, not a new version of an existing
> one.
> stm32-dma is not considered for reuse because register layout is completely
> different and doesn't rely on descriptors mechanism.
> stm32-mdma is based on descriptors mechanism but there are significant
> differences in register layout and descriptors structure.
> 
> Signed-off-by: Amelie Delaunay 
> ---
> v3:
> - fix typo in commit message (Compartment)
> - add a paragraph in commit message to explain why stm32-dma and stm32-mdma
>   are not reused
> - add explicit write memory barrier on the last descriptor initialization
> v2:
> - lowercase in hex values
> - compatible has been updated to st,stm32mp25-dma3 (SoC specific)
> ---
>  drivers/dma/stm32/Kconfig  |   10 +
>  drivers/dma/stm32/Makefile |1 +
>  drivers/dma/stm32/stm32-dma3.c | 1440 
>  3 files changed, 1451 insertions(+)
>  create mode 100644 drivers/dma/stm32/stm32-dma3.c
> 
> diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> index b72ae1a4502f..4d8d8063133b 100644
> --- a/drivers/dma/stm32/Kconfig
> +++ b/drivers/dma/stm32/Kconfig
> @@ -34,4 +34,14 @@ config STM32_MDMA
> If you have a board based on STM32 SoC with such DMA controller
> and want to use MDMA say Y here.
>  
> +config STM32_DMA3
> + tristate "STMicroelectronics STM32 DMA3 support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> +   Enable support for the on-chip DMA3 controller on STMicroelectronics
> +   STM32 platforms.
> +   If you have a board based on STM32 SoC with such DMA3 controller
> +   and want to use DMA3, say Y here.
> +
>  endif
> diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> index 663a3896a881..5082db4b4c1c 100644
> --- a/drivers/dma/stm32/Makefile
> +++ b/drivers/dma/stm32/Makefile
> @@ -2,3 +2,4 @@
>  obj-$(CONFIG_STM32_DMA) += stm32-dma.o
>  obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
>  obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> new file mode 100644
> index ..134c08a7ee96
> --- /dev/null
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -0,0 +1,1440 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * STM32 DMA3 controller driver
> + *
> + * Copyright (C) STMicroelectronics 2024
> + * Author(s): Amelie Delaunay 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../virt-dma.h"
> +
> +#define STM32_DMA3_SECCFGR   0x00
> +#define STM32_DMA3_PRIVCFGR  0x04
> +#define STM32_DMA3_RCFGLOCKR 0x08
> +#define STM32_DMA3_MISR  0x0c
> +#define STM32_DMA3_SMISR 0x10
> +
> +#define STM32_DMA3_CLBAR(x)  (0x50 + 0x80 * (x))
> +#define STM32_DMA3_CCIDCFGR(x)   (0x54 + 0x80 * (x))
> +#define STM32_DMA3_CSEMCR(x) (0x58 + 0x80 * (x))
> +#define STM32_DMA3_CFCR(x)   (0x5c + 0x80 * (x))
> +#define STM32_DMA3_CSR(x)(0x60 + 0x80 * (x))
> +#define STM32_DMA3_CCR(x)(0x64 + 0x80 * (x))
> +#define STM32_DMA3_CTR1(x)   (0x90 + 0x80 * (x))
> +#define STM32_DMA3_CTR2(x)   (0x94 + 0x80 * (x))
> +#define STM32_DMA3_CBR1(x)   (0x98 + 0x80 * (x))
> +#define STM32_DMA3_CSAR(x)   (0x9c + 0x80 * (x))
> +#define STM32_DMA3_CDAR(x)   (0xa0 + 0x80 * (x))
> +#define STM32_DMA3_CLLR(x)   (0xcc + 0x80 * (x))
> +
> +#define STM32_DMA3_HWCFGR13  0xfc0 /* G_PER_CTRL(X) x=8..15 *

Re: [PATCH v3 06/12] dmaengine: stm32-dma3: add DMA_CYCLIC capability

2024-05-20 Thread Frank Li
On Mon, May 20, 2024 at 05:49:42PM +0200, Amelie Delaunay wrote:
> Add DMA_CYCLIC capability and relative device_prep_dma_cyclic ops with
> stm32_dma3_prep_dma_cyclic(). It reuses stm32_dma3_chan_prep_hw() and
> stm32_dma3_chan_prep_hwdesc() helpers.
> 
> Signed-off-by: Amelie Delaunay 
> ---
>  drivers/dma/stm32/stm32-dma3.c | 77 ++
>  1 file changed, 77 insertions(+)
> 
> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> index 134c08a7ee96..aae9fc018b1d 100644
> --- a/drivers/dma/stm32/stm32-dma3.c
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -1021,6 +1021,81 @@ static struct dma_async_tx_descriptor 
> *stm32_dma3_prep_slave_sg(struct dma_chan
>   return NULL;
>  }
>  
> +static struct dma_async_tx_descriptor *stm32_dma3_prep_dma_cyclic(struct 
> dma_chan *c,
> +   dma_addr_t 
> buf_addr,
> +   size_t 
> buf_len, size_t period_len,
> +   enum 
> dma_transfer_direction dir,
> +   unsigned long 
> flags)
> +{
> + struct stm32_dma3_chan *chan = to_stm32_dma3_chan(c);
> + struct stm32_dma3_swdesc *swdesc;
> + dma_addr_t src, dst;
> + u32 count, i, ctr1, ctr2;
> + int ret;
> +
> + if (!buf_len || !period_len || period_len > STM32_DMA3_MAX_BLOCK_SIZE) {
> + dev_err(chan2dev(chan), "Invalid buffer/period length\n");
> + return NULL;
> + }
> +
> + if (buf_len % period_len) {
> + dev_err(chan2dev(chan), "Buffer length not multiple of period 
> length\n");
> + return NULL;
> + }
> +
> + count = buf_len / period_len;
> + swdesc = stm32_dma3_chan_desc_alloc(chan, count);
> + if (!swdesc)
> + return NULL;
> +
> + if (dir == DMA_MEM_TO_DEV) {
> + src = buf_addr;
> + dst = chan->dma_config.dst_addr;
> +
> + ret = stm32_dma3_chan_prep_hw(chan, DMA_MEM_TO_DEV, 
> &swdesc->ccr, &ctr1, &ctr2,
> +   src, dst, period_len);
> + } else if (dir == DMA_DEV_TO_MEM) {
> + src = chan->dma_config.src_addr;
> + dst = buf_addr;
> +
> + ret = stm32_dma3_chan_prep_hw(chan, DMA_DEV_TO_MEM, 
> &swdesc->ccr, &ctr1, &ctr2,
> +   src, dst, period_len);
> + } else {
> + dev_err(chan2dev(chan), "Invalid direction\n");
> + ret = -EINVAL;
> + }
> +
> + if (ret)
> + goto err_desc_free;
> +
> + for (i = 0; i < count; i++) {
> + if (dir == DMA_MEM_TO_DEV) {
> + src = buf_addr + i * period_len;
> + dst = chan->dma_config.dst_addr;
> + } else { /* (dir == DMA_DEV_TO_MEM || dir == DMA_MEM_TO_MEM) */

look like comment is wrong.

DMA_MEM_TO_MEM will return -EINVAL at previous check. 

> + src = chan->dma_config.src_addr;
> + dst = buf_addr + i * period_len;
> + }
> +
> + stm32_dma3_chan_prep_hwdesc(chan, swdesc, i, src, dst, 
> period_len,
> + ctr1, ctr2, i == (count - 1), true);
> + }
> +
> + /* Enable Error interrupts */
> + swdesc->ccr |= CCR_USEIE | CCR_ULEIE | CCR_DTEIE;
> + /* Enable Transfer state interrupts */
> + swdesc->ccr |= CCR_TCIE;
> +
> + swdesc->cyclic = true;
> +
> + return vchan_tx_prep(&chan->vchan, &swdesc->vdesc, flags);
> +
> +err_desc_free:
> + stm32_dma3_chan_desc_free(chan, swdesc);
> +
> + return NULL;
> +}
> +
>  static void stm32_dma3_caps(struct dma_chan *c, struct dma_slave_caps *caps)
>  {
>   struct stm32_dma3_chan *chan = to_stm32_dma3_chan(c);
> @@ -1255,6 +1330,7 @@ static int stm32_dma3_probe(struct platform_device 
> *pdev)
>  
>   dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
>   dma_cap_set(DMA_PRIVATE, dma_dev->cap_mask);
> + dma_cap_set(DMA_CYCLIC, dma_dev->cap_mask);
>   dma_dev->dev = &pdev->dev;
>   /*
>* This controller supports up to 8-byte buswidth depending on the port 
> used and the
> @@ -1277,6 +1353,7 @@ static int stm32_dma3_probe(struct platform_device 
> *pdev)
>   dma_dev->device_alloc_chan_resources = stm32_dma3_alloc_chan_resources;
>   dma_dev->device_free_chan_resources = stm32_dma3_free_chan_resources;
>   dma_dev->device_prep_slave_sg = stm32_dma3_prep_slave_sg;
> + dma_dev->device_prep_dma_cyclic = stm32_dma3_prep_dma_cyclic;
>   dma_dev->device_caps = stm32_dma3_caps;
>   dma_dev->device_config = stm32_dma3_config;
>   dma_dev->device_terminate_all = stm32_dma3_terminate_all;
> -- 
> 2.25.1
> 



Re: [PATCH v4 05/12] dmaengine: Add STM32 DMA3 support

2024-05-31 Thread Frank Li
On Fri, May 31, 2024 at 05:07:05PM +0200, Amelie Delaunay wrote:
> STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> controller:
> - LPDMA (Low Power): 4 channels, no FIFO
> - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> Hardware configuration of the channels is retrieved from the hardware
> configuration registers.
> 
> The client can specify its channel requirements through device tree.
> STM32 DMA3 channels can be individually reserved either because they are
> secure, or dedicated to another CPU.
> Indeed, channels availability depends on Resource Isolation Framework
> (RIF) configuration. RIF grants access to buses with Compartment ID (CID)
> filtering, secure and privilege level. It also assigns DMA channels to one
> or several processors.
> DMA channels used by Linux should be CID-filtered and statically assigned
> to CID1 or shared with other CPUs but using semaphore. In case CID
> filtering is not configured, dma-channel-mask property can be used to
> specify available DMA channels to the kernel, otherwise such channels
> will be marked as reserved and can't be used by Linux.
> 
> STM32 DMA3 is a new STM32 DMA controller, not a new version of an existing
> one.
> stm32-dma is not considered for reuse because register layout is completely
> different and doesn't rely on descriptors mechanism.
> stm32-mdma is based on descriptors mechanism but there are significant
> differences in register layout and descriptors structure.
> 
> Signed-off-by: Amelie Delaunay 
> ---
> v4:
> - align comments alignment in enum definition
> - use __packed for stm32_dma3_hwdesc structure
> - use generic dma_wmb() instead of __iowmb()
> v3:
> - fix typo in commit message (Compartment)
> - add a paragraph in commit message to explain why stm32-dma and stm32-mdma
>   are not reused
> - add explicit write memory barrier on the last descriptor initialization
> v2:
> - lowercase in hex values
> - compatible has been updated to st,stm32mp25-dma3 (SoC specific)
> ---
>  drivers/dma/stm32/Kconfig  |   10 +
>  drivers/dma/stm32/Makefile |1 +
>  drivers/dma/stm32/stm32-dma3.c | 1440 
>  3 files changed, 1451 insertions(+)
>  create mode 100644 drivers/dma/stm32/stm32-dma3.c
> 
> diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> index b72ae1a4502f..4d8d8063133b 100644
> --- a/drivers/dma/stm32/Kconfig
> +++ b/drivers/dma/stm32/Kconfig
> @@ -34,4 +34,14 @@ config STM32_MDMA
> If you have a board based on STM32 SoC with such DMA controller
> and want to use MDMA say Y here.
>  
> +config STM32_DMA3
> + tristate "STMicroelectronics STM32 DMA3 support"
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> +   Enable support for the on-chip DMA3 controller on STMicroelectronics
> +   STM32 platforms.
> +   If you have a board based on STM32 SoC with such DMA3 controller
> +   and want to use DMA3, say Y here.
> +
>  endif
> diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> index 663a3896a881..5082db4b4c1c 100644
> --- a/drivers/dma/stm32/Makefile
> +++ b/drivers/dma/stm32/Makefile
> @@ -2,3 +2,4 @@
>  obj-$(CONFIG_STM32_DMA) += stm32-dma.o
>  obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
>  obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> new file mode 100644
> index ..49886117d29b
> --- /dev/null
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -0,0 +1,1440 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * STM32 DMA3 controller driver
> + *
> + * Copyright (C) STMicroelectronics 2024
> + * Author(s): Amelie Delaunay 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../virt-dma.h"
> +
> +#define STM32_DMA3_SECCFGR   0x00
> +#define STM32_DMA3_PRIVCFGR  0x04
> +#define STM32_DMA3_RCFGLOCKR 0x08
> +#define STM32_DMA3_MISR  0x0c
> +#define STM32_DMA3_SMISR 0x10
> +
> +#define STM32_DMA3_CLBAR(x)  (0x50 + 0x80 * (x))
> +#define STM32_DMA3_CCIDCFGR(x)   (0x54 + 0x80 * (x))
> +#define STM32_DMA3_CSEMCR(x) (0x58 + 0x80 * (x))
> +#define STM32_DMA3_CFCR(x)   (0x5c + 0x80 * (x))
> +#define STM32_DMA3_CSR(x)(0x60 + 0x80 * (x))
> +#define STM32_DMA3_CCR(x)(0x64 + 0x80 * (x))
> +#define STM32_DMA3_CTR1(x)   (0x90 + 0x80 * (x))
> +#define STM32_DMA3_CTR2(x)   (0x94 + 0x80 * (x))
> +#define STM32_DMA3_CBR1(x)   (0x98 + 0x80 * (x))
> +#define STM32_DMA3_CSAR(x)   (0x9c + 0x80 * (x))
> +#define STM32_DMA3_CDAR(x)   (0xa0 + 0x80 * (x))
> +#define STM32_DMA3_CLL

Re: [PATCH v4 05/12] dmaengine: Add STM32 DMA3 support

2024-06-03 Thread Frank Li
On Mon, Jun 03, 2024 at 10:09:36AM +0200, Amelie Delaunay wrote:
> On 5/31/24 22:21, Frank Li wrote:
> > On Fri, May 31, 2024 at 05:07:05PM +0200, Amelie Delaunay wrote:
> > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> > > controller:
> > > - LPDMA (Low Power): 4 channels, no FIFO
> > > - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
> > > - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
> > > Hardware configuration of the channels is retrieved from the hardware
> > > configuration registers.
> > > 
> > > The client can specify its channel requirements through device tree.
> > > STM32 DMA3 channels can be individually reserved either because they are
> > > secure, or dedicated to another CPU.
> > > Indeed, channels availability depends on Resource Isolation Framework
> > > (RIF) configuration. RIF grants access to buses with Compartment ID (CID)
> > > filtering, secure and privilege level. It also assigns DMA channels to one
> > > or several processors.
> > > DMA channels used by Linux should be CID-filtered and statically assigned
> > > to CID1 or shared with other CPUs but using semaphore. In case CID
> > > filtering is not configured, dma-channel-mask property can be used to
> > > specify available DMA channels to the kernel, otherwise such channels
> > > will be marked as reserved and can't be used by Linux.
> > > 
> > > STM32 DMA3 is a new STM32 DMA controller, not a new version of an existing
> > > one.
> > > stm32-dma is not considered for reuse because register layout is 
> > > completely
> > > different and doesn't rely on descriptors mechanism.
> > > stm32-mdma is based on descriptors mechanism but there are significant
> > > differences in register layout and descriptors structure.
> > > 
> > > Signed-off-by: Amelie Delaunay 
> > > ---
> > > v4:
> > > - align comments alignment in enum definition
> > > - use __packed for stm32_dma3_hwdesc structure
> > > - use generic dma_wmb() instead of __iowmb()
> > > v3:
> > > - fix typo in commit message (Compartment)
> > > - add a paragraph in commit message to explain why stm32-dma and 
> > > stm32-mdma
> > >are not reused
> > > - add explicit write memory barrier on the last descriptor initialization
> > > v2:
> > > - lowercase in hex values
> > > - compatible has been updated to st,stm32mp25-dma3 (SoC specific)
> > > ---
> > >   drivers/dma/stm32/Kconfig  |   10 +
> > >   drivers/dma/stm32/Makefile |1 +
> > >   drivers/dma/stm32/stm32-dma3.c | 1440 
> > >   3 files changed, 1451 insertions(+)
> > >   create mode 100644 drivers/dma/stm32/stm32-dma3.c
> > > 
> > > diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
> > > index b72ae1a4502f..4d8d8063133b 100644
> > > --- a/drivers/dma/stm32/Kconfig
> > > +++ b/drivers/dma/stm32/Kconfig
> > > @@ -34,4 +34,14 @@ config STM32_MDMA
> > > If you have a board based on STM32 SoC with such DMA 
> > > controller
> > > and want to use MDMA say Y here.
> > > +config STM32_DMA3
> > > + tristate "STMicroelectronics STM32 DMA3 support"
> > > + select DMA_ENGINE
> > > + select DMA_VIRTUAL_CHANNELS
> > > + help
> > > +   Enable support for the on-chip DMA3 controller on STMicroelectronics
> > > +   STM32 platforms.
> > > +   If you have a board based on STM32 SoC with such DMA3 controller
> > > +   and want to use DMA3, say Y here.
> > > +
> > >   endif
> > > diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
> > > index 663a3896a881..5082db4b4c1c 100644
> > > --- a/drivers/dma/stm32/Makefile
> > > +++ b/drivers/dma/stm32/Makefile
> > > @@ -2,3 +2,4 @@
> > >   obj-$(CONFIG_STM32_DMA) += stm32-dma.o
> > >   obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
> > >   obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
> > > +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
> > > diff --git a/drivers/dma/stm32/stm32-dma3.c 
> > > b/drivers/dma/stm32/stm32-dma3.c
> > > new file mode 100644
> > > index ..49886117d29b
> > > --- /dev/null
> > > +++ b/drivers/dma/stm32/stm32-dma3.c
> > > @@ -0,0 +1,1440 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
>