2020-09-14 14:11:15

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 0/7] dmaengine: add support for sama7g5 based at_xdmac

This series adds support for sama7g5-based at_xdmac.

In sama7g5, the XDMAC is present in a slightly modified form. This series
tries to adapt the existing driver to the differences.
According to the compatible string, we select one of two possible
layouts, which hold the differences in registermap and supported features.

Thanks!

Eugen Hristev (7):
dmaengine: at_xdmac: separate register defines into header file
MAINTAINERS: add dma/at_xdmac_regs.h to XDMAC driver entry
dt-bindings: dmaengine: at_xdmac: add compatible with
microchip,sama7g5
dmaengine: at_xdmac: adapt perid for mem2mem operations
dmaengine: at_xdmac: add support for sama7g5 based at_xdmac
dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property
dmaengine: at_xdmac: add AXI priority support and recommended settings

.../devicetree/bindings/dma/atmel-xdma.txt | 9 +-
MAINTAINERS | 1 +
drivers/dma/at_xdmac.c | 273 +++++++-----------
drivers/dma/at_xdmac_regs.h | 161 +++++++++++
4 files changed, 281 insertions(+), 163 deletions(-)
create mode 100644 drivers/dma/at_xdmac_regs.h

--
2.25.1


2020-09-14 14:11:46

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 1/7] dmaengine: at_xdmac: separate register defines into header file

Separate register defines into header file.
This is required to support a slightly different version of the register
map in new hardware versions of the XDMAC.

Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/dma/at_xdmac.c | 143 +--------------------------------
drivers/dma/at_xdmac_regs.h | 154 ++++++++++++++++++++++++++++++++++++
2 files changed, 155 insertions(+), 142 deletions(-)
create mode 100644 drivers/dma/at_xdmac_regs.h

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index fd92f048c491..fab19e00a7be 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -24,148 +24,7 @@

#include "dmaengine.h"

-/* Global registers */
-#define AT_XDMAC_GTYPE 0x00 /* Global Type Register */
-#define AT_XDMAC_NB_CH(i) (((i) & 0x1F) + 1) /* Number of Channels Minus One */
-#define AT_XDMAC_FIFO_SZ(i) (((i) >> 5) & 0x7FF) /* Number of Bytes */
-#define AT_XDMAC_NB_REQ(i) ((((i) >> 16) & 0x3F) + 1) /* Number of Peripheral Requests Minus One */
-#define AT_XDMAC_GCFG 0x04 /* Global Configuration Register */
-#define AT_XDMAC_GWAC 0x08 /* Global Weighted Arbiter Configuration Register */
-#define AT_XDMAC_GIE 0x0C /* Global Interrupt Enable Register */
-#define AT_XDMAC_GID 0x10 /* Global Interrupt Disable Register */
-#define AT_XDMAC_GIM 0x14 /* Global Interrupt Mask Register */
-#define AT_XDMAC_GIS 0x18 /* Global Interrupt Status Register */
-#define AT_XDMAC_GE 0x1C /* Global Channel Enable Register */
-#define AT_XDMAC_GD 0x20 /* Global Channel Disable Register */
-#define AT_XDMAC_GS 0x24 /* Global Channel Status Register */
-#define AT_XDMAC_GRS 0x28 /* Global Channel Read Suspend Register */
-#define AT_XDMAC_GWS 0x2C /* Global Write Suspend Register */
-#define AT_XDMAC_GRWS 0x30 /* Global Channel Read Write Suspend Register */
-#define AT_XDMAC_GRWR 0x34 /* Global Channel Read Write Resume Register */
-#define AT_XDMAC_GSWR 0x38 /* Global Channel Software Request Register */
-#define AT_XDMAC_GSWS 0x3C /* Global channel Software Request Status Register */
-#define AT_XDMAC_GSWF 0x40 /* Global Channel Software Flush Request Register */
-#define AT_XDMAC_VERSION 0xFFC /* XDMAC Version Register */
-
-/* Channel relative registers offsets */
-#define AT_XDMAC_CIE 0x00 /* Channel Interrupt Enable Register */
-#define AT_XDMAC_CIE_BIE BIT(0) /* End of Block Interrupt Enable Bit */
-#define AT_XDMAC_CIE_LIE BIT(1) /* End of Linked List Interrupt Enable Bit */
-#define AT_XDMAC_CIE_DIE BIT(2) /* End of Disable Interrupt Enable Bit */
-#define AT_XDMAC_CIE_FIE BIT(3) /* End of Flush Interrupt Enable Bit */
-#define AT_XDMAC_CIE_RBEIE BIT(4) /* Read Bus Error Interrupt Enable Bit */
-#define AT_XDMAC_CIE_WBEIE BIT(5) /* Write Bus Error Interrupt Enable Bit */
-#define AT_XDMAC_CIE_ROIE BIT(6) /* Request Overflow Interrupt Enable Bit */
-#define AT_XDMAC_CID 0x04 /* Channel Interrupt Disable Register */
-#define AT_XDMAC_CID_BID BIT(0) /* End of Block Interrupt Disable Bit */
-#define AT_XDMAC_CID_LID BIT(1) /* End of Linked List Interrupt Disable Bit */
-#define AT_XDMAC_CID_DID BIT(2) /* End of Disable Interrupt Disable Bit */
-#define AT_XDMAC_CID_FID BIT(3) /* End of Flush Interrupt Disable Bit */
-#define AT_XDMAC_CID_RBEID BIT(4) /* Read Bus Error Interrupt Disable Bit */
-#define AT_XDMAC_CID_WBEID BIT(5) /* Write Bus Error Interrupt Disable Bit */
-#define AT_XDMAC_CID_ROID BIT(6) /* Request Overflow Interrupt Disable Bit */
-#define AT_XDMAC_CIM 0x08 /* Channel Interrupt Mask Register */
-#define AT_XDMAC_CIM_BIM BIT(0) /* End of Block Interrupt Mask Bit */
-#define AT_XDMAC_CIM_LIM BIT(1) /* End of Linked List Interrupt Mask Bit */
-#define AT_XDMAC_CIM_DIM BIT(2) /* End of Disable Interrupt Mask Bit */
-#define AT_XDMAC_CIM_FIM BIT(3) /* End of Flush Interrupt Mask Bit */
-#define AT_XDMAC_CIM_RBEIM BIT(4) /* Read Bus Error Interrupt Mask Bit */
-#define AT_XDMAC_CIM_WBEIM BIT(5) /* Write Bus Error Interrupt Mask Bit */
-#define AT_XDMAC_CIM_ROIM BIT(6) /* Request Overflow Interrupt Mask Bit */
-#define AT_XDMAC_CIS 0x0C /* Channel Interrupt Status Register */
-#define AT_XDMAC_CIS_BIS BIT(0) /* End of Block Interrupt Status Bit */
-#define AT_XDMAC_CIS_LIS BIT(1) /* End of Linked List Interrupt Status Bit */
-#define AT_XDMAC_CIS_DIS BIT(2) /* End of Disable Interrupt Status Bit */
-#define AT_XDMAC_CIS_FIS BIT(3) /* End of Flush Interrupt Status Bit */
-#define AT_XDMAC_CIS_RBEIS BIT(4) /* Read Bus Error Interrupt Status Bit */
-#define AT_XDMAC_CIS_WBEIS BIT(5) /* Write Bus Error Interrupt Status Bit */
-#define AT_XDMAC_CIS_ROIS BIT(6) /* Request Overflow Interrupt Status Bit */
-#define AT_XDMAC_CSA 0x10 /* Channel Source Address Register */
-#define AT_XDMAC_CDA 0x14 /* Channel Destination Address Register */
-#define AT_XDMAC_CNDA 0x18 /* Channel Next Descriptor Address Register */
-#define AT_XDMAC_CNDA_NDAIF(i) ((i) & 0x1) /* Channel x Next Descriptor Interface */
-#define AT_XDMAC_CNDA_NDA(i) ((i) & 0xfffffffc) /* Channel x Next Descriptor Address */
-#define AT_XDMAC_CNDC 0x1C /* Channel Next Descriptor Control Register */
-#define AT_XDMAC_CNDC_NDE (0x1 << 0) /* Channel x Next Descriptor Enable */
-#define AT_XDMAC_CNDC_NDSUP (0x1 << 1) /* Channel x Next Descriptor Source Update */
-#define AT_XDMAC_CNDC_NDDUP (0x1 << 2) /* Channel x Next Descriptor Destination Update */
-#define AT_XDMAC_CNDC_NDVIEW_NDV0 (0x0 << 3) /* Channel x Next Descriptor View 0 */
-#define AT_XDMAC_CNDC_NDVIEW_NDV1 (0x1 << 3) /* Channel x Next Descriptor View 1 */
-#define AT_XDMAC_CNDC_NDVIEW_NDV2 (0x2 << 3) /* Channel x Next Descriptor View 2 */
-#define AT_XDMAC_CNDC_NDVIEW_NDV3 (0x3 << 3) /* Channel x Next Descriptor View 3 */
-#define AT_XDMAC_CUBC 0x20 /* Channel Microblock Control Register */
-#define AT_XDMAC_CBC 0x24 /* Channel Block Control Register */
-#define AT_XDMAC_CC 0x28 /* Channel Configuration Register */
-#define AT_XDMAC_CC_TYPE (0x1 << 0) /* Channel Transfer Type */
-#define AT_XDMAC_CC_TYPE_MEM_TRAN (0x0 << 0) /* Memory to Memory Transfer */
-#define AT_XDMAC_CC_TYPE_PER_TRAN (0x1 << 0) /* Peripheral to Memory or Memory to Peripheral Transfer */
-#define AT_XDMAC_CC_MBSIZE_MASK (0x3 << 1)
-#define AT_XDMAC_CC_MBSIZE_SINGLE (0x0 << 1)
-#define AT_XDMAC_CC_MBSIZE_FOUR (0x1 << 1)
-#define AT_XDMAC_CC_MBSIZE_EIGHT (0x2 << 1)
-#define AT_XDMAC_CC_MBSIZE_SIXTEEN (0x3 << 1)
-#define AT_XDMAC_CC_DSYNC (0x1 << 4) /* Channel Synchronization */
-#define AT_XDMAC_CC_DSYNC_PER2MEM (0x0 << 4)
-#define AT_XDMAC_CC_DSYNC_MEM2PER (0x1 << 4)
-#define AT_XDMAC_CC_PROT (0x1 << 5) /* Channel Protection */
-#define AT_XDMAC_CC_PROT_SEC (0x0 << 5)
-#define AT_XDMAC_CC_PROT_UNSEC (0x1 << 5)
-#define AT_XDMAC_CC_SWREQ (0x1 << 6) /* Channel Software Request Trigger */
-#define AT_XDMAC_CC_SWREQ_HWR_CONNECTED (0x0 << 6)
-#define AT_XDMAC_CC_SWREQ_SWR_CONNECTED (0x1 << 6)
-#define AT_XDMAC_CC_MEMSET (0x1 << 7) /* Channel Fill Block of memory */
-#define AT_XDMAC_CC_MEMSET_NORMAL_MODE (0x0 << 7)
-#define AT_XDMAC_CC_MEMSET_HW_MODE (0x1 << 7)
-#define AT_XDMAC_CC_CSIZE(i) ((0x7 & (i)) << 8) /* Channel Chunk Size */
-#define AT_XDMAC_CC_DWIDTH_OFFSET 11
-#define AT_XDMAC_CC_DWIDTH_MASK (0x3 << AT_XDMAC_CC_DWIDTH_OFFSET)
-#define AT_XDMAC_CC_DWIDTH(i) ((0x3 & (i)) << AT_XDMAC_CC_DWIDTH_OFFSET) /* Channel Data Width */
-#define AT_XDMAC_CC_DWIDTH_BYTE 0x0
-#define AT_XDMAC_CC_DWIDTH_HALFWORD 0x1
-#define AT_XDMAC_CC_DWIDTH_WORD 0x2
-#define AT_XDMAC_CC_DWIDTH_DWORD 0x3
-#define AT_XDMAC_CC_SIF(i) ((0x1 & (i)) << 13) /* Channel Source Interface Identifier */
-#define AT_XDMAC_CC_DIF(i) ((0x1 & (i)) << 14) /* Channel Destination Interface Identifier */
-#define AT_XDMAC_CC_SAM_MASK (0x3 << 16) /* Channel Source Addressing Mode */
-#define AT_XDMAC_CC_SAM_FIXED_AM (0x0 << 16)
-#define AT_XDMAC_CC_SAM_INCREMENTED_AM (0x1 << 16)
-#define AT_XDMAC_CC_SAM_UBS_AM (0x2 << 16)
-#define AT_XDMAC_CC_SAM_UBS_DS_AM (0x3 << 16)
-#define AT_XDMAC_CC_DAM_MASK (0x3 << 18) /* Channel Source Addressing Mode */
-#define AT_XDMAC_CC_DAM_FIXED_AM (0x0 << 18)
-#define AT_XDMAC_CC_DAM_INCREMENTED_AM (0x1 << 18)
-#define AT_XDMAC_CC_DAM_UBS_AM (0x2 << 18)
-#define AT_XDMAC_CC_DAM_UBS_DS_AM (0x3 << 18)
-#define AT_XDMAC_CC_INITD (0x1 << 21) /* Channel Initialization Terminated (read only) */
-#define AT_XDMAC_CC_INITD_TERMINATED (0x0 << 21)
-#define AT_XDMAC_CC_INITD_IN_PROGRESS (0x1 << 21)
-#define AT_XDMAC_CC_RDIP (0x1 << 22) /* Read in Progress (read only) */
-#define AT_XDMAC_CC_RDIP_DONE (0x0 << 22)
-#define AT_XDMAC_CC_RDIP_IN_PROGRESS (0x1 << 22)
-#define AT_XDMAC_CC_WRIP (0x1 << 23) /* Write in Progress (read only) */
-#define AT_XDMAC_CC_WRIP_DONE (0x0 << 23)
-#define AT_XDMAC_CC_WRIP_IN_PROGRESS (0x1 << 23)
-#define AT_XDMAC_CC_PERID(i) (0x7f & (i) << 24) /* Channel Peripheral Identifier */
-#define AT_XDMAC_CDS_MSP 0x2C /* Channel Data Stride Memory Set Pattern */
-#define AT_XDMAC_CSUS 0x30 /* Channel Source Microblock Stride */
-#define AT_XDMAC_CDUS 0x34 /* Channel Destination Microblock Stride */
-
-#define AT_XDMAC_CHAN_REG_BASE 0x50 /* Channel registers base address */
-
-/* Microblock control members */
-#define AT_XDMAC_MBR_UBC_UBLEN_MAX 0xFFFFFFUL /* Maximum Microblock Length */
-#define AT_XDMAC_MBR_UBC_NDE (0x1 << 24) /* Next Descriptor Enable */
-#define AT_XDMAC_MBR_UBC_NSEN (0x1 << 25) /* Next Descriptor Source Update */
-#define AT_XDMAC_MBR_UBC_NDEN (0x1 << 26) /* Next Descriptor Destination Update */
-#define AT_XDMAC_MBR_UBC_NDV0 (0x0 << 27) /* Next Descriptor View 0 */
-#define AT_XDMAC_MBR_UBC_NDV1 (0x1 << 27) /* Next Descriptor View 1 */
-#define AT_XDMAC_MBR_UBC_NDV2 (0x2 << 27) /* Next Descriptor View 2 */
-#define AT_XDMAC_MBR_UBC_NDV3 (0x3 << 27) /* Next Descriptor View 3 */
-
-#define AT_XDMAC_MAX_CHAN 0x20
-#define AT_XDMAC_MAX_CSIZE 16 /* 16 data */
-#define AT_XDMAC_MAX_DWIDTH 8 /* 64 bits */
-#define AT_XDMAC_RESIDUE_MAX_RETRIES 5
+#include "at_xdmac_regs.h"

#define AT_XDMAC_DMA_BUSWIDTHS\
(BIT(DMA_SLAVE_BUSWIDTH_UNDEFINED) |\
diff --git a/drivers/dma/at_xdmac_regs.h b/drivers/dma/at_xdmac_regs.h
new file mode 100644
index 000000000000..3f7dda4c5743
--- /dev/null
+++ b/drivers/dma/at_xdmac_regs.h
@@ -0,0 +1,154 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Header file for the Atmel Extensible DMA Controller (aka XDMAC on AT91 systems)
+ *
+ * Copyright (C) 2014-2020 Microchip Technology, Inc. and its subsidiaries
+ *
+ */
+#ifndef AT_XDMAC_REGS_H
+#define AT_XDMAC_REGS_H
+
+/* Global registers */
+#define AT_XDMAC_GTYPE 0x00 /* Global Type Register */
+#define AT_XDMAC_NB_CH(i) (((i) & 0x1F) + 1) /* Number of Channels Minus One */
+#define AT_XDMAC_FIFO_SZ(i) (((i) >> 5) & 0x7FF) /* Number of Bytes */
+#define AT_XDMAC_NB_REQ(i) ((((i) >> 16) & 0x3F) + 1) /* Number of Peripheral Requests Minus One */
+#define AT_XDMAC_GCFG 0x04 /* Global Configuration Register */
+#define AT_XDMAC_GWAC 0x08 /* Global Weighted Arbiter Configuration Register */
+#define AT_XDMAC_GIE 0x0C /* Global Interrupt Enable Register */
+#define AT_XDMAC_GID 0x10 /* Global Interrupt Disable Register */
+#define AT_XDMAC_GIM 0x14 /* Global Interrupt Mask Register */
+#define AT_XDMAC_GIS 0x18 /* Global Interrupt Status Register */
+#define AT_XDMAC_GE 0x1C /* Global Channel Enable Register */
+#define AT_XDMAC_GD 0x20 /* Global Channel Disable Register */
+#define AT_XDMAC_GS 0x24 /* Global Channel Status Register */
+#define AT_XDMAC_GRS 0x28 /* Global Channel Read Suspend Register */
+#define AT_XDMAC_GWS 0x2C /* Global Write Suspend Register */
+#define AT_XDMAC_GRWS 0x30 /* Global Channel Read Write Suspend Register */
+#define AT_XDMAC_GRWR 0x34 /* Global Channel Read Write Resume Register */
+#define AT_XDMAC_GSWR 0x38 /* Global Channel Software Request Register */
+#define AT_XDMAC_GSWS 0x3C /* Global channel Software Request Status Register */
+#define AT_XDMAC_GSWF 0x40 /* Global Channel Software Flush Request Register */
+#define AT_XDMAC_VERSION 0xFFC /* XDMAC Version Register */
+
+/* Channel relative registers offsets */
+#define AT_XDMAC_CIE 0x00 /* Channel Interrupt Enable Register */
+#define AT_XDMAC_CIE_BIE BIT(0) /* End of Block Interrupt Enable Bit */
+#define AT_XDMAC_CIE_LIE BIT(1) /* End of Linked List Interrupt Enable Bit */
+#define AT_XDMAC_CIE_DIE BIT(2) /* End of Disable Interrupt Enable Bit */
+#define AT_XDMAC_CIE_FIE BIT(3) /* End of Flush Interrupt Enable Bit */
+#define AT_XDMAC_CIE_RBEIE BIT(4) /* Read Bus Error Interrupt Enable Bit */
+#define AT_XDMAC_CIE_WBEIE BIT(5) /* Write Bus Error Interrupt Enable Bit */
+#define AT_XDMAC_CIE_ROIE BIT(6) /* Request Overflow Interrupt Enable Bit */
+#define AT_XDMAC_CID 0x04 /* Channel Interrupt Disable Register */
+#define AT_XDMAC_CID_BID BIT(0) /* End of Block Interrupt Disable Bit */
+#define AT_XDMAC_CID_LID BIT(1) /* End of Linked List Interrupt Disable Bit */
+#define AT_XDMAC_CID_DID BIT(2) /* End of Disable Interrupt Disable Bit */
+#define AT_XDMAC_CID_FID BIT(3) /* End of Flush Interrupt Disable Bit */
+#define AT_XDMAC_CID_RBEID BIT(4) /* Read Bus Error Interrupt Disable Bit */
+#define AT_XDMAC_CID_WBEID BIT(5) /* Write Bus Error Interrupt Disable Bit */
+#define AT_XDMAC_CID_ROID BIT(6) /* Request Overflow Interrupt Disable Bit */
+#define AT_XDMAC_CIM 0x08 /* Channel Interrupt Mask Register */
+#define AT_XDMAC_CIM_BIM BIT(0) /* End of Block Interrupt Mask Bit */
+#define AT_XDMAC_CIM_LIM BIT(1) /* End of Linked List Interrupt Mask Bit */
+#define AT_XDMAC_CIM_DIM BIT(2) /* End of Disable Interrupt Mask Bit */
+#define AT_XDMAC_CIM_FIM BIT(3) /* End of Flush Interrupt Mask Bit */
+#define AT_XDMAC_CIM_RBEIM BIT(4) /* Read Bus Error Interrupt Mask Bit */
+#define AT_XDMAC_CIM_WBEIM BIT(5) /* Write Bus Error Interrupt Mask Bit */
+#define AT_XDMAC_CIM_ROIM BIT(6) /* Request Overflow Interrupt Mask Bit */
+#define AT_XDMAC_CIS 0x0C /* Channel Interrupt Status Register */
+#define AT_XDMAC_CIS_BIS BIT(0) /* End of Block Interrupt Status Bit */
+#define AT_XDMAC_CIS_LIS BIT(1) /* End of Linked List Interrupt Status Bit */
+#define AT_XDMAC_CIS_DIS BIT(2) /* End of Disable Interrupt Status Bit */
+#define AT_XDMAC_CIS_FIS BIT(3) /* End of Flush Interrupt Status Bit */
+#define AT_XDMAC_CIS_RBEIS BIT(4) /* Read Bus Error Interrupt Status Bit */
+#define AT_XDMAC_CIS_WBEIS BIT(5) /* Write Bus Error Interrupt Status Bit */
+#define AT_XDMAC_CIS_ROIS BIT(6) /* Request Overflow Interrupt Status Bit */
+#define AT_XDMAC_CSA 0x10 /* Channel Source Address Register */
+#define AT_XDMAC_CDA 0x14 /* Channel Destination Address Register */
+#define AT_XDMAC_CNDA 0x18 /* Channel Next Descriptor Address Register */
+#define AT_XDMAC_CNDA_NDAIF(i) ((i) & 0x1) /* Channel x Next Descriptor Interface */
+#define AT_XDMAC_CNDA_NDA(i) ((i) & 0xfffffffc) /* Channel x Next Descriptor Address */
+#define AT_XDMAC_CNDC 0x1C /* Channel Next Descriptor Control Register */
+#define AT_XDMAC_CNDC_NDE (0x1 << 0) /* Channel x Next Descriptor Enable */
+#define AT_XDMAC_CNDC_NDSUP (0x1 << 1) /* Channel x Next Descriptor Source Update */
+#define AT_XDMAC_CNDC_NDDUP (0x1 << 2) /* Channel x Next Descriptor Destination Update */
+#define AT_XDMAC_CNDC_NDVIEW_NDV0 (0x0 << 3) /* Channel x Next Descriptor View 0 */
+#define AT_XDMAC_CNDC_NDVIEW_NDV1 (0x1 << 3) /* Channel x Next Descriptor View 1 */
+#define AT_XDMAC_CNDC_NDVIEW_NDV2 (0x2 << 3) /* Channel x Next Descriptor View 2 */
+#define AT_XDMAC_CNDC_NDVIEW_NDV3 (0x3 << 3) /* Channel x Next Descriptor View 3 */
+#define AT_XDMAC_CUBC 0x20 /* Channel Microblock Control Register */
+#define AT_XDMAC_CBC 0x24 /* Channel Block Control Register */
+#define AT_XDMAC_CC 0x28 /* Channel Configuration Register */
+#define AT_XDMAC_CC_TYPE (0x1 << 0) /* Channel Transfer Type */
+#define AT_XDMAC_CC_TYPE_MEM_TRAN (0x0 << 0) /* Memory to Memory Transfer */
+#define AT_XDMAC_CC_TYPE_PER_TRAN (0x1 << 0) /* Peripheral to Memory or Memory to Peripheral Transfer */
+#define AT_XDMAC_CC_MBSIZE_MASK (0x3 << 1)
+#define AT_XDMAC_CC_MBSIZE_SINGLE (0x0 << 1)
+#define AT_XDMAC_CC_MBSIZE_FOUR (0x1 << 1)
+#define AT_XDMAC_CC_MBSIZE_EIGHT (0x2 << 1)
+#define AT_XDMAC_CC_MBSIZE_SIXTEEN (0x3 << 1)
+#define AT_XDMAC_CC_DSYNC (0x1 << 4) /* Channel Synchronization */
+#define AT_XDMAC_CC_DSYNC_PER2MEM (0x0 << 4)
+#define AT_XDMAC_CC_DSYNC_MEM2PER (0x1 << 4)
+#define AT_XDMAC_CC_PROT (0x1 << 5) /* Channel Protection */
+#define AT_XDMAC_CC_PROT_SEC (0x0 << 5)
+#define AT_XDMAC_CC_PROT_UNSEC (0x1 << 5)
+#define AT_XDMAC_CC_SWREQ (0x1 << 6) /* Channel Software Request Trigger */
+#define AT_XDMAC_CC_SWREQ_HWR_CONNECTED (0x0 << 6)
+#define AT_XDMAC_CC_SWREQ_SWR_CONNECTED (0x1 << 6)
+#define AT_XDMAC_CC_MEMSET (0x1 << 7) /* Channel Fill Block of memory */
+#define AT_XDMAC_CC_MEMSET_NORMAL_MODE (0x0 << 7)
+#define AT_XDMAC_CC_MEMSET_HW_MODE (0x1 << 7)
+#define AT_XDMAC_CC_CSIZE(i) ((0x7 & (i)) << 8) /* Channel Chunk Size */
+#define AT_XDMAC_CC_DWIDTH_OFFSET 11
+#define AT_XDMAC_CC_DWIDTH_MASK (0x3 << AT_XDMAC_CC_DWIDTH_OFFSET)
+#define AT_XDMAC_CC_DWIDTH(i) ((0x3 & (i)) << AT_XDMAC_CC_DWIDTH_OFFSET) /* Channel Data Width */
+#define AT_XDMAC_CC_DWIDTH_BYTE 0x0
+#define AT_XDMAC_CC_DWIDTH_HALFWORD 0x1
+#define AT_XDMAC_CC_DWIDTH_WORD 0x2
+#define AT_XDMAC_CC_DWIDTH_DWORD 0x3
+#define AT_XDMAC_CC_SIF(i) ((0x1 & (i)) << 13) /* Channel Source Interface Identifier */
+#define AT_XDMAC_CC_DIF(i) ((0x1 & (i)) << 14) /* Channel Destination Interface Identifier */
+#define AT_XDMAC_CC_SAM_MASK (0x3 << 16) /* Channel Source Addressing Mode */
+#define AT_XDMAC_CC_SAM_FIXED_AM (0x0 << 16)
+#define AT_XDMAC_CC_SAM_INCREMENTED_AM (0x1 << 16)
+#define AT_XDMAC_CC_SAM_UBS_AM (0x2 << 16)
+#define AT_XDMAC_CC_SAM_UBS_DS_AM (0x3 << 16)
+#define AT_XDMAC_CC_DAM_MASK (0x3 << 18) /* Channel Source Addressing Mode */
+#define AT_XDMAC_CC_DAM_FIXED_AM (0x0 << 18)
+#define AT_XDMAC_CC_DAM_INCREMENTED_AM (0x1 << 18)
+#define AT_XDMAC_CC_DAM_UBS_AM (0x2 << 18)
+#define AT_XDMAC_CC_DAM_UBS_DS_AM (0x3 << 18)
+#define AT_XDMAC_CC_INITD (0x1 << 21) /* Channel Initialization Terminated (read only) */
+#define AT_XDMAC_CC_INITD_TERMINATED (0x0 << 21)
+#define AT_XDMAC_CC_INITD_IN_PROGRESS (0x1 << 21)
+#define AT_XDMAC_CC_RDIP (0x1 << 22) /* Read in Progress (read only) */
+#define AT_XDMAC_CC_RDIP_DONE (0x0 << 22)
+#define AT_XDMAC_CC_RDIP_IN_PROGRESS (0x1 << 22)
+#define AT_XDMAC_CC_WRIP (0x1 << 23) /* Write in Progress (read only) */
+#define AT_XDMAC_CC_WRIP_DONE (0x0 << 23)
+#define AT_XDMAC_CC_WRIP_IN_PROGRESS (0x1 << 23)
+#define AT_XDMAC_CC_PERID(i) (0x7f & (i) << 24) /* Channel Peripheral Identifier */
+#define AT_XDMAC_CDS_MSP 0x2C /* Channel Data Stride Memory Set Pattern */
+#define AT_XDMAC_CSUS 0x30 /* Channel Source Microblock Stride */
+#define AT_XDMAC_CDUS 0x34 /* Channel Destination Microblock Stride */
+
+#define AT_XDMAC_CHAN_REG_BASE 0x50 /* Channel registers base address */
+
+/* Microblock control members */
+#define AT_XDMAC_MBR_UBC_UBLEN_MAX 0xFFFFFFUL /* Maximum Microblock Length */
+#define AT_XDMAC_MBR_UBC_NDE (0x1 << 24) /* Next Descriptor Enable */
+#define AT_XDMAC_MBR_UBC_NSEN (0x1 << 25) /* Next Descriptor Source Update */
+#define AT_XDMAC_MBR_UBC_NDEN (0x1 << 26) /* Next Descriptor Destination Update */
+#define AT_XDMAC_MBR_UBC_NDV0 (0x0 << 27) /* Next Descriptor View 0 */
+#define AT_XDMAC_MBR_UBC_NDV1 (0x1 << 27) /* Next Descriptor View 1 */
+#define AT_XDMAC_MBR_UBC_NDV2 (0x2 << 27) /* Next Descriptor View 2 */
+#define AT_XDMAC_MBR_UBC_NDV3 (0x3 << 27) /* Next Descriptor View 3 */
+
+#define AT_XDMAC_MAX_CHAN 0x20
+#define AT_XDMAC_MAX_CSIZE 16 /* 16 data */
+#define AT_XDMAC_MAX_DWIDTH 8 /* 64 bits */
+#define AT_XDMAC_RESIDUE_MAX_RETRIES 5
+
+#endif
--
2.25.1

2020-09-14 14:12:58

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 4/7] dmaengine: at_xdmac: adapt perid for mem2mem operations

The PERID in the CC register for mem2mem operations must match an unused
PERID.
The PERID field is 7 bits, but the selected value is 0x3f.
On later products we can have more reserved PERIDs for actual peripherals,
thus this needs to be increased to maximum size.
Changing the value to 0x7f, which is the maximum for 7 bits field.

Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/dma/at_xdmac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index fab19e00a7be..81bb90206092 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -726,7 +726,7 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
* match the one of another channel. If not, it could lead to spurious
* flag status.
*/
- u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
+ u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
| AT_XDMAC_CC_DIF(0)
| AT_XDMAC_CC_SIF(0)
| AT_XDMAC_CC_MBSIZE_SIXTEEN
@@ -908,7 +908,7 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
* match the one of another channel. If not, it could lead to spurious
* flag status.
*/
- u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
+ u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
| AT_XDMAC_CC_DAM_INCREMENTED_AM
| AT_XDMAC_CC_SAM_INCREMENTED_AM
| AT_XDMAC_CC_DIF(0)
@@ -1014,7 +1014,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
* match the one of another channel. If not, it could lead to spurious
* flag status.
*/
- u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
+ u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
| AT_XDMAC_CC_DAM_UBS_AM
| AT_XDMAC_CC_SAM_INCREMENTED_AM
| AT_XDMAC_CC_DIF(0)
--
2.25.1

2020-09-14 14:13:50

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 7/7] dmaengine: at_xdmac: add AXI priority support and recommended settings

The sama7g5 version of the XDMAC supports priority configuration and
outstanding capabilities.
Add defines for the specific registers for this configuration, together
with recommended settings.
However the settings are very different if the XDMAC is a mem2mem or a
per2mem controller.
Thus, we need to differentiate according to device tree property.

Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/dma/at_xdmac.c | 25 +++++++++++++++++++++++++
drivers/dma/at_xdmac_regs.h | 16 ++++++++++++++++
2 files changed, 41 insertions(+)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 874484a4e79f..8ea5558e127d 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -57,6 +57,8 @@ struct at_xdmac_layout {
u8 chan_cc_reg_base;
/* Source/Destination Interface must be specified or not */
bool sdif;
+ /* AXI queue priority configuration supported */
+ bool axi_config;
};

/* ----- Channels ----- */
@@ -135,6 +137,7 @@ static struct at_xdmac_layout at_xdmac_sama5d4_layout = {
.gswf = 0x40,
.chan_cc_reg_base = 0x50,
.sdif = true,
+ .axi_config = false,
};

static struct at_xdmac_layout at_xdmac_sama7g5_layout = {
@@ -147,6 +150,7 @@ static struct at_xdmac_layout at_xdmac_sama7g5_layout = {
.gswf = 0x50,
.chan_cc_reg_base = 0x60,
.sdif = false,
+ .axi_config = true,
};

static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, unsigned int chan_nb)
@@ -1863,6 +1867,25 @@ static int atmel_xdmac_resume(struct device *dev)
}
#endif /* CONFIG_PM_SLEEP */

+static void at_xdmac_axi_config(struct platform_device *pdev)
+{
+ struct at_xdmac *atxdmac = (struct at_xdmac *)platform_get_drvdata(pdev);
+ bool dev_m2m;
+
+ if (!atxdmac->layout->axi_config)
+ return; /* Not supported */
+
+ dev_m2m = of_property_read_bool(pdev->dev.of_node, "microchip,m2m");
+
+ if (dev_m2m) {
+ at_xdmac_write(atxdmac, AT_XDMAC_GCFG, AT_XDMAC_GCFG_M2M);
+ at_xdmac_write(atxdmac, AT_XDMAC_GWAC, AT_XDMAC_GWAC_M2M);
+ } else {
+ at_xdmac_write(atxdmac, AT_XDMAC_GCFG, AT_XDMAC_GCFG_P2M);
+ at_xdmac_write(atxdmac, AT_XDMAC_GWAC, AT_XDMAC_GWAC_P2M);
+ }
+}
+
static int at_xdmac_probe(struct platform_device *pdev)
{
struct at_xdmac *atxdmac;
@@ -2008,6 +2031,8 @@ static int at_xdmac_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "%d channels, mapped at 0x%p\n",
nr_channels, atxdmac->regs);

+ at_xdmac_axi_config(pdev);
+
return 0;

err_dma_unregister:
diff --git a/drivers/dma/at_xdmac_regs.h b/drivers/dma/at_xdmac_regs.h
index 7b4b4e24de70..e5a58f6194aa 100644
--- a/drivers/dma/at_xdmac_regs.h
+++ b/drivers/dma/at_xdmac_regs.h
@@ -14,7 +14,23 @@
#define AT_XDMAC_FIFO_SZ(i) (((i) >> 5) & 0x7FF) /* Number of Bytes */
#define AT_XDMAC_NB_REQ(i) ((((i) >> 16) & 0x3F) + 1) /* Number of Peripheral Requests Minus One */
#define AT_XDMAC_GCFG 0x04 /* Global Configuration Register */
+#define AT_XDMAC_WRHP(i) (((i) & 0xF) << 4)
+#define AT_XDMAC_WRMP(i) (((i) & 0xF) << 8)
+#define AT_XDMAC_WRLP(i) (((i) & 0xF) << 12)
+#define AT_XDMAC_RDHP(i) (((i) & 0xF) << 16)
+#define AT_XDMAC_RDMP(i) (((i) & 0xF) << 20)
+#define AT_XDMAC_RDLP(i) (((i) & 0xF) << 24)
+#define AT_XDMAC_RDSG(i) (((i) & 0xF) << 28)
+#define AT_XDMAC_GCFG_M2M (AT_XDMAC_RDLP(0xF) | AT_XDMAC_WRLP(0xF))
+#define AT_XDMAC_GCFG_P2M (AT_XDMAC_RDSG(0x1) | AT_XDMAC_RDHP(0x3) | \
+ AT_XDMAC_WRHP(0x5))
#define AT_XDMAC_GWAC 0x08 /* Global Weighted Arbiter Configuration Register */
+#define AT_XDMAC_PW0(i) (((i) & 0xF) << 0)
+#define AT_XDMAC_PW1(i) (((i) & 0xF) << 4)
+#define AT_XDMAC_PW2(i) (((i) & 0xF) << 8)
+#define AT_XDMAC_PW3(i) (((i) & 0xF) << 12)
+#define AT_XDMAC_GWAC_M2M 0
+#define AT_XDMAC_GWAC_P2M (AT_XDMAC_PW0(0xF) | AT_XDMAC_PW2(0xF))
#define AT_XDMAC_GIE 0x0C /* Global Interrupt Enable Register */
#define AT_XDMAC_GID 0x10 /* Global Interrupt Disable Register */
#define AT_XDMAC_GIM 0x14 /* Global Interrupt Mask Register */
--
2.25.1

2020-09-14 14:13:55

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 5/7] dmaengine: at_xdmac: add support for sama7g5 based at_xdmac

SAMA7G5 SoC uses a slightly different variant of the AT_XDMAC.
Added support by a new compatible and a layout struct that copes
to the specific version considering the compatible string.
Only the differences in register map are present in the layout struct.
I reworked the register access for this part that has the differences.
Also the Source/Destination Interface bits are no longer valid for this
variant of the XDMAC. Thus, the layout also has a bool for specifying
whether these bits are required or not.

Signed-off-by: Eugen Hristev <[email protected]>
---
drivers/dma/at_xdmac.c | 99 ++++++++++++++++++++++++++++++-------
drivers/dma/at_xdmac_regs.h | 9 ----
2 files changed, 82 insertions(+), 26 deletions(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 81bb90206092..874484a4e79f 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -38,6 +38,27 @@ enum atc_status {
AT_XDMAC_CHAN_IS_PAUSED,
};

+struct at_xdmac_layout {
+ /* Global Channel Read Suspend Register */
+ u8 grs;
+ /* Global Write Suspend Register */
+ u8 gws;
+ /* Global Channel Read Write Suspend Register */
+ u8 grws;
+ /* Global Channel Read Write Resume Register */
+ u8 grwr;
+ /* Global Channel Software Request Register */
+ u8 gswr;
+ /* Global channel Software Request Status Register */
+ u8 gsws;
+ /* Global Channel Software Flush Request Register */
+ u8 gswf;
+ /* Channel reg base */
+ u8 chan_cc_reg_base;
+ /* Source/Destination Interface must be specified or not */
+ bool sdif;
+};
+
/* ----- Channels ----- */
struct at_xdmac_chan {
struct dma_chan chan;
@@ -71,6 +92,7 @@ struct at_xdmac {
struct clk *clk;
u32 save_gim;
struct dma_pool *at_xdmac_desc_pool;
+ const struct at_xdmac_layout *layout;
struct at_xdmac_chan chan[];
};

@@ -103,9 +125,33 @@ struct at_xdmac_desc {
struct list_head xfer_node;
} __aligned(sizeof(u64));

+static struct at_xdmac_layout at_xdmac_sama5d4_layout = {
+ .grs = 0x28,
+ .gws = 0x2C,
+ .grws = 0x30,
+ .grwr = 0x34,
+ .gswr = 0x38,
+ .gsws = 0x3C,
+ .gswf = 0x40,
+ .chan_cc_reg_base = 0x50,
+ .sdif = true,
+};
+
+static struct at_xdmac_layout at_xdmac_sama7g5_layout = {
+ .grs = 0x30,
+ .gws = 0x38,
+ .grws = 0x40,
+ .grwr = 0x44,
+ .gswr = 0x48,
+ .gsws = 0x4C,
+ .gswf = 0x50,
+ .chan_cc_reg_base = 0x60,
+ .sdif = false,
+};
+
static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, unsigned int chan_nb)
{
- return atxdmac->regs + (AT_XDMAC_CHAN_REG_BASE + chan_nb * 0x40);
+ return atxdmac->regs + (atxdmac->layout->chan_cc_reg_base + chan_nb * 0x40);
}

#define at_xdmac_read(atxdmac, reg) readl_relaxed((atxdmac)->regs + (reg))
@@ -204,8 +250,10 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan,
first->active_xfer = true;

/* Tell xdmac where to get the first descriptor. */
- reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys)
- | AT_XDMAC_CNDA_NDAIF(atchan->memif);
+ reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys);
+ if (atxdmac->layout->sdif)
+ reg |= AT_XDMAC_CNDA_NDAIF(atchan->memif);
+
at_xdmac_chan_write(atchan, AT_XDMAC_CNDA, reg);

/*
@@ -400,6 +448,7 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
enum dma_transfer_direction direction)
{
struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
+ struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
int csize, dwidth;

if (direction == DMA_DEV_TO_MEM) {
@@ -407,12 +456,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
AT91_XDMAC_DT_PERID(atchan->perid)
| AT_XDMAC_CC_DAM_INCREMENTED_AM
| AT_XDMAC_CC_SAM_FIXED_AM
- | AT_XDMAC_CC_DIF(atchan->memif)
- | AT_XDMAC_CC_SIF(atchan->perif)
| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
| AT_XDMAC_CC_DSYNC_PER2MEM
| AT_XDMAC_CC_MBSIZE_SIXTEEN
| AT_XDMAC_CC_TYPE_PER_TRAN;
+ if (atxdmac->layout->sdif)
+ atchan->cfg |= AT_XDMAC_CC_DIF(atchan->memif)
+ | AT_XDMAC_CC_SIF(atchan->perif);
+
csize = ffs(atchan->sconfig.src_maxburst) - 1;
if (csize < 0) {
dev_err(chan2dev(chan), "invalid src maxburst value\n");
@@ -430,12 +481,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
AT91_XDMAC_DT_PERID(atchan->perid)
| AT_XDMAC_CC_DAM_FIXED_AM
| AT_XDMAC_CC_SAM_INCREMENTED_AM
- | AT_XDMAC_CC_DIF(atchan->perif)
- | AT_XDMAC_CC_SIF(atchan->memif)
| AT_XDMAC_CC_SWREQ_HWR_CONNECTED
| AT_XDMAC_CC_DSYNC_MEM2PER
| AT_XDMAC_CC_MBSIZE_SIXTEEN
| AT_XDMAC_CC_TYPE_PER_TRAN;
+ if (atxdmac->layout->sdif)
+ atchan->cfg |= AT_XDMAC_CC_DIF(atchan->perif)
+ | AT_XDMAC_CC_SIF(atchan->memif);
+
csize = ffs(atchan->sconfig.dst_maxburst) - 1;
if (csize < 0) {
dev_err(chan2dev(chan), "invalid src maxburst value\n");
@@ -711,6 +764,7 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
struct data_chunk *chunk)
{
struct at_xdmac_desc *desc;
+ struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
u32 dwidth;
unsigned long flags;
size_t ublen;
@@ -727,10 +781,10 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
* flag status.
*/
u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
- | AT_XDMAC_CC_DIF(0)
- | AT_XDMAC_CC_SIF(0)
| AT_XDMAC_CC_MBSIZE_SIXTEEN
| AT_XDMAC_CC_TYPE_MEM_TRAN;
+ if (atxdmac->layout->sdif)
+ chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);

dwidth = at_xdmac_align_width(chan, src | dst | chunk->size);
if (chunk->size >= (AT_XDMAC_MBR_UBC_UBLEN_MAX << dwidth)) {
@@ -893,6 +947,7 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
size_t len, unsigned long flags)
{
struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
+ struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
struct at_xdmac_desc *first = NULL, *prev = NULL;
size_t remaining_size = len, xfer_size = 0, ublen;
dma_addr_t src_addr = src, dst_addr = dest;
@@ -911,12 +966,13 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
| AT_XDMAC_CC_DAM_INCREMENTED_AM
| AT_XDMAC_CC_SAM_INCREMENTED_AM
- | AT_XDMAC_CC_DIF(0)
- | AT_XDMAC_CC_SIF(0)
| AT_XDMAC_CC_MBSIZE_SIXTEEN
| AT_XDMAC_CC_TYPE_MEM_TRAN;
unsigned long irqflags;

+ if (atxdmac->layout->sdif)
+ chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);
+
dev_dbg(chan2dev(chan), "%s: src=%pad, dest=%pad, len=%zd, flags=0x%lx\n",
__func__, &src, &dest, len, flags);

@@ -999,6 +1055,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
int value)
{
struct at_xdmac_desc *desc;
+ struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
unsigned long flags;
size_t ublen;
u32 dwidth;
@@ -1017,11 +1074,11 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
| AT_XDMAC_CC_DAM_UBS_AM
| AT_XDMAC_CC_SAM_INCREMENTED_AM
- | AT_XDMAC_CC_DIF(0)
- | AT_XDMAC_CC_SIF(0)
| AT_XDMAC_CC_MBSIZE_SIXTEEN
| AT_XDMAC_CC_MEMSET_HW_MODE
| AT_XDMAC_CC_TYPE_MEM_TRAN;
+ if (atxdmac->layout->sdif)
+ chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);

dwidth = at_xdmac_align_width(chan, dst_addr);

@@ -1297,7 +1354,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
mask = AT_XDMAC_CC_TYPE | AT_XDMAC_CC_DSYNC;
value = AT_XDMAC_CC_TYPE_PER_TRAN | AT_XDMAC_CC_DSYNC_PER2MEM;
if ((desc->lld.mbr_cfg & mask) == value) {
- at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
+ at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
cpu_relax();
}
@@ -1355,7 +1412,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
* FIFO flush ensures that data are really written.
*/
if ((desc->lld.mbr_cfg & mask) == value) {
- at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
+ at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
cpu_relax();
}
@@ -1620,7 +1677,7 @@ static int at_xdmac_device_pause(struct dma_chan *chan)
return 0;

spin_lock_irqsave(&atchan->lock, flags);
- at_xdmac_write(atxdmac, AT_XDMAC_GRWS, atchan->mask);
+ at_xdmac_write(atxdmac, atxdmac->layout->grws, atchan->mask);
while (at_xdmac_chan_read(atchan, AT_XDMAC_CC)
& (AT_XDMAC_CC_WRIP | AT_XDMAC_CC_RDIP))
cpu_relax();
@@ -1643,7 +1700,7 @@ static int at_xdmac_device_resume(struct dma_chan *chan)
return 0;
}

- at_xdmac_write(atxdmac, AT_XDMAC_GRWR, atchan->mask);
+ at_xdmac_write(atxdmac, atxdmac->layout->grwr, atchan->mask);
clear_bit(AT_XDMAC_CHAN_IS_PAUSED, &atchan->status);
spin_unlock_irqrestore(&atchan->lock, flags);

@@ -1845,6 +1902,10 @@ static int at_xdmac_probe(struct platform_device *pdev)
atxdmac->regs = base;
atxdmac->irq = irq;

+ atxdmac->layout = of_device_get_match_data(&pdev->dev);
+ if (!atxdmac->layout)
+ return -ENODEV;
+
atxdmac->clk = devm_clk_get(&pdev->dev, "dma_clk");
if (IS_ERR(atxdmac->clk)) {
dev_err(&pdev->dev, "can't get dma_clk\n");
@@ -1988,6 +2049,10 @@ static const struct dev_pm_ops atmel_xdmac_dev_pm_ops = {
static const struct of_device_id atmel_xdmac_dt_ids[] = {
{
.compatible = "atmel,sama5d4-dma",
+ .data = &at_xdmac_sama5d4_layout,
+ }, {
+ .compatible = "microchip,sama7g5-dma",
+ .data = &at_xdmac_sama7g5_layout,
}, {
/* sentinel */
}
diff --git a/drivers/dma/at_xdmac_regs.h b/drivers/dma/at_xdmac_regs.h
index 3f7dda4c5743..7b4b4e24de70 100644
--- a/drivers/dma/at_xdmac_regs.h
+++ b/drivers/dma/at_xdmac_regs.h
@@ -22,13 +22,6 @@
#define AT_XDMAC_GE 0x1C /* Global Channel Enable Register */
#define AT_XDMAC_GD 0x20 /* Global Channel Disable Register */
#define AT_XDMAC_GS 0x24 /* Global Channel Status Register */
-#define AT_XDMAC_GRS 0x28 /* Global Channel Read Suspend Register */
-#define AT_XDMAC_GWS 0x2C /* Global Write Suspend Register */
-#define AT_XDMAC_GRWS 0x30 /* Global Channel Read Write Suspend Register */
-#define AT_XDMAC_GRWR 0x34 /* Global Channel Read Write Resume Register */
-#define AT_XDMAC_GSWR 0x38 /* Global Channel Software Request Register */
-#define AT_XDMAC_GSWS 0x3C /* Global channel Software Request Status Register */
-#define AT_XDMAC_GSWF 0x40 /* Global Channel Software Flush Request Register */
#define AT_XDMAC_VERSION 0xFFC /* XDMAC Version Register */

/* Channel relative registers offsets */
@@ -134,8 +127,6 @@
#define AT_XDMAC_CSUS 0x30 /* Channel Source Microblock Stride */
#define AT_XDMAC_CDUS 0x34 /* Channel Destination Microblock Stride */

-#define AT_XDMAC_CHAN_REG_BASE 0x50 /* Channel registers base address */
-
/* Microblock control members */
#define AT_XDMAC_MBR_UBC_UBLEN_MAX 0xFFFFFFUL /* Maximum Microblock Length */
#define AT_XDMAC_MBR_UBC_NDE (0x1 << 24) /* Next Descriptor Enable */
--
2.25.1

2020-09-14 14:14:19

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 6/7] dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property

Add optional microchip,m2m property that specifies if a controller is
dedicated to memory to memory operations only.

Signed-off-by: Eugen Hristev <[email protected]>
---
Documentation/devicetree/bindings/dma/atmel-xdma.txt | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
index 510b7f25ba24..642da6b95a29 100644
--- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
+++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
@@ -15,6 +15,12 @@ the dmas property of client devices.
interface identifier,
- bit 30-24: PERID, peripheral identifier.

+Optional properties:
+- microchip,m2m: this controller is connected on AXI only to memory and it's
+ dedicated to memory to memory DMA operations. If this option is
+ missing, it's assumed that the DMA controller is connected to
+ peripherals, thus it's a per2mem and mem2per.
+
Example:

dma1: dma-controller@f0004000 {
--
2.25.1

2020-09-14 14:15:09

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 3/7] dt-bindings: dmaengine: at_xdmac: add compatible with microchip,sama7g5

Add compatible to sama7g5 SoC.

Signed-off-by: Eugen Hristev <[email protected]>
---
Documentation/devicetree/bindings/dma/atmel-xdma.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
index 4dc398e1a371..510b7f25ba24 100644
--- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
+++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
@@ -2,7 +2,8 @@

* XDMA Controller
Required properties:
-- compatible: Should be "atmel,sama5d4-dma" or "microchip,sam9x60-dma".
+- compatible: Should be "atmel,sama5d4-dma", "microchip,sam9x60-dma" or
+ "microchip,sama7g5-dma".
- reg: Should contain DMA registers location and length.
- interrupts: Should contain DMA interrupt.
- #dma-cells: Must be <1>, used to represent the number of integer cells in
--
2.25.1

2020-09-14 14:16:06

by Eugen Hristev

[permalink] [raw]
Subject: [PATCH 2/7] MAINTAINERS: add dma/at_xdmac_regs.h to XDMAC driver entry

Add new header file for the at_xdmac regs definition to the proper
MAINTAINERS entry.

Signed-off-by: Eugen Hristev <[email protected]>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b5cfab015bd6..312ba6ae5fc7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11361,6 +11361,7 @@ F: Documentation/devicetree/bindings/dma/atmel-dma.txt
F: drivers/dma/at_hdmac.c
F: drivers/dma/at_hdmac_regs.h
F: drivers/dma/at_xdmac.c
+F: drivers/dma/at_xdmac_regs.h
F: include/dt-bindings/dma/at91.h
F: include/linux/platform_data/dma-atmel.h

--
2.25.1

2020-09-23 00:04:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/7] dt-bindings: dmaengine: at_xdmac: add compatible with microchip,sama7g5

On Mon, 14 Sep 2020 17:09:52 +0300, Eugen Hristev wrote:
> Add compatible to sama7g5 SoC.
>
> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/atmel-xdma.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>

Acked-by: Rob Herring <[email protected]>

2020-09-23 00:05:33

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property

On Mon, Sep 14, 2020 at 05:09:55PM +0300, Eugen Hristev wrote:
> Add optional microchip,m2m property that specifies if a controller is
> dedicated to memory to memory operations only.
>
> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/atmel-xdma.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> index 510b7f25ba24..642da6b95a29 100644
> --- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> +++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> @@ -15,6 +15,12 @@ the dmas property of client devices.
> interface identifier,
> - bit 30-24: PERID, peripheral identifier.
>
> +Optional properties:
> +- microchip,m2m: this controller is connected on AXI only to memory and it's
> + dedicated to memory to memory DMA operations. If this option is
> + missing, it's assumed that the DMA controller is connected to
> + peripherals, thus it's a per2mem and mem2per.

Wouldn't 'dma-requests = <0>' cover this case?

Rob

2020-09-23 05:16:49

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 2/7] MAINTAINERS: add dma/at_xdmac_regs.h to XDMAC driver entry

On 9/14/20 5:09 PM, Eugen Hristev wrote:
> Add new header file for the at_xdmac regs definition to the proper
> MAINTAINERS entry.
>
> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b5cfab015bd6..312ba6ae5fc7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11361,6 +11361,7 @@ F: Documentation/devicetree/bindings/dma/atmel-dma.txt
> F: drivers/dma/at_hdmac.c
> F: drivers/dma/at_hdmac_regs.h
> F: drivers/dma/at_xdmac.c
> +F: drivers/dma/at_xdmac_regs.h

A dedicated entry for at_xdmac_regs.h will not be needed,
but still we're here, let's shrink these lines to only one. A change
like the one from below is welcomed:
+F: drivers/dma/at_*

ta

> F: include/dt-bindings/dma/at91.h
> F: include/linux/platform_data/dma-atmel.h
>
>

2020-09-23 05:18:43

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 1/7] dmaengine: at_xdmac: separate register defines into header file

Hi, Eugen,

On 9/14/20 5:09 PM, Eugen Hristev wrote:
> Separate register defines into header file.
> This is required to support a slightly different version of the register
> map in new hardware versions of the XDMAC.
>
> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> drivers/dma/at_xdmac.c | 143 +--------------------------------
> drivers/dma/at_xdmac_regs.h | 154 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 155 insertions(+), 142 deletions(-)
> create mode 100644 drivers/dma/at_xdmac_regs.h

Even with the sama7g5 support there is a single .c file that includes
the .h. I wouldn't split the registers definitions in a dedicated file.

Cheers,
ta

2020-09-23 05:33:15

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 4/7] dmaengine: at_xdmac: adapt perid for mem2mem operations

On 9/14/20 5:09 PM, Eugen Hristev wrote:
> The PERID in the CC register for mem2mem operations must match an unused
> PERID.
> The PERID field is 7 bits, but the selected value is 0x3f.
> On later products we can have more reserved PERIDs for actual peripherals,
> thus this needs to be increased to maximum size.
> Changing the value to 0x7f, which is the maximum for 7 bits field.
>

Maybe it is worth to explain that for memory-to-memory transfers, PERID
should be set to an unused peripheral ID, and the maximum value seems the
safest. Anyway with or without this addressed, one can add:

Reviewed-by: Tudor Ambarus <[email protected]>

> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> drivers/dma/at_xdmac.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index fab19e00a7be..81bb90206092 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -726,7 +726,7 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
> * match the one of another channel. If not, it could lead to spurious
> * flag status.
> */
> - u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
> + u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> | AT_XDMAC_CC_DIF(0)
> | AT_XDMAC_CC_SIF(0)
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> @@ -908,7 +908,7 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
> * match the one of another channel. If not, it could lead to spurious
> * flag status.
> */
> - u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
> + u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> | AT_XDMAC_CC_DAM_INCREMENTED_AM
> | AT_XDMAC_CC_SAM_INCREMENTED_AM
> | AT_XDMAC_CC_DIF(0)
> @@ -1014,7 +1014,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
> * match the one of another channel. If not, it could lead to spurious
> * flag status.
> */
> - u32 chan_cc = AT_XDMAC_CC_PERID(0x3f)
> + u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> | AT_XDMAC_CC_DAM_UBS_AM
> | AT_XDMAC_CC_SAM_INCREMENTED_AM
> | AT_XDMAC_CC_DIF(0)
>

2020-09-23 05:39:36

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 4/7] dmaengine: at_xdmac: adapt perid for mem2mem operations

On 9/23/20 8:30 AM, Tudor Ambarus - M18064 wrote:
> On 9/14/20 5:09 PM, Eugen Hristev wrote:
>> The PERID in the CC register for mem2mem operations must match an unused
>> PERID.
>> The PERID field is 7 bits, but the selected value is 0x3f.
>> On later products we can have more reserved PERIDs for actual peripherals,
>> thus this needs to be increased to maximum size.
>> Changing the value to 0x7f, which is the maximum for 7 bits field.
>>
>
> Maybe it is worth to explain that for memory-to-memory transfers, PERID
> should be set to an unused peripheral ID, and the maximum value seems the
> safest. Anyway with or without this addressed, one can add:
>

:) I somehow misread your commit message, you already described that, it's fine.

> Reviewed-by: Tudor Ambarus <[email protected]>
>
>> Signed-off-by: Eugen Hristev <[email protected]>
>> ---
>> drivers/dma/at_xdmac.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>

