Message ID | 20191101084135.14811-1-peter.ujfalusi@ti.com |
---|---|
Headers | show |
Series | dmaengine/soc: Add Texas Instruments UDMA support | expand |
On 01/11/2019 10:41, Peter Ujfalusi wrote: > In K3 architecture the DMA operates within threads. One end of the thread > is UDMAP, the other is on the peripheral side. > > The UDMAP channel configuration depends on the needs of the remote > endpoint and it can be differ from peripheral to peripheral. > > This patch adds database for am654 and j721e and small API to fetch the > PSI-L endpoint configuration from the database which should only used by > the DMA driver(s). > > Another API is added for native peripherals to give possibility to pass new > configuration for the threads they are using, which is needed to be able to > handle changes caused by different firmware loaded for the peripheral for > example. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/ti/Kconfig | 3 + > drivers/dma/ti/Makefile | 1 + > drivers/dma/ti/k3-psil-am654.c | 172 ++++++++++++++++++++++++++ > drivers/dma/ti/k3-psil-j721e.c | 219 +++++++++++++++++++++++++++++++++ > drivers/dma/ti/k3-psil-priv.h | 39 ++++++ > drivers/dma/ti/k3-psil.c | 97 +++++++++++++++ > include/linux/dma/k3-psil.h | 47 +++++++ > 7 files changed, 578 insertions(+) > create mode 100644 drivers/dma/ti/k3-psil-am654.c > create mode 100644 drivers/dma/ti/k3-psil-j721e.c > create mode 100644 drivers/dma/ti/k3-psil-priv.h > create mode 100644 drivers/dma/ti/k3-psil.c > create mode 100644 include/linux/dma/k3-psil.h > > diff --git a/drivers/dma/ti/Kconfig b/drivers/dma/ti/Kconfig > index d507c24fbf31..72f3d2728178 100644 > --- a/drivers/dma/ti/Kconfig > +++ b/drivers/dma/ti/Kconfig > @@ -34,5 +34,8 @@ config DMA_OMAP > Enable support for the TI sDMA (System DMA or DMA4) controller. This > DMA engine is found on OMAP and DRA7xx parts. > > +config TI_K3_PSIL > + bool > + > config TI_DMA_CROSSBAR > bool > diff --git a/drivers/dma/ti/Makefile b/drivers/dma/ti/Makefile > index 113e59ec9c32..f8d912ad7eaf 100644 > --- a/drivers/dma/ti/Makefile > +++ b/drivers/dma/ti/Makefile > @@ -2,4 +2,5 @@ > obj-$(CONFIG_TI_CPPI41) += cppi41.o > obj-$(CONFIG_TI_EDMA) += edma.o > obj-$(CONFIG_DMA_OMAP) += omap-dma.o > +obj-$(CONFIG_TI_K3_PSIL) += k3-psil.o k3-psil-am654.o k3-psil-j721e.o > obj-$(CONFIG_TI_DMA_CROSSBAR) += dma-crossbar.o > diff --git a/drivers/dma/ti/k3-psil-am654.c b/drivers/dma/ti/k3-psil-am654.c > new file mode 100644 > index 000000000000..edd7fff36f44 > --- /dev/null > +++ b/drivers/dma/ti/k3-psil-am654.c > @@ -0,0 +1,172 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + * Author: Peter Ujfalusi <peter.ujfalusi@ti.com> > + */ > + > +#include <linux/kernel.h> > + > +#include "k3-psil-priv.h" > + > +#define PSIL_PDMA_XY_TR(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_PDMA_XY, \ > + }, \ > + } > + > +#define PSIL_PDMA_XY_PKT(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_PDMA_XY, \ > + .pkt_mode = 1, \ > + }, \ > + } > + > +#define PSIL_ETHERNET(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_NATIVE, \ > + .pkt_mode = 1, \ > + .needs_epib = 1, \ > + .psd_size = 16, \ > + }, \ > + } > + > +#define PSIL_SA2UL(x, tx) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_NATIVE, \ > + .pkt_mode = 1, \ > + .needs_epib = 1, \ > + .psd_size = 64, \ > + .notdpkt = tx, \ > + }, \ > + } > + > +/* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */ > +struct psil_ep am654_src_ep_map[] = { > + /* SA2UL */ > + PSIL_SA2UL(0x4000, 0), > + PSIL_SA2UL(0x4001, 0), > + /* PRU_ICSSG0 */ > + PSIL_ETHERNET(0x4100), > + PSIL_ETHERNET(0x4101), > + PSIL_ETHERNET(0x4102), > + PSIL_ETHERNET(0x4103), > + /* PRU_ICSSG1 */ > + PSIL_ETHERNET(0x4200), > + PSIL_ETHERNET(0x4201), > + PSIL_ETHERNET(0x4202), > + PSIL_ETHERNET(0x4203), > + /* PRU_ICSSG2 */ > + PSIL_ETHERNET(0x4300), > + PSIL_ETHERNET(0x4301), > + PSIL_ETHERNET(0x4302), > + PSIL_ETHERNET(0x4303), > + /* PDMA0 - McASPs */ > + PSIL_PDMA_XY_TR(0x4400), > + PSIL_PDMA_XY_TR(0x4401), > + PSIL_PDMA_XY_TR(0x4402), > + /* PDMA1 - SPI0-4 */ > + PSIL_PDMA_XY_PKT(0x4500), > + PSIL_PDMA_XY_PKT(0x4501), > + PSIL_PDMA_XY_PKT(0x4502), > + PSIL_PDMA_XY_PKT(0x4503), > + PSIL_PDMA_XY_PKT(0x4504), > + PSIL_PDMA_XY_PKT(0x4505), > + PSIL_PDMA_XY_PKT(0x4506), > + PSIL_PDMA_XY_PKT(0x4507), > + PSIL_PDMA_XY_PKT(0x4508), > + PSIL_PDMA_XY_PKT(0x4509), > + PSIL_PDMA_XY_PKT(0x450a), > + PSIL_PDMA_XY_PKT(0x450b), > + PSIL_PDMA_XY_PKT(0x450c), > + PSIL_PDMA_XY_PKT(0x450d), > + PSIL_PDMA_XY_PKT(0x450e), > + PSIL_PDMA_XY_PKT(0x450f), > + PSIL_PDMA_XY_PKT(0x4510), > + PSIL_PDMA_XY_PKT(0x4511), > + PSIL_PDMA_XY_PKT(0x4512), > + PSIL_PDMA_XY_PKT(0x4513), > + /* PDMA1 - USART0-2 */ > + PSIL_PDMA_XY_PKT(0x4514), > + PSIL_PDMA_XY_PKT(0x4515), > + PSIL_PDMA_XY_PKT(0x4516), > + /* CPSW0 */ > + PSIL_ETHERNET(0x7000), > + /* MCU_PDMA0 - ADCs */ > + PSIL_PDMA_XY_TR(0x7100), > + PSIL_PDMA_XY_TR(0x7101), > + PSIL_PDMA_XY_TR(0x7102), > + PSIL_PDMA_XY_TR(0x7103), > + /* MCU_PDMA1 - MCU_SPI0-2 */ > + PSIL_PDMA_XY_PKT(0x7200), > + PSIL_PDMA_XY_PKT(0x7201), > + PSIL_PDMA_XY_PKT(0x7202), > + PSIL_PDMA_XY_PKT(0x7203), > + PSIL_PDMA_XY_PKT(0x7204), > + PSIL_PDMA_XY_PKT(0x7205), > + PSIL_PDMA_XY_PKT(0x7206), > + PSIL_PDMA_XY_PKT(0x7207), > + PSIL_PDMA_XY_PKT(0x7208), > + PSIL_PDMA_XY_PKT(0x7209), > + PSIL_PDMA_XY_PKT(0x720a), > + PSIL_PDMA_XY_PKT(0x720b), > + /* MCU_PDMA1 - MCU_USART0 */ > + PSIL_PDMA_XY_PKT(0x7212), > +}; > + > +/* PSI-L destination thread IDs, used for TX (DMA_MEM_TO_DEV) */ > +struct psil_ep am654_dst_ep_map[] = { > + /* SA2UL */ > + PSIL_SA2UL(0xc000, 1), > + /* PRU_ICSSG0 */ > + PSIL_ETHERNET(0xc100), > + PSIL_ETHERNET(0xc101), > + PSIL_ETHERNET(0xc102), > + PSIL_ETHERNET(0xc103), > + PSIL_ETHERNET(0xc104), > + PSIL_ETHERNET(0xc105), > + PSIL_ETHERNET(0xc106), > + PSIL_ETHERNET(0xc107), > + /* PRU_ICSSG1 */ > + PSIL_ETHERNET(0xc200), > + PSIL_ETHERNET(0xc201), > + PSIL_ETHERNET(0xc202), > + PSIL_ETHERNET(0xc203), > + PSIL_ETHERNET(0xc204), > + PSIL_ETHERNET(0xc205), > + PSIL_ETHERNET(0xc206), > + PSIL_ETHERNET(0xc207), > + /* PRU_ICSSG2 */ > + PSIL_ETHERNET(0xc300), > + PSIL_ETHERNET(0xc301), > + PSIL_ETHERNET(0xc302), > + PSIL_ETHERNET(0xc303), > + PSIL_ETHERNET(0xc304), > + PSIL_ETHERNET(0xc305), > + PSIL_ETHERNET(0xc306), > + PSIL_ETHERNET(0xc307), > + /* CPSW0 */ > + PSIL_ETHERNET(0xf000), > + PSIL_ETHERNET(0xf001), > + PSIL_ETHERNET(0xf002), > + PSIL_ETHERNET(0xf003), > + PSIL_ETHERNET(0xf004), > + PSIL_ETHERNET(0xf005), > + PSIL_ETHERNET(0xf006), > + PSIL_ETHERNET(0xf007), > +}; > + > +struct psil_ep_map am654_ep_map = { > + .name = "am654", > + .src = am654_src_ep_map, > + .src_count = ARRAY_SIZE(am654_src_ep_map), > + .dst = am654_dst_ep_map, > + .dst_count = ARRAY_SIZE(am654_dst_ep_map), > +}; > diff --git a/drivers/dma/ti/k3-psil-j721e.c b/drivers/dma/ti/k3-psil-j721e.c > new file mode 100644 > index 000000000000..86e1ff57e197 > --- /dev/null > +++ b/drivers/dma/ti/k3-psil-j721e.c > @@ -0,0 +1,219 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + * Author: Peter Ujfalusi <peter.ujfalusi@ti.com> > + */ > + > +#include <linux/kernel.h> > + > +#include "k3-psil-priv.h" > + > +#define PSIL_PDMA_XY_TR(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_PDMA_XY, \ > + }, \ > + } > + > +#define PSIL_PDMA_XY_PKT(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_PDMA_XY, \ > + .pkt_mode = 1, \ > + }, \ > + } > + > +#define PSIL_PDMA_MCASP(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_PDMA_XY, \ > + .pdma_acc32 = 1, \ > + .pdma_burst = 1, \ > + }, \ > + } > + > +#define PSIL_ETHERNET(x) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_NATIVE, \ > + .pkt_mode = 1, \ > + .needs_epib = 1, \ > + .psd_size = 16, \ > + }, \ > + } > + > +#define PSIL_SA2UL(x, tx) \ > + { \ > + .thread_id = x, \ > + .ep_config = { \ > + .ep_type = PSIL_EP_NATIVE, \ > + .pkt_mode = 1, \ > + .needs_epib = 1, \ > + .psd_size = 64, \ > + .notdpkt = tx, \ > + }, \ > + } > + > +/* PSI-L source thread IDs, used for RX (DMA_DEV_TO_MEM) */ > +struct psil_ep j721e_src_ep_map[] = { > + /* SA2UL */ > + PSIL_SA2UL(0x4000, 0), > + PSIL_SA2UL(0x4001, 0), > + /* PRU_ICSSG0 */ > + PSIL_ETHERNET(0x4100), > + PSIL_ETHERNET(0x4101), > + PSIL_ETHERNET(0x4102), > + PSIL_ETHERNET(0x4103), > + /* PRU_ICSSG1 */ > + PSIL_ETHERNET(0x4200), > + PSIL_ETHERNET(0x4201), > + PSIL_ETHERNET(0x4202), > + PSIL_ETHERNET(0x4203), > + /* PDMA6 (PSIL_PDMA_MCASP_G0) - McASP0-2 */ > + PSIL_PDMA_MCASP(0x4400), > + PSIL_PDMA_MCASP(0x4401), > + PSIL_PDMA_MCASP(0x4402), > + /* PDMA7 (PSIL_PDMA_MCASP_G1) - McASP3-11 */ > + PSIL_PDMA_MCASP(0x4500), > + PSIL_PDMA_MCASP(0x4501), > + PSIL_PDMA_MCASP(0x4502), > + PSIL_PDMA_MCASP(0x4503), > + PSIL_PDMA_MCASP(0x4504), > + PSIL_PDMA_MCASP(0x4505), > + PSIL_PDMA_MCASP(0x4506), > + PSIL_PDMA_MCASP(0x4507), > + PSIL_PDMA_MCASP(0x4508), > + /* PDMA8 (PDMA_MISC_G0) - SPI0-1 */ > + PSIL_PDMA_XY_PKT(0x4600), > + PSIL_PDMA_XY_PKT(0x4601), > + PSIL_PDMA_XY_PKT(0x4602), > + PSIL_PDMA_XY_PKT(0x4603), > + PSIL_PDMA_XY_PKT(0x4604), > + PSIL_PDMA_XY_PKT(0x4605), > + PSIL_PDMA_XY_PKT(0x4606), > + PSIL_PDMA_XY_PKT(0x4607), > + /* PDMA9 (PDMA_MISC_G1) - SPI2-3 */ > + PSIL_PDMA_XY_PKT(0x460c), > + PSIL_PDMA_XY_PKT(0x460d), > + PSIL_PDMA_XY_PKT(0x460e), > + PSIL_PDMA_XY_PKT(0x460f), > + PSIL_PDMA_XY_PKT(0x4610), > + PSIL_PDMA_XY_PKT(0x4611), > + PSIL_PDMA_XY_PKT(0x4612), > + PSIL_PDMA_XY_PKT(0x4613), > + /* PDMA10 (PDMA_MISC_G2) - SPI4-5 */ > + PSIL_PDMA_XY_PKT(0x4618), > + PSIL_PDMA_XY_PKT(0x4619), > + PSIL_PDMA_XY_PKT(0x461a), > + PSIL_PDMA_XY_PKT(0x461b), > + PSIL_PDMA_XY_PKT(0x461c), > + PSIL_PDMA_XY_PKT(0x461d), > + PSIL_PDMA_XY_PKT(0x461e), > + PSIL_PDMA_XY_PKT(0x461f), > + /* PDMA11 (PDMA_MISC_G3) */ > + PSIL_PDMA_XY_PKT(0x4624), > + PSIL_PDMA_XY_PKT(0x4625), > + PSIL_PDMA_XY_PKT(0x4626), > + PSIL_PDMA_XY_PKT(0x4627), > + PSIL_PDMA_XY_PKT(0x4628), > + PSIL_PDMA_XY_PKT(0x4629), > + PSIL_PDMA_XY_PKT(0x4630), > + PSIL_PDMA_XY_PKT(0x463a), > + /* PDMA13 (PDMA_USART_G0) - UART0-1 */ > + PSIL_PDMA_XY_PKT(0x4700), > + PSIL_PDMA_XY_PKT(0x4701), > + /* PDMA14 (PDMA_USART_G1) - UART2-3 */ > + PSIL_PDMA_XY_PKT(0x4702), > + PSIL_PDMA_XY_PKT(0x4703), > + /* PDMA15 (PDMA_USART_G2) - UART4-9 */ > + PSIL_PDMA_XY_PKT(0x4704), > + PSIL_PDMA_XY_PKT(0x4705), > + PSIL_PDMA_XY_PKT(0x4706), > + PSIL_PDMA_XY_PKT(0x4707), > + PSIL_PDMA_XY_PKT(0x4708), > + PSIL_PDMA_XY_PKT(0x4709), > + /* CPSW9 */ > + PSIL_ETHERNET(0x4a00), > + /* CPSW0 */ > + PSIL_ETHERNET(0x7000), > + /* MCU_PDMA0 (MCU_PDMA_MISC_G0) - SPI0 */ > + PSIL_PDMA_XY_PKT(0x7100), > + PSIL_PDMA_XY_PKT(0x7101), > + PSIL_PDMA_XY_PKT(0x7102), > + PSIL_PDMA_XY_PKT(0x7103), > + /* MCU_PDMA1 (MCU_PDMA_MISC_G1) - SPI1-2 */ > + PSIL_PDMA_XY_PKT(0x7200), > + PSIL_PDMA_XY_PKT(0x7201), > + PSIL_PDMA_XY_PKT(0x7202), > + PSIL_PDMA_XY_PKT(0x7203), > + PSIL_PDMA_XY_PKT(0x7204), > + PSIL_PDMA_XY_PKT(0x7205), > + PSIL_PDMA_XY_PKT(0x7206), > + PSIL_PDMA_XY_PKT(0x7207), > + /* MCU_PDMA2 (MCU_PDMA_MISC_G2) - UART0 */ > + PSIL_PDMA_XY_PKT(0x7300), > + /* MCU_PDMA_ADC - ADC0-1 */ > + PSIL_PDMA_XY_TR(0x7400), > + PSIL_PDMA_XY_TR(0x7401), > + PSIL_PDMA_XY_TR(0x7402), > + PSIL_PDMA_XY_TR(0x7403), > + /* SA2UL */ > + PSIL_SA2UL(0x7500, 0), > + PSIL_SA2UL(0x7501, 0), > +}; > + > +/* PSI-L destination thread IDs, used for TX (DMA_MEM_TO_DEV) */ > +struct psil_ep j721e_dst_ep_map[] = { > + /* SA2UL */ > + PSIL_SA2UL(0xc000, 1), > + /* PRU_ICSSG0 */ > + PSIL_ETHERNET(0xc100), > + PSIL_ETHERNET(0xc101), > + PSIL_ETHERNET(0xc102), > + PSIL_ETHERNET(0xc103), > + PSIL_ETHERNET(0xc104), > + PSIL_ETHERNET(0xc105), > + PSIL_ETHERNET(0xc106), > + PSIL_ETHERNET(0xc107), > + /* PRU_ICSSG1 */ > + PSIL_ETHERNET(0xc200), > + PSIL_ETHERNET(0xc201), > + PSIL_ETHERNET(0xc202), > + PSIL_ETHERNET(0xc203), > + PSIL_ETHERNET(0xc204), > + PSIL_ETHERNET(0xc205), > + PSIL_ETHERNET(0xc206), > + PSIL_ETHERNET(0xc207), > + /* CPSW9 */ > + PSIL_ETHERNET(0xca00), > + PSIL_ETHERNET(0xca01), > + PSIL_ETHERNET(0xca02), > + PSIL_ETHERNET(0xca03), > + PSIL_ETHERNET(0xca04), > + PSIL_ETHERNET(0xca05), > + PSIL_ETHERNET(0xca06), > + PSIL_ETHERNET(0xca07), > + /* CPSW0 */ > + PSIL_ETHERNET(0xf000), > + PSIL_ETHERNET(0xf001), > + PSIL_ETHERNET(0xf002), > + PSIL_ETHERNET(0xf003), > + PSIL_ETHERNET(0xf004), > + PSIL_ETHERNET(0xf005), > + PSIL_ETHERNET(0xf006), > + PSIL_ETHERNET(0xf007), > + /* SA2UL */ > + PSIL_SA2UL(0xf500, 1), > +}; > + > +struct psil_ep_map j721e_ep_map = { > + .name = "j721e", > + .src = j721e_src_ep_map, > + .src_count = ARRAY_SIZE(j721e_src_ep_map), > + .dst = j721e_dst_ep_map, > + .dst_count = ARRAY_SIZE(j721e_dst_ep_map), > +}; > diff --git a/drivers/dma/ti/k3-psil-priv.h b/drivers/dma/ti/k3-psil-priv.h > new file mode 100644 > index 000000000000..f74420653d8a > --- /dev/null > +++ b/drivers/dma/ti/k3-psil-priv.h > @@ -0,0 +1,39 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + */ > + > +#ifndef K3_PSIL_PRIV_H_ > +#define K3_PSIL_PRIV_H_ > + > +#include <linux/dma/k3-psil.h> > + > +struct psil_ep { > + u32 thread_id; > + struct psil_endpoint_config ep_config; > +}; > + > +/** > + * struct psil_ep_map - PSI-L thread ID configuration maps > + * @name: Name of the map, set it to the name of the SoC > + * @src: Array of source PSI-L thread configurations > + * @src_count: Number of entries in the src array > + * @dst: Array of destination PSI-L thread configurations > + * @dst_count: Number of entries in the dst array > + * > + * In case of symmetric configuration for a matching src/dst thread (for example > + * 0x4400 and 0xc400) only the src configuration can be present. If no dst > + * configuration found the code will look for (dst_thread_id & ~0x8000) to find > + * the symmetric match. > + */ > +struct psil_ep_map { > + char *name; > + struct psil_ep *src; > + int src_count; > + struct psil_ep *dst; > + int dst_count; > +}; > + > +struct psil_endpoint_config *psil_get_ep_config(u32 thread_id); > + > +#endif /* K3_PSIL_PRIV_H_ */ > diff --git a/drivers/dma/ti/k3-psil.c b/drivers/dma/ti/k3-psil.c > new file mode 100644 > index 000000000000..e610022f09f4 > --- /dev/null > +++ b/drivers/dma/ti/k3-psil.c > @@ -0,0 +1,97 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + * Author: Peter Ujfalusi <peter.ujfalusi@ti.com> > + */ > + > +#include <linux/kernel.h> > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > + > +#include "k3-psil-priv.h" > + > +extern struct psil_ep_map am654_ep_map; > +extern struct psil_ep_map j721e_ep_map; > + > +static DEFINE_MUTEX(ep_map_mutex); > +static struct psil_ep_map *soc_ep_map; So, you are only protecting the high level soc_ep_map pointer only. You don't need to protect the database itself via some usecounting or something, or are you doing it within the DMA driver? -Tero > + > +struct psil_endpoint_config *psil_get_ep_config(u32 thread_id) > +{ > + int i; > + > + mutex_lock(&ep_map_mutex); > + if (!soc_ep_map) { > + if (of_machine_is_compatible("ti,am654")) { > + soc_ep_map = &am654_ep_map; > + } else if (of_machine_is_compatible("ti,j721e")) { > + soc_ep_map = &j721e_ep_map; > + } else { > + pr_err("PSIL: No compatible machine found for map\n"); > + return ERR_PTR(-ENOTSUPP); > + } > + pr_debug("%s: Using map for %s\n", __func__, soc_ep_map->name); > + } > + mutex_unlock(&ep_map_mutex); > + > + if (thread_id & K3_PSIL_DST_THREAD_ID_OFFSET && soc_ep_map->dst) { > + /* check in destination thread map */ > + for (i = 0; i < soc_ep_map->dst_count; i++) { > + if (soc_ep_map->dst[i].thread_id == thread_id) > + return &soc_ep_map->dst[i].ep_config; > + } > + } > + > + thread_id &= ~K3_PSIL_DST_THREAD_ID_OFFSET; > + if (soc_ep_map->src) { > + for (i = 0; i < soc_ep_map->src_count; i++) { > + if (soc_ep_map->src[i].thread_id == thread_id) > + return &soc_ep_map->src[i].ep_config; > + } > + } > + > + return ERR_PTR(-ENOENT); > +} > +EXPORT_SYMBOL(psil_get_ep_config); > + > +int psil_set_new_ep_config(struct device *dev, const char *name, > + struct psil_endpoint_config *ep_config) > +{ > + struct psil_endpoint_config *dst_ep_config; > + struct of_phandle_args dma_spec; > + u32 thread_id; > + int index; > + > + if (!dev || !dev->of_node) > + return -EINVAL; > + > + index = of_property_match_string(dev->of_node, "dma-names", name); > + if (index < 0) > + return index; > + > + if (of_parse_phandle_with_args(dev->of_node, "dmas", "#dma-cells", > + index, &dma_spec)) > + return -ENOENT; > + > + thread_id = dma_spec.args[0]; > + > + dst_ep_config = psil_get_ep_config(thread_id); > + if (IS_ERR(dst_ep_config)) { > + pr_err("PSIL: thread ID 0x%04x not defined in map\n", > + thread_id); > + of_node_put(dma_spec.np); > + return PTR_ERR(dst_ep_config); > + } > + > + memcpy(dst_ep_config, ep_config, sizeof(*dst_ep_config)); > + > + of_node_put(dma_spec.np); > + return 0; > +} > +EXPORT_SYMBOL(psil_set_new_ep_config); > + > +MODULE_DESCRIPTION("TI K3 PSI-L endpoint database"); > +MODULE_AUTHOR("Peter Ujfalusi <peter.ujfalusi@ti.com>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/dma/k3-psil.h b/include/linux/dma/k3-psil.h > new file mode 100644 > index 000000000000..16e9c8c6f839 > --- /dev/null > +++ b/include/linux/dma/k3-psil.h > @@ -0,0 +1,47 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + */ > + > +#ifndef K3_PSIL_H_ > +#define K3_PSIL_H_ > + > +#include <linux/types.h> > + > +#define K3_PSIL_DST_THREAD_ID_OFFSET 0x8000 > + > +struct device; > + > +/* Channel Throughput Levels */ > +enum udma_tp_level { > + UDMA_TP_NORMAL = 0, > + UDMA_TP_HIGH = 1, > + UDMA_TP_ULTRAHIGH = 2, > + UDMA_TP_LAST, > +}; > + > +enum psil_endpoint_type { > + PSIL_EP_NATIVE = 0, > + PSIL_EP_PDMA_XY, > + PSIL_EP_PDMA_MCAN, > + PSIL_EP_PDMA_AASRC, > +}; > + > +struct psil_endpoint_config { > + enum psil_endpoint_type ep_type; > + > + unsigned pkt_mode:1; > + unsigned notdpkt:1; > + unsigned needs_epib:1; > + u32 psd_size; > + enum udma_tp_level channel_tpl; > + > + /* PDMA properties, valid for PSIL_EP_PDMA_* */ > + unsigned pdma_acc32:1; > + unsigned pdma_burst:1; > +}; > + > +int psil_set_new_ep_config(struct device *dev, const char *name, > + struct psil_endpoint_config *ep_config); > + > +#endif /* K3_PSIL_H_ */ > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 05/11/2019 9.49, Tero Kristo wrote: > On 01/11/2019 10:41, Peter Ujfalusi wrote: >> In K3 architecture the DMA operates within threads. One end of the thread >> is UDMAP, the other is on the peripheral side. >> >> The UDMAP channel configuration depends on the needs of the remote >> endpoint and it can be differ from peripheral to peripheral. >> >> This patch adds database for am654 and j721e and small API to fetch the >> PSI-L endpoint configuration from the database which should only used by >> the DMA driver(s). >> >> Another API is added for native peripherals to give possibility to >> pass new >> configuration for the threads they are using, which is needed to be >> able to >> handle changes caused by different firmware loaded for the peripheral for >> example. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> drivers/dma/ti/Kconfig | 3 + >> drivers/dma/ti/Makefile | 1 + >> drivers/dma/ti/k3-psil-am654.c | 172 ++++++++++++++++++++++++++ >> drivers/dma/ti/k3-psil-j721e.c | 219 +++++++++++++++++++++++++++++++++ >> drivers/dma/ti/k3-psil-priv.h | 39 ++++++ >> drivers/dma/ti/k3-psil.c | 97 +++++++++++++++ >> include/linux/dma/k3-psil.h | 47 +++++++ >> 7 files changed, 578 insertions(+) >> create mode 100644 drivers/dma/ti/k3-psil-am654.c >> create mode 100644 drivers/dma/ti/k3-psil-j721e.c >> create mode 100644 drivers/dma/ti/k3-psil-priv.h >> create mode 100644 drivers/dma/ti/k3-psil.c >> create mode 100644 include/linux/dma/k3-psil.h ... >> diff --git a/drivers/dma/ti/k3-psil.c b/drivers/dma/ti/k3-psil.c >> new file mode 100644 >> index 000000000000..e610022f09f4 >> --- /dev/null >> +++ b/drivers/dma/ti/k3-psil.c >> @@ -0,0 +1,97 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2019 Texas Instruments Incorporated - >> http://www.ti.com >> + * Author: Peter Ujfalusi <peter.ujfalusi@ti.com> >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/device.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> +#include <linux/of.h> >> + >> +#include "k3-psil-priv.h" >> + >> +extern struct psil_ep_map am654_ep_map; >> +extern struct psil_ep_map j721e_ep_map; >> + >> +static DEFINE_MUTEX(ep_map_mutex); >> +static struct psil_ep_map *soc_ep_map; > > So, you are only protecting the high level soc_ep_map pointer only. You > don't need to protect the database itself via some usecounting or > something, or are you doing it within the DMA driver? That's correct, I protect only the soc_ep_map. The DMA drivers can look up threads concurrently I just need to make sure that the soc_ep_map is configured when the first psil_get_ep_config() comes. After this the DMA drivers are free to look up things. The ep_config update will be coming from the DMA client driver(s) and not from the DMA driver. The clinet driver knows how thier PSI-L endpoint if configured so they could update the default configuration _before_ they would request a DMA channel. > > -Tero > >> + >> +struct psil_endpoint_config *psil_get_ep_config(u32 thread_id) >> +{ >> + int i; >> + >> + mutex_lock(&ep_map_mutex); >> + if (!soc_ep_map) { >> + if (of_machine_is_compatible("ti,am654")) { >> + soc_ep_map = &am654_ep_map; >> + } else if (of_machine_is_compatible("ti,j721e")) { >> + soc_ep_map = &j721e_ep_map; >> + } else { >> + pr_err("PSIL: No compatible machine found for map\n"); >> + return ERR_PTR(-ENOTSUPP); >> + } >> + pr_debug("%s: Using map for %s\n", __func__, soc_ep_map->name); >> + } >> + mutex_unlock(&ep_map_mutex); >> + >> + if (thread_id & K3_PSIL_DST_THREAD_ID_OFFSET && soc_ep_map->dst) { >> + /* check in destination thread map */ >> + for (i = 0; i < soc_ep_map->dst_count; i++) { >> + if (soc_ep_map->dst[i].thread_id == thread_id) >> + return &soc_ep_map->dst[i].ep_config; >> + } >> + } >> + >> + thread_id &= ~K3_PSIL_DST_THREAD_ID_OFFSET; >> + if (soc_ep_map->src) { >> + for (i = 0; i < soc_ep_map->src_count; i++) { >> + if (soc_ep_map->src[i].thread_id == thread_id) >> + return &soc_ep_map->src[i].ep_config; >> + } >> + } >> + >> + return ERR_PTR(-ENOENT); >> +} >> +EXPORT_SYMBOL(psil_get_ep_config); >> + >> +int psil_set_new_ep_config(struct device *dev, const char *name, >> + struct psil_endpoint_config *ep_config) >> +{ >> + struct psil_endpoint_config *dst_ep_config; >> + struct of_phandle_args dma_spec; >> + u32 thread_id; >> + int index; >> + >> + if (!dev || !dev->of_node) >> + return -EINVAL; >> + >> + index = of_property_match_string(dev->of_node, "dma-names", name); >> + if (index < 0) >> + return index; >> + >> + if (of_parse_phandle_with_args(dev->of_node, "dmas", "#dma-cells", >> + index, &dma_spec)) >> + return -ENOENT; >> + >> + thread_id = dma_spec.args[0]; >> + >> + dst_ep_config = psil_get_ep_config(thread_id); >> + if (IS_ERR(dst_ep_config)) { >> + pr_err("PSIL: thread ID 0x%04x not defined in map\n", >> + thread_id); >> + of_node_put(dma_spec.np); >> + return PTR_ERR(dst_ep_config); >> + } >> + >> + memcpy(dst_ep_config, ep_config, sizeof(*dst_ep_config)); >> + >> + of_node_put(dma_spec.np); >> + return 0; >> +} >> +EXPORT_SYMBOL(psil_set_new_ep_config); >> + >> +MODULE_DESCRIPTION("TI K3 PSI-L endpoint database"); >> +MODULE_AUTHOR("Peter Ujfalusi <peter.ujfalusi@ti.com>"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/dma/k3-psil.h b/include/linux/dma/k3-psil.h >> new file mode 100644 >> index 000000000000..16e9c8c6f839 >> --- /dev/null >> +++ b/include/linux/dma/k3-psil.h >> @@ -0,0 +1,47 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (C) 2019 Texas Instruments Incorporated - >> http://www.ti.com >> + */ >> + >> +#ifndef K3_PSIL_H_ >> +#define K3_PSIL_H_ >> + >> +#include <linux/types.h> >> + >> +#define K3_PSIL_DST_THREAD_ID_OFFSET 0x8000 >> + >> +struct device; >> + >> +/* Channel Throughput Levels */ >> +enum udma_tp_level { >> + UDMA_TP_NORMAL = 0, >> + UDMA_TP_HIGH = 1, >> + UDMA_TP_ULTRAHIGH = 2, >> + UDMA_TP_LAST, >> +}; >> + >> +enum psil_endpoint_type { >> + PSIL_EP_NATIVE = 0, >> + PSIL_EP_PDMA_XY, >> + PSIL_EP_PDMA_MCAN, >> + PSIL_EP_PDMA_AASRC, >> +}; >> + >> +struct psil_endpoint_config { >> + enum psil_endpoint_type ep_type; >> + >> + unsigned pkt_mode:1; >> + unsigned notdpkt:1; >> + unsigned needs_epib:1; >> + u32 psd_size; >> + enum udma_tp_level channel_tpl; >> + >> + /* PDMA properties, valid for PSIL_EP_PDMA_* */ >> + unsigned pdma_acc32:1; >> + unsigned pdma_burst:1; >> +}; >> + >> +int psil_set_new_ep_config(struct device *dev, const char *name, >> + struct psil_endpoint_config *ep_config); >> + >> +#endif /* K3_PSIL_H_ */ >> > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi Peter, On 01/11/2019 10:41, Peter Ujfalusi wrote: > In K3 architecture the DMA operates within threads. One end of the thread > is UDMAP, the other is on the peripheral side. > > The UDMAP channel configuration depends on the needs of the remote > endpoint and it can be differ from peripheral to peripheral. > > This patch adds database for am654 and j721e and small API to fetch the > PSI-L endpoint configuration from the database which should only used by > the DMA driver(s). > > Another API is added for native peripherals to give possibility to pass new > configuration for the threads they are using, which is needed to be able to > handle changes caused by different firmware loaded for the peripheral for > example. I have no objection to this approach, but ... > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > drivers/dma/ti/Kconfig | 3 + > drivers/dma/ti/Makefile | 1 + > drivers/dma/ti/k3-psil-am654.c | 172 ++++++++++++++++++++++++++ > drivers/dma/ti/k3-psil-j721e.c | 219 +++++++++++++++++++++++++++++++++ > drivers/dma/ti/k3-psil-priv.h | 39 ++++++ > drivers/dma/ti/k3-psil.c | 97 +++++++++++++++ > include/linux/dma/k3-psil.h | 47 +++++++ > 7 files changed, 578 insertions(+) > create mode 100644 drivers/dma/ti/k3-psil-am654.c > create mode 100644 drivers/dma/ti/k3-psil-j721e.c > create mode 100644 drivers/dma/ti/k3-psil-priv.h > create mode 100644 drivers/dma/ti/k3-psil.c > create mode 100644 include/linux/dma/k3-psil.h > [...] > diff --git a/include/linux/dma/k3-psil.h b/include/linux/dma/k3-psil.h > new file mode 100644 > index 000000000000..16e9c8c6f839 > --- /dev/null > +++ b/include/linux/dma/k3-psil.h > @@ -0,0 +1,47 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + */ > + > +#ifndef K3_PSIL_H_ > +#define K3_PSIL_H_ > + > +#include <linux/types.h> > + > +#define K3_PSIL_DST_THREAD_ID_OFFSET 0x8000 > + > +struct device; > + > +/* Channel Throughput Levels */ > +enum udma_tp_level { > + UDMA_TP_NORMAL = 0, > + UDMA_TP_HIGH = 1, > + UDMA_TP_ULTRAHIGH = 2, > + UDMA_TP_LAST, > +}; > + > +enum psil_endpoint_type { > + PSIL_EP_NATIVE = 0, > + PSIL_EP_PDMA_XY, > + PSIL_EP_PDMA_MCAN, > + PSIL_EP_PDMA_AASRC, > +}; > + > +struct psil_endpoint_config { > + enum psil_endpoint_type ep_type; > + > + unsigned pkt_mode:1; > + unsigned notdpkt:1; > + unsigned needs_epib:1; > + u32 psd_size; > + enum udma_tp_level channel_tpl; > + > + /* PDMA properties, valid for PSIL_EP_PDMA_* */ > + unsigned pdma_acc32:1; > + unsigned pdma_burst:1; > +}; > + > +int psil_set_new_ep_config(struct device *dev, const char *name, > + struct psil_endpoint_config *ep_config); > + > +#endif /* K3_PSIL_H_ */ > I see no user now of this public interface, so I think it better to drop it until there will be real user of it. -- Best regards, grygorii
On 05/11/2019 12.00, Grygorii Strashko wrote: > Hi Peter, > > On 01/11/2019 10:41, Peter Ujfalusi wrote: >> In K3 architecture the DMA operates within threads. One end of the thread >> is UDMAP, the other is on the peripheral side. >> >> The UDMAP channel configuration depends on the needs of the remote >> endpoint and it can be differ from peripheral to peripheral. >> >> This patch adds database for am654 and j721e and small API to fetch the >> PSI-L endpoint configuration from the database which should only used by >> the DMA driver(s). >> >> Another API is added for native peripherals to give possibility to >> pass new >> configuration for the threads they are using, which is needed to be >> able to >> handle changes caused by different firmware loaded for the peripheral for >> example. > > I have no objection to this approach, but ... > >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> --- >> drivers/dma/ti/Kconfig | 3 + >> drivers/dma/ti/Makefile | 1 + >> drivers/dma/ti/k3-psil-am654.c | 172 ++++++++++++++++++++++++++ >> drivers/dma/ti/k3-psil-j721e.c | 219 +++++++++++++++++++++++++++++++++ >> drivers/dma/ti/k3-psil-priv.h | 39 ++++++ >> drivers/dma/ti/k3-psil.c | 97 +++++++++++++++ >> include/linux/dma/k3-psil.h | 47 +++++++ >> 7 files changed, 578 insertions(+) >> create mode 100644 drivers/dma/ti/k3-psil-am654.c >> create mode 100644 drivers/dma/ti/k3-psil-j721e.c >> create mode 100644 drivers/dma/ti/k3-psil-priv.h >> create mode 100644 drivers/dma/ti/k3-psil.c >> create mode 100644 include/linux/dma/k3-psil.h >> > > [...] > >> diff --git a/include/linux/dma/k3-psil.h b/include/linux/dma/k3-psil.h >> new file mode 100644 >> index 000000000000..16e9c8c6f839 >> --- /dev/null >> +++ b/include/linux/dma/k3-psil.h >> @@ -0,0 +1,47 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (C) 2019 Texas Instruments Incorporated - >> http://www.ti.com >> + */ >> + >> +#ifndef K3_PSIL_H_ >> +#define K3_PSIL_H_ >> + >> +#include <linux/types.h> >> + >> +#define K3_PSIL_DST_THREAD_ID_OFFSET 0x8000 >> + >> +struct device; >> + >> +/* Channel Throughput Levels */ >> +enum udma_tp_level { >> + UDMA_TP_NORMAL = 0, >> + UDMA_TP_HIGH = 1, >> + UDMA_TP_ULTRAHIGH = 2, >> + UDMA_TP_LAST, >> +}; >> + >> +enum psil_endpoint_type { >> + PSIL_EP_NATIVE = 0, >> + PSIL_EP_PDMA_XY, >> + PSIL_EP_PDMA_MCAN, >> + PSIL_EP_PDMA_AASRC, >> +}; >> + >> +struct psil_endpoint_config { >> + enum psil_endpoint_type ep_type; >> + >> + unsigned pkt_mode:1; >> + unsigned notdpkt:1; >> + unsigned needs_epib:1; >> + u32 psd_size; >> + enum udma_tp_level channel_tpl; >> + >> + /* PDMA properties, valid for PSIL_EP_PDMA_* */ >> + unsigned pdma_acc32:1; >> + unsigned pdma_burst:1; >> +}; >> + >> +int psil_set_new_ep_config(struct device *dev, const char *name, >> + struct psil_endpoint_config *ep_config); >> + >> +#endif /* K3_PSIL_H_ */ >> > > I see no user now of this public interface, so I think it better to drop > it until > there will be real user of it. The same argument is valid for the glue layer ;) This is only going to be used by native PSI-L devices and the psil_endpoint_config is going to be extended to facilitate their needs to give information to the DMA driver on how to set things up. I would rather avoid churn later on than adding the support from the start. The point is that the PSI-L endpoint configuration is part of the PSI-L peripheral and based on factors these configurations might differ from the default one. For example if we want to merge the two physical rx channel for sa2ul (so they use the same rflow) or other things we (I) can not foresee yet. Or if different firmware is loaded for them and it affects their PSI-L configuration. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 05/11/2019 12:27, Peter Ujfalusi wrote: > > > On 05/11/2019 12.00, Grygorii Strashko wrote: >> Hi Peter, >> >> On 01/11/2019 10:41, Peter Ujfalusi wrote: >>> In K3 architecture the DMA operates within threads. One end of the thread >>> is UDMAP, the other is on the peripheral side. >>> >>> The UDMAP channel configuration depends on the needs of the remote >>> endpoint and it can be differ from peripheral to peripheral. >>> >>> This patch adds database for am654 and j721e and small API to fetch the >>> PSI-L endpoint configuration from the database which should only used by >>> the DMA driver(s). >>> >>> Another API is added for native peripherals to give possibility to >>> pass new >>> configuration for the threads they are using, which is needed to be >>> able to >>> handle changes caused by different firmware loaded for the peripheral for >>> example. >> >> I have no objection to this approach, but ... >> >>> >>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >>> --- >>> drivers/dma/ti/Kconfig | 3 + >>> drivers/dma/ti/Makefile | 1 + >>> drivers/dma/ti/k3-psil-am654.c | 172 ++++++++++++++++++++++++++ >>> drivers/dma/ti/k3-psil-j721e.c | 219 +++++++++++++++++++++++++++++++++ >>> drivers/dma/ti/k3-psil-priv.h | 39 ++++++ >>> drivers/dma/ti/k3-psil.c | 97 +++++++++++++++ >>> include/linux/dma/k3-psil.h | 47 +++++++ >>> 7 files changed, 578 insertions(+) >>> create mode 100644 drivers/dma/ti/k3-psil-am654.c >>> create mode 100644 drivers/dma/ti/k3-psil-j721e.c >>> create mode 100644 drivers/dma/ti/k3-psil-priv.h >>> create mode 100644 drivers/dma/ti/k3-psil.c >>> create mode 100644 include/linux/dma/k3-psil.h >>> >> >> [...] >> >>> diff --git a/include/linux/dma/k3-psil.h b/include/linux/dma/k3-psil.h >>> new file mode 100644 >>> index 000000000000..16e9c8c6f839 >>> --- /dev/null >>> +++ b/include/linux/dma/k3-psil.h >>> @@ -0,0 +1,47 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Copyright (C) 2019 Texas Instruments Incorporated - >>> http://www.ti.com >>> + */ >>> + >>> +#ifndef K3_PSIL_H_ >>> +#define K3_PSIL_H_ >>> + >>> +#include <linux/types.h> >>> + >>> +#define K3_PSIL_DST_THREAD_ID_OFFSET 0x8000 >>> + >>> +struct device; >>> + >>> +/* Channel Throughput Levels */ >>> +enum udma_tp_level { >>> + UDMA_TP_NORMAL = 0, >>> + UDMA_TP_HIGH = 1, >>> + UDMA_TP_ULTRAHIGH = 2, >>> + UDMA_TP_LAST, >>> +}; >>> + >>> +enum psil_endpoint_type { >>> + PSIL_EP_NATIVE = 0, >>> + PSIL_EP_PDMA_XY, >>> + PSIL_EP_PDMA_MCAN, >>> + PSIL_EP_PDMA_AASRC, >>> +}; >>> + >>> +struct psil_endpoint_config { >>> + enum psil_endpoint_type ep_type; >>> + >>> + unsigned pkt_mode:1; >>> + unsigned notdpkt:1; >>> + unsigned needs_epib:1; >>> + u32 psd_size; >>> + enum udma_tp_level channel_tpl; >>> + >>> + /* PDMA properties, valid for PSIL_EP_PDMA_* */ >>> + unsigned pdma_acc32:1; >>> + unsigned pdma_burst:1; >>> +}; >>> + >>> +int psil_set_new_ep_config(struct device *dev, const char *name, >>> + struct psil_endpoint_config *ep_config); >>> + >>> +#endif /* K3_PSIL_H_ */ >>> >> >> I see no user now of this public interface, so I think it better to drop >> it until >> there will be real user of it. > > The same argument is valid for the glue layer ;) > > This is only going to be used by native PSI-L devices and the > psil_endpoint_config is going to be extended to facilitate their needs > to give information to the DMA driver on how to set things up. > > I would rather avoid churn later on than adding the support from the start. > > The point is that the PSI-L endpoint configuration is part of the PSI-L > peripheral and based on factors these configurations might differ from > the default one. For example if we want to merge the two physical rx > channel for sa2ul (so they use the same rflow) or other things we (I) > can not foresee yet. > Or if different firmware is loaded for them and it affects their PSI-L > configuration. Ok. It's up to you. otherwise: Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com> -- Best regards, grygorii
On 01-11-19, 10:41, Peter Ujfalusi wrote: > From: Grygorii Strashko <grygorii.strashko@ti.com> > > The Ring Accelerator (RINGACC or RA) provides hardware acceleration to > enable straightforward passing of work between a producer and a consumer. > There is one RINGACC module per NAVSS on TI AM65x and j721e. > > This patch introduces RINGACC device tree bindings. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/soc/ti/k3-ringacc.txt | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt > > diff --git a/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt b/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt > new file mode 100644 > index 000000000000..86954cf4fa99 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt > @@ -0,0 +1,59 @@ > +* Texas Instruments K3 NavigatorSS Ring Accelerator > + > +The Ring Accelerator (RA) is a machine which converts read/write accesses > +from/to a constant address into corresponding read/write accesses from/to a > +circular data structure in memory. The RA eliminates the need for each DMA > +controller which needs to access ring elements from having to know the current > +state of the ring (base address, current offset). The DMA controller > +performs a read or write access to a specific address range (which maps to the > +source interface on the RA) and the RA replaces the address for the transaction > +with a new address which corresponds to the head or tail element of the ring > +(head for reads, tail for writes). > + > +The Ring Accelerator is a hardware module that is responsible for accelerating > +management of the packet queues. The K3 SoCs can have more than one RA instances > + > +Required properties: > +- compatible : Must be "ti,am654-navss-ringacc"; > +- reg : Should contain register location and length of the following > + named register regions. > +- reg-names : should be > + "rt" - The RA Ring Real-time Control/Status Registers > + "fifos" - The RA Queues Registers > + "proxy_gcfg" - The RA Proxy Global Config Registers > + "proxy_target" - The RA Proxy Datapath Registers > +- ti,num-rings : Number of rings supported by RA > +- ti,sci-rm-range-gp-rings : TI-SCI RM subtype for GP ring range > +- ti,sci : phandle on TI-SCI compatible System controller node > +- ti,sci-dev-id : TI-SCI device id > +- msi-parent : phandle for "ti,sci-inta" interrupt controller > + > +Optional properties: > + -- ti,dma-ring-reset-quirk : enable ringacc / udma ring state interoperability > + issue software w/a > + > +Example: > + > +ringacc: ringacc@3c000000 { > + compatible = "ti,am654-navss-ringacc"; > + reg = <0x0 0x3c000000 0x0 0x400000>, > + <0x0 0x38000000 0x0 0x400000>, > + <0x0 0x31120000 0x0 0x100>, > + <0x0 0x33000000 0x0 0x40000>; > + reg-names = "rt", "fifos", > + "proxy_gcfg", "proxy_target"; > + ti,num-rings = <818>; > + ti,sci-rm-range-gp-rings = <0x2>; /* GP ring range */ > + ti,dma-ring-reset-quirk; > + ti,sci = <&dmsc>; > + ti,sci-dev-id = <187>; why do we need dev-id for? doesn't phandle the line above help? -- ~Vinod
On 01-11-19, 10:41, Peter Ujfalusi wrote: > A DMA hardware can have big cache or FIFO and the amount of data sitting in > the DMA fabric can be an interest for the clients. > > For example in audio we want to know the delay in the data flow and in case > the DMA have significantly large FIFO/cache, it can affect the latenc/delay > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > Reviewed-by: Tero Kristo <t-kristo@ti.com> > --- > drivers/dma/dmaengine.h | 8 ++++++++ > include/linux/dmaengine.h | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h > index 501c0b063f85..b0b97475707a 100644 > --- a/drivers/dma/dmaengine.h > +++ b/drivers/dma/dmaengine.h > @@ -77,6 +77,7 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan, > state->last = complete; > state->used = used; > state->residue = 0; > + state->in_flight_bytes = 0; > } > return dma_async_is_complete(cookie, complete, used); > } > @@ -87,6 +88,13 @@ static inline void dma_set_residue(struct dma_tx_state *state, u32 residue) > state->residue = residue; > } > > +static inline void dma_set_in_flight_bytes(struct dma_tx_state *state, > + u32 in_flight_bytes) > +{ > + if (state) > + state->in_flight_bytes = in_flight_bytes; > +} > + > struct dmaengine_desc_callback { > dma_async_tx_callback callback; > dma_async_tx_callback_result callback_result; > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 0e8b426bbde9..c4c5219030a6 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -682,11 +682,13 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr > * @residue: the remaining number of bytes left to transmit > * on the selected transfer for states DMA_IN_PROGRESS and > * DMA_PAUSED if this is implemented in the driver, else 0 > + * @in_flight_bytes: amount of data in bytes cached by the DMA. > */ > struct dma_tx_state { > dma_cookie_t last; > dma_cookie_t used; > u32 residue; > + u32 in_flight_bytes; Should we add this here or use the dmaengine_result() -- ~Vinod
On 01-11-19, 10:41, Peter Ujfalusi wrote: > Split patch for review containing: channel rsource allocation and free s/rsource/resource > +static int udma_tisci_tx_channel_config(struct udma_chan *uc) > +{ > + struct udma_dev *ud = uc->ud; > + struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; > + const struct ti_sci_rm_udmap_ops *tisci_ops = tisci_rm->tisci_udmap_ops; > + struct udma_tchan *tchan = uc->tchan; > + int tc_ring = k3_ringacc_get_ring_id(tchan->tc_ring); > + struct ti_sci_msg_rm_udmap_tx_ch_cfg req_tx = { 0 }; > + u32 mode, fetch_size; > + int ret = 0; > + > + if (uc->pkt_mode) { > + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_PKT_PBRR; > + fetch_size = cppi5_hdesc_calc_size(uc->needs_epib, uc->psd_size, > + 0); > + } else { > + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_3RDP_PBRR; > + fetch_size = sizeof(struct cppi5_desc_hdr_t); > + } > + > + req_tx.valid_params = > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_PAUSE_ON_ERR_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_FILT_EINFO_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_FILT_PSWORDS_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CHAN_TYPE_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_SUPR_TDPKT_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_FETCH_SIZE_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CQ_QNUM_VALID; bunch of these are repeat, you can define a COMMON_VALID_PARAMS and use that + specific ones.. > + > + req_tx.nav_id = tisci_rm->tisci_dev_id; > + req_tx.index = tchan->id; > + req_tx.tx_pause_on_err = 0; > + req_tx.tx_filt_einfo = 0; > + req_tx.tx_filt_pswords = 0; i think initialization to 0 is superfluous > + req_tx.tx_chan_type = mode; > + req_tx.tx_supr_tdpkt = uc->notdpkt; > + req_tx.tx_fetch_size = fetch_size >> 2; > + req_tx.txcq_qnum = tc_ring; > + if (uc->ep_type == PSIL_EP_PDMA_XY) { > + /* wait for peer to complete the teardown for PDMAs */ > + req_tx.valid_params |= > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_TDTYPE_VALID; > + req_tx.tx_tdtype = 1; > + } > + > + ret = tisci_ops->tx_ch_cfg(tisci_rm->tisci, &req_tx); > + if (ret) > + dev_err(ud->dev, "tchan%d cfg failed %d\n", tchan->id, ret); > + > + return ret; > +} > + > +static int udma_tisci_rx_channel_config(struct udma_chan *uc) > +{ > + struct udma_dev *ud = uc->ud; > + struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; > + const struct ti_sci_rm_udmap_ops *tisci_ops = tisci_rm->tisci_udmap_ops; > + struct udma_rchan *rchan = uc->rchan; > + int fd_ring = k3_ringacc_get_ring_id(rchan->fd_ring); > + int rx_ring = k3_ringacc_get_ring_id(rchan->r_ring); > + struct ti_sci_msg_rm_udmap_rx_ch_cfg req_rx = { 0 }; > + struct ti_sci_msg_rm_udmap_flow_cfg flow_req = { 0 }; > + u32 mode, fetch_size; > + int ret = 0; > + > + if (uc->pkt_mode) { > + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_PKT_PBRR; > + fetch_size = cppi5_hdesc_calc_size(uc->needs_epib, > + uc->psd_size, 0); > + } else { > + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_3RDP_PBRR; > + fetch_size = sizeof(struct cppi5_desc_hdr_t); > + } > + > + req_rx.valid_params = > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_PAUSE_ON_ERR_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_FETCH_SIZE_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CQ_QNUM_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CHAN_TYPE_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_IGNORE_SHORT_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_IGNORE_LONG_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_FLOWID_START_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_FLOWID_CNT_VALID; > + > + req_rx.nav_id = tisci_rm->tisci_dev_id; > + req_rx.index = rchan->id; > + req_rx.rx_fetch_size = fetch_size >> 2; > + req_rx.rxcq_qnum = rx_ring; > + req_rx.rx_pause_on_err = 0; > + req_rx.rx_chan_type = mode; > + req_rx.rx_ignore_short = 0; > + req_rx.rx_ignore_long = 0; > + req_rx.flowid_start = 0; > + req_rx.flowid_cnt = 0; > + > + ret = tisci_ops->rx_ch_cfg(tisci_rm->tisci, &req_rx); > + if (ret) { > + dev_err(ud->dev, "rchan%d cfg failed %d\n", rchan->id, ret); > + return ret; > + } > + > + flow_req.valid_params = > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_EINFO_PRESENT_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_PSINFO_PRESENT_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_ERROR_HANDLING_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DESC_TYPE_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_QNUM_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_SRC_TAG_HI_SEL_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_SRC_TAG_LO_SEL_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_TAG_HI_SEL_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_TAG_LO_SEL_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ0_SZ0_QNUM_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ1_QNUM_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ2_QNUM_VALID | > + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ3_QNUM_VALID; > + > + flow_req.nav_id = tisci_rm->tisci_dev_id; > + flow_req.flow_index = rchan->id; > + > + if (uc->needs_epib) > + flow_req.rx_einfo_present = 1; > + else > + flow_req.rx_einfo_present = 0; > + if (uc->psd_size) > + flow_req.rx_psinfo_present = 1; > + else > + flow_req.rx_psinfo_present = 0; > + flow_req.rx_error_handling = 1; > + flow_req.rx_desc_type = 0; > + flow_req.rx_dest_qnum = rx_ring; > + flow_req.rx_src_tag_hi_sel = 2; > + flow_req.rx_src_tag_lo_sel = 4; > + flow_req.rx_dest_tag_hi_sel = 5; > + flow_req.rx_dest_tag_lo_sel = 4; can we get rid of magic numbers here and elsewhere, or at least comment on what these mean.. > +static int udma_alloc_chan_resources(struct dma_chan *chan) > +{ > + struct udma_chan *uc = to_udma_chan(chan); > + struct udma_dev *ud = to_udma_dev(chan->device); > + const struct udma_match_data *match_data = ud->match_data; > + struct k3_ring *irq_ring; > + u32 irq_udma_idx; > + int ret; > + > + if (uc->pkt_mode || uc->dir == DMA_MEM_TO_MEM) { > + uc->use_dma_pool = true; > + /* in case of MEM_TO_MEM we have maximum of two TRs */ > + if (uc->dir == DMA_MEM_TO_MEM) { > + uc->hdesc_size = cppi5_trdesc_calc_size( > + sizeof(struct cppi5_tr_type15_t), 2); > + uc->pkt_mode = false; > + } > + } > + > + if (uc->use_dma_pool) { > + uc->hdesc_pool = dma_pool_create(uc->name, ud->ddev.dev, > + uc->hdesc_size, ud->desc_align, > + 0); > + if (!uc->hdesc_pool) { > + dev_err(ud->ddev.dev, > + "Descriptor pool allocation failed\n"); > + uc->use_dma_pool = false; > + return -ENOMEM; > + } > + } > + > + /* > + * Make sure that the completion is in a known state: > + * No teardown, the channel is idle > + */ > + reinit_completion(&uc->teardown_completed); > + complete_all(&uc->teardown_completed); should we not complete first and then do reinit to bring a clean state? > + uc->state = UDMA_CHAN_IS_IDLE; > + > + switch (uc->dir) { > + case DMA_MEM_TO_MEM: can you explain why a allocation should be channel dependent, shouldn't these things be done in prep_ calls? I looked ahead and checked the prep_ calls and we can use any direction so this somehow doesn't make sense! -- ~Vinod
On 01-11-19, 10:41, Peter Ujfalusi wrote: > +/* Not much yet */ ? > +static enum dma_status udma_tx_status(struct dma_chan *chan, > + dma_cookie_t cookie, > + struct dma_tx_state *txstate) > +{ > + struct udma_chan *uc = to_udma_chan(chan); > + enum dma_status ret; > + unsigned long flags; > + > + spin_lock_irqsave(&uc->vc.lock, flags); > + > + ret = dma_cookie_status(chan, cookie, txstate); > + > + if (!udma_is_chan_running(uc)) > + ret = DMA_COMPLETE; so a paused channel will result in dma complete status? -- ~Vinod
On 01-11-19, 10:41, Peter Ujfalusi wrote: > From: Grygorii Strashko <grygorii.strashko@ti.com> > > Certain users can not use right now the DMAengine API due to missing > features in the core. Prime example is Networking. > > These users can use the glue layer interface to avoid misuse of DMAengine > API and when the core gains the needed features they can be converted to > use generic API. Can you add some notes on what all features does this layer implement.. -- ~Vinod
On 11/11/2019 6.07, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: >> From: Grygorii Strashko <grygorii.strashko@ti.com> >> >> The Ring Accelerator (RINGACC or RA) provides hardware acceleration to >> enable straightforward passing of work between a producer and a consumer. >> There is one RINGACC module per NAVSS on TI AM65x and j721e. >> >> This patch introduces RINGACC device tree bindings. >> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> Reviewed-by: Rob Herring <robh@kernel.org> >> --- >> .../devicetree/bindings/soc/ti/k3-ringacc.txt | 59 +++++++++++++++++++ >> 1 file changed, 59 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt >> >> diff --git a/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt b/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt >> new file mode 100644 >> index 000000000000..86954cf4fa99 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soc/ti/k3-ringacc.txt >> @@ -0,0 +1,59 @@ >> +* Texas Instruments K3 NavigatorSS Ring Accelerator >> + >> +The Ring Accelerator (RA) is a machine which converts read/write accesses >> +from/to a constant address into corresponding read/write accesses from/to a >> +circular data structure in memory. The RA eliminates the need for each DMA >> +controller which needs to access ring elements from having to know the current >> +state of the ring (base address, current offset). The DMA controller >> +performs a read or write access to a specific address range (which maps to the >> +source interface on the RA) and the RA replaces the address for the transaction >> +with a new address which corresponds to the head or tail element of the ring >> +(head for reads, tail for writes). >> + >> +The Ring Accelerator is a hardware module that is responsible for accelerating >> +management of the packet queues. The K3 SoCs can have more than one RA instances >> + >> +Required properties: >> +- compatible : Must be "ti,am654-navss-ringacc"; >> +- reg : Should contain register location and length of the following >> + named register regions. >> +- reg-names : should be >> + "rt" - The RA Ring Real-time Control/Status Registers >> + "fifos" - The RA Queues Registers >> + "proxy_gcfg" - The RA Proxy Global Config Registers >> + "proxy_target" - The RA Proxy Datapath Registers >> +- ti,num-rings : Number of rings supported by RA >> +- ti,sci-rm-range-gp-rings : TI-SCI RM subtype for GP ring range >> +- ti,sci : phandle on TI-SCI compatible System controller node >> +- ti,sci-dev-id : TI-SCI device id >> +- msi-parent : phandle for "ti,sci-inta" interrupt controller >> + >> +Optional properties: >> + -- ti,dma-ring-reset-quirk : enable ringacc / udma ring state interoperability >> + issue software w/a >> + >> +Example: >> + >> +ringacc: ringacc@3c000000 { >> + compatible = "ti,am654-navss-ringacc"; >> + reg = <0x0 0x3c000000 0x0 0x400000>, >> + <0x0 0x38000000 0x0 0x400000>, >> + <0x0 0x31120000 0x0 0x100>, >> + <0x0 0x33000000 0x0 0x40000>; >> + reg-names = "rt", "fifos", >> + "proxy_gcfg", "proxy_target"; >> + ti,num-rings = <818>; >> + ti,sci-rm-range-gp-rings = <0x2>; /* GP ring range */ >> + ti,dma-ring-reset-quirk; >> + ti,sci = <&dmsc>; >> + ti,sci-dev-id = <187>; > > why do we need dev-id for? doesn't phandle the line above help? the ti,sci-dev-id is the device ID of the ring accelerator which is needed for the resource management implemented in sysfw. This is based on how the ti,sci-inta binding has defined it: Documentation/devicetree/bindings/interrupt-controller/ti,sci-inta.txt I'll update the document to make it clear. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/11/2019 6.39, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: >> A DMA hardware can have big cache or FIFO and the amount of data sitting in >> the DMA fabric can be an interest for the clients. >> >> For example in audio we want to know the delay in the data flow and in case >> the DMA have significantly large FIFO/cache, it can affect the latenc/delay >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> Reviewed-by: Tero Kristo <t-kristo@ti.com> >> --- >> drivers/dma/dmaengine.h | 8 ++++++++ >> include/linux/dmaengine.h | 2 ++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h >> index 501c0b063f85..b0b97475707a 100644 >> --- a/drivers/dma/dmaengine.h >> +++ b/drivers/dma/dmaengine.h >> @@ -77,6 +77,7 @@ static inline enum dma_status dma_cookie_status(struct dma_chan *chan, >> state->last = complete; >> state->used = used; >> state->residue = 0; >> + state->in_flight_bytes = 0; >> } >> return dma_async_is_complete(cookie, complete, used); >> } >> @@ -87,6 +88,13 @@ static inline void dma_set_residue(struct dma_tx_state *state, u32 residue) >> state->residue = residue; >> } >> >> +static inline void dma_set_in_flight_bytes(struct dma_tx_state *state, >> + u32 in_flight_bytes) >> +{ >> + if (state) >> + state->in_flight_bytes = in_flight_bytes; >> +} >> + >> struct dmaengine_desc_callback { >> dma_async_tx_callback callback; >> dma_async_tx_callback_result callback_result; >> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h >> index 0e8b426bbde9..c4c5219030a6 100644 >> --- a/include/linux/dmaengine.h >> +++ b/include/linux/dmaengine.h >> @@ -682,11 +682,13 @@ static inline struct dma_async_tx_descriptor *txd_next(struct dma_async_tx_descr >> * @residue: the remaining number of bytes left to transmit >> * on the selected transfer for states DMA_IN_PROGRESS and >> * DMA_PAUSED if this is implemented in the driver, else 0 >> + * @in_flight_bytes: amount of data in bytes cached by the DMA. >> */ >> struct dma_tx_state { >> dma_cookie_t last; >> dma_cookie_t used; >> u32 residue; >> + u32 in_flight_bytes; > > Should we add this here or use the dmaengine_result() Ideally at the time dmaengine_result is used (at tx completion callback) there should be nothing in flight ;) The reason why it is added to dma_tx_state is that clients can check at any time while the DMA is running the number of cached bytes. Audio needs this for cyclic and UART also needs to know it. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/11/2019 6.47, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: > >> --- /dev/null >> +++ b/drivers/dma/ti/k3-psil.c >> @@ -0,0 +1,97 @@ >> +// SPDX-License-Identifier: GPL-2.0 > > ... > >> +extern struct psil_ep_map am654_ep_map; >> +extern struct psil_ep_map j721e_ep_map; >> + >> +static DEFINE_MUTEX(ep_map_mutex); >> +static struct psil_ep_map *soc_ep_map; >> + >> +struct psil_endpoint_config *psil_get_ep_config(u32 thread_id) >> +{ >> + int i; >> + >> + mutex_lock(&ep_map_mutex); >> + if (!soc_ep_map) { >> + if (of_machine_is_compatible("ti,am654")) { >> + soc_ep_map = &am654_ep_map; >> + } else if (of_machine_is_compatible("ti,j721e")) { >> + soc_ep_map = &j721e_ep_map; >> + } else { >> + pr_err("PSIL: No compatible machine found for map\n"); >> + return ERR_PTR(-ENOTSUPP); >> + } >> + pr_debug("%s: Using map for %s\n", __func__, soc_ep_map->name); >> + } >> + mutex_unlock(&ep_map_mutex); >> + >> + if (thread_id & K3_PSIL_DST_THREAD_ID_OFFSET && soc_ep_map->dst) { >> + /* check in destination thread map */ >> + for (i = 0; i < soc_ep_map->dst_count; i++) { >> + if (soc_ep_map->dst[i].thread_id == thread_id) >> + return &soc_ep_map->dst[i].ep_config; >> + } >> + } >> + >> + thread_id &= ~K3_PSIL_DST_THREAD_ID_OFFSET; >> + if (soc_ep_map->src) { >> + for (i = 0; i < soc_ep_map->src_count; i++) { >> + if (soc_ep_map->src[i].thread_id == thread_id) >> + return &soc_ep_map->src[i].ep_config; >> + } >> + } >> + >> + return ERR_PTR(-ENOENT); >> +} >> +EXPORT_SYMBOL(psil_get_ep_config); > > This doesn't match the license of this module, we need it to be > EXPORT_SYMBOL_GPL Oops, will fix it. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/11/2019 8.06, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: >> Split patch for review containing: channel rsource allocation and free > > s/rsource/resource I'll try to remember to fix up this temporally commit message, at the end these split patches are going to be squashed into one commit when things are ready to be applied. >> +static int udma_tisci_tx_channel_config(struct udma_chan *uc) >> +{ >> + struct udma_dev *ud = uc->ud; >> + struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; >> + const struct ti_sci_rm_udmap_ops *tisci_ops = tisci_rm->tisci_udmap_ops; >> + struct udma_tchan *tchan = uc->tchan; >> + int tc_ring = k3_ringacc_get_ring_id(tchan->tc_ring); >> + struct ti_sci_msg_rm_udmap_tx_ch_cfg req_tx = { 0 }; >> + u32 mode, fetch_size; >> + int ret = 0; >> + >> + if (uc->pkt_mode) { >> + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_PKT_PBRR; >> + fetch_size = cppi5_hdesc_calc_size(uc->needs_epib, uc->psd_size, >> + 0); >> + } else { >> + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_3RDP_PBRR; >> + fetch_size = sizeof(struct cppi5_desc_hdr_t); >> + } >> + >> + req_tx.valid_params = >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_PAUSE_ON_ERR_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_FILT_EINFO_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_FILT_PSWORDS_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CHAN_TYPE_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_SUPR_TDPKT_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_FETCH_SIZE_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CQ_QNUM_VALID; > > bunch of these are repeat, you can define a COMMON_VALID_PARAMS and use > that + specific ones.. OK, I'll try to sanitize these a bit. >> + >> + req_tx.nav_id = tisci_rm->tisci_dev_id; >> + req_tx.index = tchan->id; >> + req_tx.tx_pause_on_err = 0; >> + req_tx.tx_filt_einfo = 0; >> + req_tx.tx_filt_pswords = 0; > > i think initialization to 0 is superfluous Indeed, I'll remove these. >> + req_tx.tx_chan_type = mode; >> + req_tx.tx_supr_tdpkt = uc->notdpkt; >> + req_tx.tx_fetch_size = fetch_size >> 2; >> + req_tx.txcq_qnum = tc_ring; >> + if (uc->ep_type == PSIL_EP_PDMA_XY) { >> + /* wait for peer to complete the teardown for PDMAs */ >> + req_tx.valid_params |= >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_TX_TDTYPE_VALID; >> + req_tx.tx_tdtype = 1; >> + } >> + >> + ret = tisci_ops->tx_ch_cfg(tisci_rm->tisci, &req_tx); >> + if (ret) >> + dev_err(ud->dev, "tchan%d cfg failed %d\n", tchan->id, ret); >> + >> + return ret; >> +} >> + >> +static int udma_tisci_rx_channel_config(struct udma_chan *uc) >> +{ >> + struct udma_dev *ud = uc->ud; >> + struct udma_tisci_rm *tisci_rm = &ud->tisci_rm; >> + const struct ti_sci_rm_udmap_ops *tisci_ops = tisci_rm->tisci_udmap_ops; >> + struct udma_rchan *rchan = uc->rchan; >> + int fd_ring = k3_ringacc_get_ring_id(rchan->fd_ring); >> + int rx_ring = k3_ringacc_get_ring_id(rchan->r_ring); >> + struct ti_sci_msg_rm_udmap_rx_ch_cfg req_rx = { 0 }; >> + struct ti_sci_msg_rm_udmap_flow_cfg flow_req = { 0 }; >> + u32 mode, fetch_size; >> + int ret = 0; >> + >> + if (uc->pkt_mode) { >> + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_PKT_PBRR; >> + fetch_size = cppi5_hdesc_calc_size(uc->needs_epib, >> + uc->psd_size, 0); >> + } else { >> + mode = TI_SCI_RM_UDMAP_CHAN_TYPE_3RDP_PBRR; >> + fetch_size = sizeof(struct cppi5_desc_hdr_t); >> + } >> + >> + req_rx.valid_params = >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_PAUSE_ON_ERR_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_FETCH_SIZE_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CQ_QNUM_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_CHAN_TYPE_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_IGNORE_SHORT_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_IGNORE_LONG_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_FLOWID_START_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_CH_RX_FLOWID_CNT_VALID; >> + >> + req_rx.nav_id = tisci_rm->tisci_dev_id; >> + req_rx.index = rchan->id; >> + req_rx.rx_fetch_size = fetch_size >> 2; >> + req_rx.rxcq_qnum = rx_ring; >> + req_rx.rx_pause_on_err = 0; >> + req_rx.rx_chan_type = mode; >> + req_rx.rx_ignore_short = 0; >> + req_rx.rx_ignore_long = 0; >> + req_rx.flowid_start = 0; >> + req_rx.flowid_cnt = 0; >> + >> + ret = tisci_ops->rx_ch_cfg(tisci_rm->tisci, &req_rx); >> + if (ret) { >> + dev_err(ud->dev, "rchan%d cfg failed %d\n", rchan->id, ret); >> + return ret; >> + } >> + >> + flow_req.valid_params = >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_EINFO_PRESENT_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_PSINFO_PRESENT_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_ERROR_HANDLING_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DESC_TYPE_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_QNUM_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_SRC_TAG_HI_SEL_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_SRC_TAG_LO_SEL_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_TAG_HI_SEL_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_DEST_TAG_LO_SEL_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ0_SZ0_QNUM_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ1_QNUM_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ2_QNUM_VALID | >> + TI_SCI_MSG_VALUE_RM_UDMAP_FLOW_FDQ3_QNUM_VALID; >> + >> + flow_req.nav_id = tisci_rm->tisci_dev_id; >> + flow_req.flow_index = rchan->id; >> + >> + if (uc->needs_epib) >> + flow_req.rx_einfo_present = 1; >> + else >> + flow_req.rx_einfo_present = 0; >> + if (uc->psd_size) >> + flow_req.rx_psinfo_present = 1; >> + else >> + flow_req.rx_psinfo_present = 0; >> + flow_req.rx_error_handling = 1; >> + flow_req.rx_desc_type = 0; >> + flow_req.rx_dest_qnum = rx_ring; >> + flow_req.rx_src_tag_hi_sel = 2; >> + flow_req.rx_src_tag_lo_sel = 4; >> + flow_req.rx_dest_tag_hi_sel = 5; >> + flow_req.rx_dest_tag_lo_sel = 4; > > can we get rid of magic numbers here and elsewhere, or at least comment > on what these mean.. True, I'll clean it up. >> +static int udma_alloc_chan_resources(struct dma_chan *chan) >> +{ >> + struct udma_chan *uc = to_udma_chan(chan); >> + struct udma_dev *ud = to_udma_dev(chan->device); >> + const struct udma_match_data *match_data = ud->match_data; >> + struct k3_ring *irq_ring; >> + u32 irq_udma_idx; >> + int ret; >> + >> + if (uc->pkt_mode || uc->dir == DMA_MEM_TO_MEM) { >> + uc->use_dma_pool = true; >> + /* in case of MEM_TO_MEM we have maximum of two TRs */ >> + if (uc->dir == DMA_MEM_TO_MEM) { >> + uc->hdesc_size = cppi5_trdesc_calc_size( >> + sizeof(struct cppi5_tr_type15_t), 2); >> + uc->pkt_mode = false; >> + } >> + } >> + >> + if (uc->use_dma_pool) { >> + uc->hdesc_pool = dma_pool_create(uc->name, ud->ddev.dev, >> + uc->hdesc_size, ud->desc_align, >> + 0); >> + if (!uc->hdesc_pool) { >> + dev_err(ud->ddev.dev, >> + "Descriptor pool allocation failed\n"); >> + uc->use_dma_pool = false; >> + return -ENOMEM; >> + } >> + } >> + >> + /* >> + * Make sure that the completion is in a known state: >> + * No teardown, the channel is idle >> + */ >> + reinit_completion(&uc->teardown_completed); >> + complete_all(&uc->teardown_completed); > > should we not complete first and then do reinit to bring a clean state? The reason why it is like this is that the udma_synchronize() is checking the completion and if the client requested the channel and calls terminate_all_sync() without any transfer then no one will mark the completion completed. >> + uc->state = UDMA_CHAN_IS_IDLE; >> + >> + switch (uc->dir) { >> + case DMA_MEM_TO_MEM: > > can you explain why a allocation should be channel dependent, shouldn't > these things be done in prep_ calls? A channel can not change direction, it is either MEM_TO_DEV, DEV_TO_MEM or MEM_TO_MEM and it is set when the channel is requested. > I looked ahead and checked the prep_ calls and we can use any direction > so this somehow doesn't make sense! I'm checking in the prep callbacks if the requested direction is matching with the channel direction. I just can not change the channel direction runtime. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/11/2019 8.09, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: > >> +/* Not much yet */ > > ? Forgot to remove it when I did implemented the tx_status() ;) > >> +static enum dma_status udma_tx_status(struct dma_chan *chan, >> + dma_cookie_t cookie, >> + struct dma_tx_state *txstate) >> +{ >> + struct udma_chan *uc = to_udma_chan(chan); >> + enum dma_status ret; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&uc->vc.lock, flags); >> + >> + ret = dma_cookie_status(chan, cookie, txstate); >> + >> + if (!udma_is_chan_running(uc)) >> + ret = DMA_COMPLETE; > > so a paused channel will result in dma complete status? The channel is still enabled (running), the pause only sets a bit in the channel's real time control register. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 11/11/2019 8.12, Vinod Koul wrote: > On 01-11-19, 10:41, Peter Ujfalusi wrote: >> From: Grygorii Strashko <grygorii.strashko@ti.com> >> >> Certain users can not use right now the DMAengine API due to missing >> features in the core. Prime example is Networking. >> >> These users can use the glue layer interface to avoid misuse of DMAengine >> API and when the core gains the needed features they can be converted to >> use generic API. > > Can you add some notes on what all features does this layer implement.. In the commit message or in the code? - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 12/11/2019 7.37, Vinod Koul wrote: > On 11-11-19, 12:31, Peter Ujfalusi wrote: >> >> >> On 11/11/2019 8.12, Vinod Koul wrote: >>> On 01-11-19, 10:41, Peter Ujfalusi wrote: >>>> From: Grygorii Strashko <grygorii.strashko@ti.com> >>>> >>>> Certain users can not use right now the DMAengine API due to missing >>>> features in the core. Prime example is Networking. >>>> >>>> These users can use the glue layer interface to avoid misuse of DMAengine >>>> API and when the core gains the needed features they can be converted to >>>> use generic API. >>> >>> Can you add some notes on what all features does this layer implement.. >> >> In the commit message or in the code? > > commit here so that we know what to expect. Can you check the v5 commit message if it is sufficient? If not, I can make it more verbose for v6. - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki