Patchwork [RFC,v2,1/5] dma: mpc512x: re-order mpc8308 specific instructions

login
register
mail settings
Submitter Gerhard Sittig
Date July 14, 2013, 12:01 p.m.
Message ID <1373803321-11628-2-git-send-email-gsi@denx.de>
Download mbox | patch
Permalink /patch/258898/
State Superseded
Delegated to: Anatolij Gustschin
Headers show

Comments

Gerhard Sittig - July 14, 2013, 12:01 p.m.
it's rather unexpected to have the MPC8308 specific code in the 'else'
branch so distant from the "is MPC8308?" check

concentrate the test and the specific code for MPC8308 in the 'if'
branch and handle MPC512x in the 'else' branch; use a symbolic channel
count which in combination with the re-ordering obsoletes a comment

this modification only re-orders instructions but doesn't change behaviour

Signed-off-by: Gerhard Sittig <gsi@denx.de>
---
 drivers/dma/mpc512x_dma.c |   48 +++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 17 deletions(-)
Alexander Popov - Aug. 12, 2013, 1:38 p.m.
Hello everyone!

2013/7/14 Gerhard Sittig <gsi@denx.de>:
> @@ -50,9 +50,23 @@
>  #define MPC_DMA_DESCRIPTORS    64
>
>  /* Macro definitions */
> -#define MPC_DMA_CHANNELS       64
>  #define MPC_DMA_TCD_OFFSET     0x1000
>
> +/*
> + * the maximum channel count, and specific channels which need
> + * special processing, for individual hardware variants
> + *
> + * and the maximum channel count over all supported controllers,
> + * used for data structure sizes
> + */
> +enum mpc8308_dmachan_id_t {
> +       MPC8308_DMACHAN_MAX = 16,
> +};
> +enum mpc512x_dmachan_id_t {
> +       MPC512x_DMACHAN_MAX = 64,
> +};
> +#define MPC_DMA_CHANNELS       64
> +

I offer to use #define instead of enum here
since individual channels don't require special handling.

Best regards,
Alexander.

Patch

diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2d95673..4e03f1e 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -50,9 +50,23 @@ 
 #define MPC_DMA_DESCRIPTORS	64
 
 /* Macro definitions */
-#define MPC_DMA_CHANNELS	64
 #define MPC_DMA_TCD_OFFSET	0x1000
 
+/*
+ * the maximum channel count, and specific channels which need
+ * special processing, for individual hardware variants
+ *
+ * and the maximum channel count over all supported controllers,
+ * used for data structure sizes
+ */
+enum mpc8308_dmachan_id_t {
+	MPC8308_DMACHAN_MAX = 16,
+};
+enum mpc512x_dmachan_id_t {
+	MPC512x_DMACHAN_MAX = 64,
+};
+#define MPC_DMA_CHANNELS	64
+
 /* Arbitration mode of group and channel */
 #define MPC_DMA_DMACR_EDCG	(1 << 31)
 #define MPC_DMA_DMACR_ERGA	(1 << 3)
@@ -716,10 +730,10 @@  static int mpc_dma_probe(struct platform_device *op)
 
 	dma = &mdma->dma;
 	dma->dev = dev;
-	if (!mdma->is_mpc8308)
-		dma->chancnt = MPC_DMA_CHANNELS;
+	if (mdma->is_mpc8308)
+		dma->chancnt = MPC8308_DMACHAN_MAX;
 	else
-		dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */
+		dma->chancnt = MPC512x_DMACHAN_MAX;
 	dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources;
 	dma->device_free_chan_resources = mpc_dma_free_chan_resources;
 	dma->device_issue_pending = mpc_dma_issue_pending;
@@ -753,7 +767,19 @@  static int mpc_dma_probe(struct platform_device *op)
 	 * - Round-robin group arbitration,
 	 * - Round-robin channel arbitration.
 	 */
-	if (!mdma->is_mpc8308) {
+	if (mdma->is_mpc8308) {
+		/* MPC8308 has 16 channels and lacks some registers */
+		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
+
+		/* enable snooping */
+		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
+		/* Disable error interrupts */
+		out_be32(&mdma->regs->dmaeeil, 0);
+
+		/* Clear interrupts status */
+		out_be32(&mdma->regs->dmaintl, 0xFFFF);
+		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
+	} else {
 		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG |
 					MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA);
 
@@ -774,18 +800,6 @@  static int mpc_dma_probe(struct platform_device *op)
 		/* Route interrupts to IPIC */
 		out_be32(&mdma->regs->dmaihsa, 0);
 		out_be32(&mdma->regs->dmailsa, 0);
-	} else {
-		/* MPC8308 has 16 channels and lacks some registers */
-		out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA);
-
-		/* enable snooping */
-		out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE);
-		/* Disable error interrupts */
-		out_be32(&mdma->regs->dmaeeil, 0);
-
-		/* Clear interrupts status */
-		out_be32(&mdma->regs->dmaintl, 0xFFFF);
-		out_be32(&mdma->regs->dmaerrl, 0xFFFF);
 	}
 
 	/* Register DMA engine */