2020-09-23 07:09:42

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH 5/7] dmaengine: at_xdmac: add support for sama7g5 based at_xdmac

On 9/14/20 5:09 PM, Eugen Hristev wrote:
> SAMA7G5 SoC uses a slightly different variant of the AT_XDMAC.
> Added support by a new compatible and a layout struct that copes
> to the specific version considering the compatible string.
> Only the differences in register map are present in the layout struct.
> I reworked the register access for this part that has the differences.
> Also the Source/Destination Interface bits are no longer valid for this
> variant of the XDMAC. Thus, the layout also has a bool for specifying
> whether these bits are required or not.
>
> Signed-off-by: Eugen Hristev <[email protected]>
> ---
> drivers/dma/at_xdmac.c | 99 ++++++++++++++++++++++++++++++-------
> drivers/dma/at_xdmac_regs.h | 9 ----
> 2 files changed, 82 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 81bb90206092..874484a4e79f 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -38,6 +38,27 @@ enum atc_status {
> AT_XDMAC_CHAN_IS_PAUSED,
> };
>
> +struct at_xdmac_layout {
> + /* Global Channel Read Suspend Register */
> + u8 grs;
> + /* Global Write Suspend Register */
> + u8 gws;
> + /* Global Channel Read Write Suspend Register */
> + u8 grws;
> + /* Global Channel Read Write Resume Register */
> + u8 grwr;
> + /* Global Channel Software Request Register */
> + u8 gswr;
> + /* Global channel Software Request Status Register */
> + u8 gsws;
> + /* Global Channel Software Flush Request Register */
> + u8 gswf;
> + /* Channel reg base */
> + u8 chan_cc_reg_base;
> + /* Source/Destination Interface must be specified or not */
> + bool sdif;
> +};
> +
> /* ----- Channels ----- */
> struct at_xdmac_chan {
> struct dma_chan chan;
> @@ -71,6 +92,7 @@ struct at_xdmac {
> struct clk *clk;
> u32 save_gim;
> struct dma_pool *at_xdmac_desc_pool;
> + const struct at_xdmac_layout *layout;
> struct at_xdmac_chan chan[];
> };
>
> @@ -103,9 +125,33 @@ struct at_xdmac_desc {
> struct list_head xfer_node;
> } __aligned(sizeof(u64));
>
> +static struct at_xdmac_layout at_xdmac_sama5d4_layout = {

static const struct

> + .grs = 0x28,
> + .gws = 0x2C,
> + .grws = 0x30,
> + .grwr = 0x34,
> + .gswr = 0x38,
> + .gsws = 0x3C,
> + .gswf = 0x40,
> + .chan_cc_reg_base = 0x50,
> + .sdif = true,
> +};
> +
> +static struct at_xdmac_layout at_xdmac_sama7g5_layout = {

static const struct

> + .grs = 0x30,
> + .gws = 0x38,
> + .grws = 0x40,
> + .grwr = 0x44,
> + .gswr = 0x48,
> + .gsws = 0x4C,
> + .gswf = 0x50,
> + .chan_cc_reg_base = 0x60,

one may find these plain offsets as too raw and probably prefer some defines
for them, but I too think that the members of the struct are self-explanatory,
so I'm ok either way.

> + .sdif = false,
> +};
> +
> static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, unsigned int chan_nb)
> {
> - return atxdmac->regs + (AT_XDMAC_CHAN_REG_BASE + chan_nb * 0x40);
> + return atxdmac->regs + (atxdmac->layout->chan_cc_reg_base + chan_nb * 0x40);
> }
>
> #define at_xdmac_read(atxdmac, reg) readl_relaxed((atxdmac)->regs + (reg))
> @@ -204,8 +250,10 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan,
> first->active_xfer = true;
>
> /* Tell xdmac where to get the first descriptor. */
> - reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys)
> - | AT_XDMAC_CNDA_NDAIF(atchan->memif);
> + reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys);
> + if (atxdmac->layout->sdif)
> + reg |= AT_XDMAC_CNDA_NDAIF(atchan->memif);
> +
> at_xdmac_chan_write(atchan, AT_XDMAC_CNDA, reg);
>
> /*
> @@ -400,6 +448,7 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
> enum dma_transfer_direction direction)
> {
> struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> int csize, dwidth;
>
> if (direction == DMA_DEV_TO_MEM) {
> @@ -407,12 +456,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
> AT91_XDMAC_DT_PERID(atchan->perid)
> | AT_XDMAC_CC_DAM_INCREMENTED_AM
> | AT_XDMAC_CC_SAM_FIXED_AM
> - | AT_XDMAC_CC_DIF(atchan->memif)
> - | AT_XDMAC_CC_SIF(atchan->perif)
> | AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> | AT_XDMAC_CC_DSYNC_PER2MEM
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> | AT_XDMAC_CC_TYPE_PER_TRAN;
> + if (atxdmac->layout->sdif)
> + atchan->cfg |= AT_XDMAC_CC_DIF(atchan->memif)
> + | AT_XDMAC_CC_SIF(atchan->perif);

very odd for me to see the "|" operator on the next line. I find it hard to
read and somehow exhausting. I would put it on the first line even if throughout
the driver it's on the next line.

> +
> csize = ffs(atchan->sconfig.src_maxburst) - 1;
> if (csize < 0) {
> dev_err(chan2dev(chan), "invalid src maxburst value\n");
> @@ -430,12 +481,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
> AT91_XDMAC_DT_PERID(atchan->perid)
> | AT_XDMAC_CC_DAM_FIXED_AM
> | AT_XDMAC_CC_SAM_INCREMENTED_AM
> - | AT_XDMAC_CC_DIF(atchan->perif)
> - | AT_XDMAC_CC_SIF(atchan->memif)
> | AT_XDMAC_CC_SWREQ_HWR_CONNECTED
> | AT_XDMAC_CC_DSYNC_MEM2PER
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> | AT_XDMAC_CC_TYPE_PER_TRAN;
> + if (atxdmac->layout->sdif)
> + atchan->cfg |= AT_XDMAC_CC_DIF(atchan->perif)
> + | AT_XDMAC_CC_SIF(atchan->memif);
> +
> csize = ffs(atchan->sconfig.dst_maxburst) - 1;
> if (csize < 0) {
> dev_err(chan2dev(chan), "invalid src maxburst value\n");
> @@ -711,6 +764,7 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
> struct data_chunk *chunk)
> {
> struct at_xdmac_desc *desc;
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> u32 dwidth;
> unsigned long flags;
> size_t ublen;
> @@ -727,10 +781,10 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
> * flag status.
> */
> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> - | AT_XDMAC_CC_DIF(0)
> - | AT_XDMAC_CC_SIF(0)
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> | AT_XDMAC_CC_TYPE_MEM_TRAN;
> + if (atxdmac->layout->sdif)
> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);

there is a comment above chan_cc init that explains that we have to use
interface 0 for both source and destination, so maybe we can get rid of
this explicit OR with zero and update the comment for sama7g5 case.

>
> dwidth = at_xdmac_align_width(chan, src | dst | chunk->size);
> if (chunk->size >= (AT_XDMAC_MBR_UBC_UBLEN_MAX << dwidth)) {
> @@ -893,6 +947,7 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
> size_t len, unsigned long flags)
> {
> struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> struct at_xdmac_desc *first = NULL, *prev = NULL;
> size_t remaining_size = len, xfer_size = 0, ublen;
> dma_addr_t src_addr = src, dst_addr = dest;
> @@ -911,12 +966,13 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> | AT_XDMAC_CC_DAM_INCREMENTED_AM
> | AT_XDMAC_CC_SAM_INCREMENTED_AM
> - | AT_XDMAC_CC_DIF(0)
> - | AT_XDMAC_CC_SIF(0)
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> | AT_XDMAC_CC_TYPE_MEM_TRAN;
> unsigned long irqflags;
>
> + if (atxdmac->layout->sdif)
> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);

same here

> +
> dev_dbg(chan2dev(chan), "%s: src=%pad, dest=%pad, len=%zd, flags=0x%lx\n",
> __func__, &src, &dest, len, flags);
>
> @@ -999,6 +1055,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
> int value)
> {
> struct at_xdmac_desc *desc;
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> unsigned long flags;
> size_t ublen;
> u32 dwidth;
> @@ -1017,11 +1074,11 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
> | AT_XDMAC_CC_DAM_UBS_AM
> | AT_XDMAC_CC_SAM_INCREMENTED_AM
> - | AT_XDMAC_CC_DIF(0)
> - | AT_XDMAC_CC_SIF(0)
> | AT_XDMAC_CC_MBSIZE_SIXTEEN
> | AT_XDMAC_CC_MEMSET_HW_MODE
> | AT_XDMAC_CC_TYPE_MEM_TRAN;
> + if (atxdmac->layout->sdif)
> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);

same here

>
> dwidth = at_xdmac_align_width(chan, dst_addr);
>
> @@ -1297,7 +1354,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> mask = AT_XDMAC_CC_TYPE | AT_XDMAC_CC_DSYNC;
> value = AT_XDMAC_CC_TYPE_PER_TRAN | AT_XDMAC_CC_DSYNC_PER2MEM;
> if ((desc->lld.mbr_cfg & mask) == value) {
> - at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
> + at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
> while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
> cpu_relax();
> }
> @@ -1355,7 +1412,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
> * FIFO flush ensures that data are really written.
> */
> if ((desc->lld.mbr_cfg & mask) == value) {
> - at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
> + at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
> while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
> cpu_relax();
> }
> @@ -1620,7 +1677,7 @@ static int at_xdmac_device_pause(struct dma_chan *chan)
> return 0;
>
> spin_lock_irqsave(&atchan->lock, flags);
> - at_xdmac_write(atxdmac, AT_XDMAC_GRWS, atchan->mask);
> + at_xdmac_write(atxdmac, atxdmac->layout->grws, atchan->mask);
> while (at_xdmac_chan_read(atchan, AT_XDMAC_CC)
> & (AT_XDMAC_CC_WRIP | AT_XDMAC_CC_RDIP))
> cpu_relax();
> @@ -1643,7 +1700,7 @@ static int at_xdmac_device_resume(struct dma_chan *chan)
> return 0;
> }
>
> - at_xdmac_write(atxdmac, AT_XDMAC_GRWR, atchan->mask);
> + at_xdmac_write(atxdmac, atxdmac->layout->grwr, atchan->mask);
> clear_bit(AT_XDMAC_CHAN_IS_PAUSED, &atchan->status);
> spin_unlock_irqrestore(&atchan->lock, flags);
>
> @@ -1845,6 +1902,10 @@ static int at_xdmac_probe(struct platform_device *pdev)
> atxdmac->regs = base;
> atxdmac->irq = irq;
>
> + atxdmac->layout = of_device_get_match_data(&pdev->dev);
> + if (!atxdmac->layout)
> + return -ENODEV;

I would get the data upper in the function, after getting irq. If data is
not provided, you would spare some ops that will be done gratuitously.

With these addressed one may add:
Reviewed-by: Tudor Ambarus <[email protected]>
> +
> atxdmac->clk = devm_clk_get(&pdev->dev, "dma_clk");
> if (IS_ERR(atxdmac->clk)) {
> dev_err(&pdev->dev, "can't get dma_clk\n");
> @@ -1988,6 +2049,10 @@ static const struct dev_pm_ops atmel_xdmac_dev_pm_ops = {
> static const struct of_device_id atmel_xdmac_dt_ids[] = {
> {
> .compatible = "atmel,sama5d4-dma",
> + .data = &at_xdmac_sama5d4_layout,
> + }, {
> + .compatible = "microchip,sama7g5-dma",
> + .data = &at_xdmac_sama7g5_layout,
> }, {
> /* sentinel */
> }
> diff --git a/drivers/dma/at_xdmac_regs.h b/drivers/dma/at_xdmac_regs.h
> index 3f7dda4c5743..7b4b4e24de70 100644
> --- a/drivers/dma/at_xdmac_regs.h
> +++ b/drivers/dma/at_xdmac_regs.h
> @@ -22,13 +22,6 @@
> #define AT_XDMAC_GE 0x1C /* Global Channel Enable Register */
> #define AT_XDMAC_GD 0x20 /* Global Channel Disable Register */
> #define AT_XDMAC_GS 0x24 /* Global Channel Status Register */
> -#define AT_XDMAC_GRS 0x28 /* Global Channel Read Suspend Register */
> -#define AT_XDMAC_GWS 0x2C /* Global Write Suspend Register */
> -#define AT_XDMAC_GRWS 0x30 /* Global Channel Read Write Suspend Register */
> -#define AT_XDMAC_GRWR 0x34 /* Global Channel Read Write Resume Register */
> -#define AT_XDMAC_GSWR 0x38 /* Global Channel Software Request Register */
> -#define AT_XDMAC_GSWS 0x3C /* Global channel Software Request Status Register */
> -#define AT_XDMAC_GSWF 0x40 /* Global Channel Software Flush Request Register */
> #define AT_XDMAC_VERSION 0xFFC /* XDMAC Version Register */
>
> /* Channel relative registers offsets */
> @@ -134,8 +127,6 @@
> #define AT_XDMAC_CSUS 0x30 /* Channel Source Microblock Stride */
> #define AT_XDMAC_CDUS 0x34 /* Channel Destination Microblock Stride */
>
> -#define AT_XDMAC_CHAN_REG_BASE 0x50 /* Channel registers base address */
> -
> /* Microblock control members */
> #define AT_XDMAC_MBR_UBC_UBLEN_MAX 0xFFFFFFUL /* Maximum Microblock Length */
> #define AT_XDMAC_MBR_UBC_NDE (0x1 << 24) /* Next Descriptor Enable */
>

2020-10-16 06:55:34

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property

On 23.09.2020 02:33, Rob Herring wrote:

> On Mon, Sep 14, 2020 at 05:09:55PM +0300, Eugen Hristev wrote:
>> Add optional microchip,m2m property that specifies if a controller is
>> dedicated to memory to memory operations only.
>>
>> Signed-off-by: Eugen Hristev <[email protected]>
>> ---
>> Documentation/devicetree/bindings/dma/atmel-xdma.txt | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>> index 510b7f25ba24..642da6b95a29 100644
>> --- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>> +++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>> @@ -15,6 +15,12 @@ the dmas property of client devices.
>> interface identifier,
>> - bit 30-24: PERID, peripheral identifier.
>>
>> +Optional properties:
>> +- microchip,m2m: this controller is connected on AXI only to memory and it's
>> + dedicated to memory to memory DMA operations. If this option is
>> + missing, it's assumed that the DMA controller is connected to
>> + peripherals, thus it's a per2mem and mem2per.
>
> Wouldn't 'dma-requests = <0>' cover this case?
>
> Rob
>

Hi Rob,

I do not think so. With requests = 0, it means that actually the DMA
controller is unusable ?
Since you suggest requests = 0, it means that it cannot take requests at
all ?
I do not find another example in current DT with this property set to zero.

Eugen

2020-10-16 07:11:56

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property

On 16.10.2020 10:06, Vinod Koul wrote:
> Hi Eugen,
>
> On 16-10-20, 06:45, [email protected] wrote:
>> On 23.09.2020 02:33, Rob Herring wrote:
>>
>>> On Mon, Sep 14, 2020 at 05:09:55PM +0300, Eugen Hristev wrote:
>>>> Add optional microchip,m2m property that specifies if a controller is
>>>> dedicated to memory to memory operations only.
>>>>
>>>> Signed-off-by: Eugen Hristev <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/dma/atmel-xdma.txt | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>>>> index 510b7f25ba24..642da6b95a29 100644
>>>> --- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>>>> +++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
>>>> @@ -15,6 +15,12 @@ the dmas property of client devices.
>>>> interface identifier,
>>>> - bit 30-24: PERID, peripheral identifier.
>>>>
>>>> +Optional properties:
>>>> +- microchip,m2m: this controller is connected on AXI only to memory and it's
>>>> + dedicated to memory to memory DMA operations. If this option is
>>>> + missing, it's assumed that the DMA controller is connected to
>>>> + peripherals, thus it's a per2mem and mem2per.
>>>
>>> Wouldn't 'dma-requests = <0>' cover this case?
>>>
>>> Rob
>>>
>>
>> Hi Rob,
>>
>> I do not think so. With requests = 0, it means that actually the DMA
>> controller is unusable ?
>> Since you suggest requests = 0, it means that it cannot take requests at
>> all ?
>> I do not find another example in current DT with this property set to zero.
>
> Not really, dma-requests implies "request signals supported" which are
> used for peripheral cases. m2m does not need request signals, so it is
> very reasonable to conclude that dma-requests = <0> would imply no
> peripheral support and only m2m support.

Thanks for explaining, I will change accordingly then.

Eugen

>
> Thanks
> --
> ~Vinod
>

2020-10-16 09:01:05

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH 5/7] dmaengine: at_xdmac: add support for sama7g5 based at_xdmac

On 23.09.2020 10:08, Tudor Ambarus - M18064 wrote:
> On 9/14/20 5:09 PM, Eugen Hristev wrote:
>> SAMA7G5 SoC uses a slightly different variant of the AT_XDMAC.
>> Added support by a new compatible and a layout struct that copes
>> to the specific version considering the compatible string.
>> Only the differences in register map are present in the layout struct.
>> I reworked the register access for this part that has the differences.
>> Also the Source/Destination Interface bits are no longer valid for this
>> variant of the XDMAC. Thus, the layout also has a bool for specifying
>> whether these bits are required or not.
>>
>> Signed-off-by: Eugen Hristev <[email protected]>
>> ---
>> drivers/dma/at_xdmac.c | 99 ++++++++++++++++++++++++++++++-------
>> drivers/dma/at_xdmac_regs.h | 9 ----
>> 2 files changed, 82 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
>> index 81bb90206092..874484a4e79f 100644
>> --- a/drivers/dma/at_xdmac.c
>> +++ b/drivers/dma/at_xdmac.c
>> @@ -38,6 +38,27 @@ enum atc_status {
>> AT_XDMAC_CHAN_IS_PAUSED,
>> };
>>
>> +struct at_xdmac_layout {
>> + /* Global Channel Read Suspend Register */
>> + u8 grs;
>> + /* Global Write Suspend Register */
>> + u8 gws;
>> + /* Global Channel Read Write Suspend Register */
>> + u8 grws;
>> + /* Global Channel Read Write Resume Register */
>> + u8 grwr;
>> + /* Global Channel Software Request Register */
>> + u8 gswr;
>> + /* Global channel Software Request Status Register */
>> + u8 gsws;
>> + /* Global Channel Software Flush Request Register */
>> + u8 gswf;
>> + /* Channel reg base */
>> + u8 chan_cc_reg_base;
>> + /* Source/Destination Interface must be specified or not */
>> + bool sdif;
>> +};
>> +
>> /* ----- Channels ----- */
>> struct at_xdmac_chan {
>> struct dma_chan chan;
>> @@ -71,6 +92,7 @@ struct at_xdmac {
>> struct clk *clk;
>> u32 save_gim;
>> struct dma_pool *at_xdmac_desc_pool;
>> + const struct at_xdmac_layout *layout;
>> struct at_xdmac_chan chan[];
>> };
>>
>> @@ -103,9 +125,33 @@ struct at_xdmac_desc {
>> struct list_head xfer_node;
>> } __aligned(sizeof(u64));
>>
>> +static struct at_xdmac_layout at_xdmac_sama5d4_layout = {
>
> static const struct
>
>> + .grs = 0x28,
>> + .gws = 0x2C,
>> + .grws = 0x30,
>> + .grwr = 0x34,
>> + .gswr = 0x38,
>> + .gsws = 0x3C,
>> + .gswf = 0x40,
>> + .chan_cc_reg_base = 0x50,
>> + .sdif = true,
>> +};
>> +
>> +static struct at_xdmac_layout at_xdmac_sama7g5_layout = {
>
> static const struct
>
>> + .grs = 0x30,
>> + .gws = 0x38,
>> + .grws = 0x40,
>> + .grwr = 0x44,
>> + .gswr = 0x48,
>> + .gsws = 0x4C,
>> + .gswf = 0x50,
>> + .chan_cc_reg_base = 0x60,
>
> one may find these plain offsets as too raw and probably prefer some defines
> for them, but I too think that the members of the struct are self-explanatory,
> so I'm ok either way.
>
>> + .sdif = false,
>> +};
>> +
>> static inline void __iomem *at_xdmac_chan_reg_base(struct at_xdmac *atxdmac, unsigned int chan_nb)
>> {
>> - return atxdmac->regs + (AT_XDMAC_CHAN_REG_BASE + chan_nb * 0x40);
>> + return atxdmac->regs + (atxdmac->layout->chan_cc_reg_base + chan_nb * 0x40);
>> }
>>
>> #define at_xdmac_read(atxdmac, reg) readl_relaxed((atxdmac)->regs + (reg))
>> @@ -204,8 +250,10 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan,
>> first->active_xfer = true;
>>
>> /* Tell xdmac where to get the first descriptor. */
>> - reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys)
>> - | AT_XDMAC_CNDA_NDAIF(atchan->memif);
>> + reg = AT_XDMAC_CNDA_NDA(first->tx_dma_desc.phys);
>> + if (atxdmac->layout->sdif)
>> + reg |= AT_XDMAC_CNDA_NDAIF(atchan->memif);
>> +
>> at_xdmac_chan_write(atchan, AT_XDMAC_CNDA, reg);
>>
>> /*
>> @@ -400,6 +448,7 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
>> enum dma_transfer_direction direction)
>> {
>> struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
>> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
>> int csize, dwidth;
>>
>> if (direction == DMA_DEV_TO_MEM) {
>> @@ -407,12 +456,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
>> AT91_XDMAC_DT_PERID(atchan->perid)
>> | AT_XDMAC_CC_DAM_INCREMENTED_AM
>> | AT_XDMAC_CC_SAM_FIXED_AM
>> - | AT_XDMAC_CC_DIF(atchan->memif)
>> - | AT_XDMAC_CC_SIF(atchan->perif)
>> | AT_XDMAC_CC_SWREQ_HWR_CONNECTED
>> | AT_XDMAC_CC_DSYNC_PER2MEM
>> | AT_XDMAC_CC_MBSIZE_SIXTEEN
>> | AT_XDMAC_CC_TYPE_PER_TRAN;
>> + if (atxdmac->layout->sdif)
>> + atchan->cfg |= AT_XDMAC_CC_DIF(atchan->memif)
>> + | AT_XDMAC_CC_SIF(atchan->perif);
>
> very odd for me to see the "|" operator on the next line. I find it hard to
> read and somehow exhausting. I would put it on the first line even if throughout
> the driver it's on the next line.
>
>> +
>> csize = ffs(atchan->sconfig.src_maxburst) - 1;
>> if (csize < 0) {
>> dev_err(chan2dev(chan), "invalid src maxburst value\n");
>> @@ -430,12 +481,14 @@ static int at_xdmac_compute_chan_conf(struct dma_chan *chan,
>> AT91_XDMAC_DT_PERID(atchan->perid)
>> | AT_XDMAC_CC_DAM_FIXED_AM
>> | AT_XDMAC_CC_SAM_INCREMENTED_AM
>> - | AT_XDMAC_CC_DIF(atchan->perif)
>> - | AT_XDMAC_CC_SIF(atchan->memif)
>> | AT_XDMAC_CC_SWREQ_HWR_CONNECTED
>> | AT_XDMAC_CC_DSYNC_MEM2PER
>> | AT_XDMAC_CC_MBSIZE_SIXTEEN
>> | AT_XDMAC_CC_TYPE_PER_TRAN;
>> + if (atxdmac->layout->sdif)
>> + atchan->cfg |= AT_XDMAC_CC_DIF(atchan->perif)
>> + | AT_XDMAC_CC_SIF(atchan->memif);
>> +
>> csize = ffs(atchan->sconfig.dst_maxburst) - 1;
>> if (csize < 0) {
>> dev_err(chan2dev(chan), "invalid src maxburst value\n");
>> @@ -711,6 +764,7 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
>> struct data_chunk *chunk)
>> {
>> struct at_xdmac_desc *desc;
>> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
>> u32 dwidth;
>> unsigned long flags;
>> size_t ublen;
>> @@ -727,10 +781,10 @@ at_xdmac_interleaved_queue_desc(struct dma_chan *chan,
>> * flag status.
>> */
>> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
>> - | AT_XDMAC_CC_DIF(0)
>> - | AT_XDMAC_CC_SIF(0)
>> | AT_XDMAC_CC_MBSIZE_SIXTEEN
>> | AT_XDMAC_CC_TYPE_MEM_TRAN;
>> + if (atxdmac->layout->sdif)
>> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);
>
> there is a comment above chan_cc init that explains that we have to use
> interface 0 for both source and destination, so maybe we can get rid of
> this explicit OR with zero and update the comment for sama7g5 case.
>
>>
>> dwidth = at_xdmac_align_width(chan, src | dst | chunk->size);
>> if (chunk->size >= (AT_XDMAC_MBR_UBC_UBLEN_MAX << dwidth)) {
>> @@ -893,6 +947,7 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>> size_t len, unsigned long flags)
>> {
>> struct at_xdmac_chan *atchan = to_at_xdmac_chan(chan);
>> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
>> struct at_xdmac_desc *first = NULL, *prev = NULL;
>> size_t remaining_size = len, xfer_size = 0, ublen;
>> dma_addr_t src_addr = src, dst_addr = dest;
>> @@ -911,12 +966,13 @@ at_xdmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
>> | AT_XDMAC_CC_DAM_INCREMENTED_AM
>> | AT_XDMAC_CC_SAM_INCREMENTED_AM
>> - | AT_XDMAC_CC_DIF(0)
>> - | AT_XDMAC_CC_SIF(0)
>> | AT_XDMAC_CC_MBSIZE_SIXTEEN
>> | AT_XDMAC_CC_TYPE_MEM_TRAN;
>> unsigned long irqflags;
>>
>> + if (atxdmac->layout->sdif)
>> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);
>
> same here
>
>> +
>> dev_dbg(chan2dev(chan), "%s: src=%pad, dest=%pad, len=%zd, flags=0x%lx\n",
>> __func__, &src, &dest, len, flags);
>>
>> @@ -999,6 +1055,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
>> int value)
>> {
>> struct at_xdmac_desc *desc;
>> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
>> unsigned long flags;
>> size_t ublen;
>> u32 dwidth;
>> @@ -1017,11 +1074,11 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,
>> u32 chan_cc = AT_XDMAC_CC_PERID(0x7f)
>> | AT_XDMAC_CC_DAM_UBS_AM
>> | AT_XDMAC_CC_SAM_INCREMENTED_AM
>> - | AT_XDMAC_CC_DIF(0)
>> - | AT_XDMAC_CC_SIF(0)
>> | AT_XDMAC_CC_MBSIZE_SIXTEEN
>> | AT_XDMAC_CC_MEMSET_HW_MODE
>> | AT_XDMAC_CC_TYPE_MEM_TRAN;
>> + if (atxdmac->layout->sdif)
>> + chan_cc |= AT_XDMAC_CC_DIF(0) | AT_XDMAC_CC_SIF(0);
>
> same here
>
>>
>> dwidth = at_xdmac_align_width(chan, dst_addr);
>>
>> @@ -1297,7 +1354,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>> mask = AT_XDMAC_CC_TYPE | AT_XDMAC_CC_DSYNC;
>> value = AT_XDMAC_CC_TYPE_PER_TRAN | AT_XDMAC_CC_DSYNC_PER2MEM;
>> if ((desc->lld.mbr_cfg & mask) == value) {
>> - at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
>> + at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
>> while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
>> cpu_relax();
>> }
>> @@ -1355,7 +1412,7 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>> * FIFO flush ensures that data are really written.
>> */
>> if ((desc->lld.mbr_cfg & mask) == value) {
>> - at_xdmac_write(atxdmac, AT_XDMAC_GSWF, atchan->mask);
>> + at_xdmac_write(atxdmac, atxdmac->layout->gswf, atchan->mask);
>> while (!(at_xdmac_chan_read(atchan, AT_XDMAC_CIS) & AT_XDMAC_CIS_FIS))
>> cpu_relax();
>> }
>> @@ -1620,7 +1677,7 @@ static int at_xdmac_device_pause(struct dma_chan *chan)
>> return 0;
>>
>> spin_lock_irqsave(&atchan->lock, flags);
>> - at_xdmac_write(atxdmac, AT_XDMAC_GRWS, atchan->mask);
>> + at_xdmac_write(atxdmac, atxdmac->layout->grws, atchan->mask);
>> while (at_xdmac_chan_read(atchan, AT_XDMAC_CC)
>> & (AT_XDMAC_CC_WRIP | AT_XDMAC_CC_RDIP))
>> cpu_relax();
>> @@ -1643,7 +1700,7 @@ static int at_xdmac_device_resume(struct dma_chan *chan)
>> return 0;
>> }
>>
>> - at_xdmac_write(atxdmac, AT_XDMAC_GRWR, atchan->mask);
>> + at_xdmac_write(atxdmac, atxdmac->layout->grwr, atchan->mask);
>> clear_bit(AT_XDMAC_CHAN_IS_PAUSED, &atchan->status);
>> spin_unlock_irqrestore(&atchan->lock, flags);
>>
>> @@ -1845,6 +1902,10 @@ static int at_xdmac_probe(struct platform_device *pdev)
>> atxdmac->regs = base;
>> atxdmac->irq = irq;
>>
>> + atxdmac->layout = of_device_get_match_data(&pdev->dev);
>> + if (!atxdmac->layout)
>> + return -ENODEV;
>
> I would get the data upper in the function, after getting irq. If data is
> not provided, you would spare some ops that will be done gratuitously.

Hi Tudor,

This would be difficult to do as atxdmac is allocated just a few lines
before.
Also, actually the get_match_data should not fail normally. If this
would fail it would mean that the driver is probed to a wrong driver
compatible... which should not happen.

Thanks for reviewing. I am sending v2.

Eugen

>
> With these addressed one may add:
> Reviewed-by: Tudor Ambarus <[email protected]>
>> +
>> atxdmac->clk = devm_clk_get(&pdev->dev, "dma_clk");
>> if (IS_ERR(atxdmac->clk)) {
>> dev_err(&pdev->dev, "can't get dma_clk\n");
>> @@ -1988,6 +2049,10 @@ static const struct dev_pm_ops atmel_xdmac_dev_pm_ops = {
>> static const struct of_device_id atmel_xdmac_dt_ids[] = {
>> {
>> .compatible = "atmel,sama5d4-dma",
>> + .data = &at_xdmac_sama5d4_layout,
>> + }, {
>> + .compatible = "microchip,sama7g5-dma",
>> + .data = &at_xdmac_sama7g5_layout,
>> }, {
>> /* sentinel */
>> }
>> diff --git a/drivers/dma/at_xdmac_regs.h b/drivers/dma/at_xdmac_regs.h
>> index 3f7dda4c5743..7b4b4e24de70 100644
>> --- a/drivers/dma/at_xdmac_regs.h
>> +++ b/drivers/dma/at_xdmac_regs.h
>> @@ -22,13 +22,6 @@
>> #define AT_XDMAC_GE 0x1C /* Global Channel Enable Register */
>> #define AT_XDMAC_GD 0x20 /* Global Channel Disable Register */
>> #define AT_XDMAC_GS 0x24 /* Global Channel Status Register */
>> -#define AT_XDMAC_GRS 0x28 /* Global Channel Read Suspend Register */
>> -#define AT_XDMAC_GWS 0x2C /* Global Write Suspend Register */
>> -#define AT_XDMAC_GRWS 0x30 /* Global Channel Read Write Suspend Register */
>> -#define AT_XDMAC_GRWR 0x34 /* Global Channel Read Write Resume Register */
>> -#define AT_XDMAC_GSWR 0x38 /* Global Channel Software Request Register */
>> -#define AT_XDMAC_GSWS 0x3C /* Global channel Software Request Status Register */
>> -#define AT_XDMAC_GSWF 0x40 /* Global Channel Software Flush Request Register */
>> #define AT_XDMAC_VERSION 0xFFC /* XDMAC Version Register */
>>
>> /* Channel relative registers offsets */
>> @@ -134,8 +127,6 @@
>> #define AT_XDMAC_CSUS 0x30 /* Channel Source Microblock Stride */
>> #define AT_XDMAC_CDUS 0x34 /* Channel Destination Microblock Stride */
>>
>> -#define AT_XDMAC_CHAN_REG_BASE 0x50 /* Channel registers base address */
>> -
>> /* Microblock control members */
>> #define AT_XDMAC_MBR_UBC_UBLEN_MAX 0xFFFFFFUL /* Maximum Microblock Length */
>> #define AT_XDMAC_MBR_UBC_NDE (0x1 << 24) /* Next Descriptor Enable */
>>
>

2020-10-16 09:02:10

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 6/7] dt-bindings: dmaengine: at_xdmac: add optional microchip,m2m property

Hi Eugen,

On 16-10-20, 06:45, [email protected] wrote:
> On 23.09.2020 02:33, Rob Herring wrote:
>
> > On Mon, Sep 14, 2020 at 05:09:55PM +0300, Eugen Hristev wrote:
> >> Add optional microchip,m2m property that specifies if a controller is
> >> dedicated to memory to memory operations only.
> >>
> >> Signed-off-by: Eugen Hristev <[email protected]>
> >> ---
> >> Documentation/devicetree/bindings/dma/atmel-xdma.txt | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/dma/atmel-xdma.txt b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> >> index 510b7f25ba24..642da6b95a29 100644
> >> --- a/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> >> +++ b/Documentation/devicetree/bindings/dma/atmel-xdma.txt
> >> @@ -15,6 +15,12 @@ the dmas property of client devices.
> >> interface identifier,
> >> - bit 30-24: PERID, peripheral identifier.
> >>
> >> +Optional properties:
> >> +- microchip,m2m: this controller is connected on AXI only to memory and it's
> >> + dedicated to memory to memory DMA operations. If this option is
> >> + missing, it's assumed that the DMA controller is connected to
> >> + peripherals, thus it's a per2mem and mem2per.
> >
> > Wouldn't 'dma-requests = <0>' cover this case?
> >
> > Rob
> >
>
> Hi Rob,
>
> I do not think so. With requests = 0, it means that actually the DMA
> controller is unusable ?
> Since you suggest requests = 0, it means that it cannot take requests at
> all ?
> I do not find another example in current DT with this property set to zero.

Not really, dma-requests implies "request signals supported" which are
used for peripheral cases. m2m does not need request signals, so it is
very reasonable to conclude that dma-requests = <0> would imply no
peripheral support and only m2m support.

Thanks
--
~Vinod