dma: sudmac: add support for SUDMAC
PATCH v4 1/1 dma: sudmac: add support for SUDMAC
Some Renesas USB modules have SUDMAC. This patch supports it using the shdma-base driver.
---
about v4:
- rebase this patch on the current remotes/origin/next branch of slave-dma.git.
- remove __devinit, __devexit and __devexit_p.
drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/sh/Makefile | 1 + drivers/dma/sh/sudmac.c | 368 +++++++++++++++++++++++++++++++++++++++++++++++ drivers/dma/sh/sudmac.h | 61 ++++++++ include/linux/sudmac.h | 55 +++++++ 6 files changed, 494 insertions(+), 0 deletions(-) create mode 100644 drivers/dma/sh/sudmac.c create mode 100644 drivers/dma/sh/sudmac.h create mode 100644 include/linux/sudmac.h
drivers/dma/Kconfig +8 -0
@@ -183,6 +183,14 @@ config SH_DMAE help Enable support for the Renesas SuperH DMA controllers. +config SUDMAC + tristate "Renesas SUDMAC support" + depends on (SUPERH && SH_DMA) || (ARM && ARCH_SHMOBILE) + depends on !SH_DMA_API + select DMA_ENGINE + help + Enable support for the Renesas SUDMAC controllers. + config COH901318 bool "ST-Ericsson COH901318 DMA support" select DMA_ENGINE
drivers/dma/Makefile +1 -0
@@ -19,6 +19,7 @@ obj-$(CONFIG_AT_HDMAC) += at_hdmac.o obj-$(CONFIG_MX3_IPU) += ipu/ obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o obj-$(CONFIG_SH_DMAE) += sh/ +obj-$(CONFIG_SUDMAC) += sh/ obj-$(CONFIG_COH901318) += coh901318.o coh901318_lli.o obj-$(CONFIG_AMCC_PPC440SPE_ADMA) += ppc4xx/ obj-$(CONFIG_IMX_SDMA) += imx-sdma.o
drivers/dma/sh/Makefile +1 -0
@@ -1,2 +1,3 @@ obj-$(CONFIG_SH_DMAE) += shdma-base.o obj-$(CONFIG_SH_DMAE) += shdma.o +obj-$(CONFIG_SUDMAC) += shdma-base.o sudmac.o
drivers/dma/sh/sudmac.c new file +368 -0
368 lines not on this page · raw message
drivers/dma/sh/sudmac.h new file +61 -0
61 lines not on this page · raw message
include/linux/sudmac.h new file +55 -0
55 lines not on this page · raw message
Signature · 1 line
1.7.1
Quoted 5 lines · Yoshihiro
Some Renesas USB modules have SUDMAC. This patch supports it using the shdma-base driver.
Signed-off-by Yoshihiro Shimoda <[email protected]>Reviewed-by Guennadi Liakhovetski <[email protected]>
...
Quoted 2 lines
drivers/dma/sh/sudmac.h new file +2
-0+#define SUDMAC_MAX_CHANNELS 2 +
...
Quoted 7 lines
drivers/dma/sh/sudmac.h new file +7
-0+struct sudmac_device { +struct shdma_dev shdma_dev; +struct sudmac_chan *chan[SUDMAC_MAX_CHANNELS ]; +struct sudmac_pdata *pdata; +void __iomem *chan_reg; +}; +
...
include/linux/sudmac.h line 45
Quoted 7 lines
include/linux/sudmac.h new file +7
-0+struct sudmac_pdata { +const struct sudmac_slave_config *slave; +int slave_num; +const struct sudmac_channel *channel; +int channel_num; +}; +
I'm a bit perplexed as to why you have a hardcoded max channel count when it can all be derived from the platform data in the first place?
Hi Paul,
Quoted 22 lines · Paul Mundt
Quoted 5 lines · Yoshihiro
Some Renesas USB modules have SUDMAC. This patch supports it using the shdma-base driver.
Signed-off-by Yoshihiro Shimoda <[email protected]>Reviewed-by Guennadi Liakhovetski <[email protected]>...
Quoted 2 lines
drivers/dma/sh/sudmac.h new file +2
-0+#define SUDMAC_MAX_CHANNELS 2 +...
Quoted 7 lines
drivers/dma/sh/sudmac.h new file +7
-0+struct sudmac_device { +struct shdma_dev shdma_dev; +struct sudmac_chan *chan[SUDMAC_MAX_CHANNELS ]; +struct sudmac_pdata *pdata; +void __iomem *chan_reg; +}; +...
Quoted 10 lines
include/linux/sudmac.h line 45
Quoted 7 lines
include/linux/sudmac.h new file +7
-0+struct sudmac_pdata { +const struct sudmac_slave_config *slave; +int slave_num; +const struct sudmac_channel *channel; +int channel_num; +}; +I'm a bit perplexed as to why you have a hardcoded max channel count when it can all be derived from the platform data in the first place?
Thank you for the point.
The current driver didn't use the "sudmac_device.chan" actually. So, I should remove the "sudmac_device.chan" and SUDMAC_MAX_CHANNELS. I will submit a new patch soon.
Best regards,
Yoshihiro Shimoda
Hi Shimoda-san
I have 3 comment
Quoted 16 lines
drivers/dma/Kconfig +8
-0@@ -183,6 +183,14 @@ config SH_DMAE help Enable support for the Renesas SuperH DMA controllers. +config SUDMAC + tristate "Renesas SUDMAC support" + depends on (SUPERH && SH_DMA) || (ARM && ARCH_SHMOBILE) + depends on !SH_DMA_API + select DMA_ENGINE + help + Enable support for the Renesas SUDMAC controllers. +
(snip)
Quoted 2 lines
obj-$(CONFIG_SH_DMAE) += sh/
+obj-$(CONFIG_SUDMAC) += sh/
(snip)
Quoted 3 lines
obj-$(CONFIG_SH_DMAE) += shdma-base.o
obj-$(CONFIG_SH_DMAE) += shdma.o
+obj-$(CONFIG_SUDMAC) += shdma-base.o sudmac.o
I guess this is good timing to create driver/dma/sh/Kconfig ?
drivers/dma/sh/sudmac.h
Quoted 2 lines
drivers/dma/sh/sudmac.h new file +0
-0
This is just my opinion.
If there is many driver/code which needs/shares common definition, creating new header is nice idea.
But, in this case, sudmac.h user is sudmac.c only.
It is easy to read code, and reduce file if these definitions exist on sudmac.c IMO
Quoted 1 line
include/linux/sudmac.h +0
-0
(snip)
include/linux/sudmac.h line 53
Quoted 8 lines
include/linux/sudmac.h +8
-0+/* Definitions for the sudmac_channel.config */ +#define SUDMAC_SENDBUFM 0x1000/* b12: Transmit Buffer Mode */ +#define SUDMAC_RCVENDM 0x0100/* b8: Receive Data Transfer End Mode */ +#define SUDMAC_LBA_WAIT 0x0030/* b5-4: Local Bus Access Wait */ + +/* Definitions for the sudmac_channel.dint_end_bit */ +#define SUDMAC_CH1ENDE 0x0002/* b1: Ch1 DMA Transfer End Int Enable */ +#define SUDMAC_CH0ENDE 0x0001/* b0: Ch0 DMA Transfer End Int Enable */
I think these are used as register value,
and it comes from board/platform directly ?
But it is not good approach for me,
because it is easy to break driver.
Can you use like this style ?
--- xxxx.h ----
#define SUDMAC_TX_BUFFER_MODE (1 << 0)
#define SUDMAC_RV_END_MODE (1 << 1)
...
--- xxx.c ----
if (priv->flags & SUDMAC_TX_BUFFER_MODE)
val |= 0x1000
if (priv->flags & SUDMAC_RV_END_MODE)
val |= 0x0100...
Best regards
---
Kuninori Morimoto
Hello.
Quoted 17 lines · Kuninori Morimoto
Quoted 1 line
include/linux/sudmac.h +0
-0(snip)
include/linux/sudmac.h line 53
Quoted 8 lines
include/linux/sudmac.h +8
-0+/* Definitions for the sudmac_channel.config */ +#define SUDMAC_SENDBUFM 0x1000/* b12: Transmit Buffer Mode */ +#define SUDMAC_RCVENDM 0x0100/* b8: Receive Data Transfer End Mode */ +#define SUDMAC_LBA_WAIT 0x0030/* b5-4: Local Bus Access Wait */ + +/* Definitions for the sudmac_channel.dint_end_bit */ +#define SUDMAC_CH1ENDE 0x0002/* b1: Ch1 DMA Transfer End Int Enable */ +#define SUDMAC_CH0ENDE 0x0001/* b0: Ch0 DMA Transfer End Int Enable */ I think these are used as register value,
and it comes from board/platform directly ?But it is not good approach for me,
because it is easy to break driver.
Quoted 6 lines
Can you use like this style ?
---
xxxx.h----
#defineSUDMAC_TX_BUFFER_MODE(1 << 0)
#defineSUDMAC_RV_END_MODE(1 << 1)
...
Better yet, use the BIT() macro (althought it won't help if you'll have to
define a bitfield.
WBR, Sergei
Hi Morimoto-san,
Quoted 4 lines · Kuninori Morimoto
Hi Shimoda-san
I have 3 comment
Thank you for the review.
Quoted 17 lines
Quoted 16 lines
drivers/dma/Kconfig +8
-0@@ -183,6 +183,14 @@ config SH_DMAE help Enable support for the Renesas SuperH DMA controllers. +config SUDMAC + tristate "Renesas SUDMAC support" + depends on (SUPERH && SH_DMA) || (ARM && ARCH_SHMOBILE) + depends on !SH_DMA_API + select DMA_ENGINE + help + Enable support for the Renesas SUDMAC controllers. +(snip)
Quoted 8 lines
Quoted 2 lines
obj-$(CONFIG_SH_DMAE) += sh/
+obj-$(CONFIG_SUDMAC) += sh/(snip)
Quoted 3 lines
obj-$(CONFIG_SH_DMAE) += shdma-base.o
obj-$(CONFIG_SH_DMAE) += shdma.o
+obj-$(CONFIG_SUDMAC) += shdma-base.o sudmac.oI guess this is good timing to create driver/dma/sh/Kconfig ?
I think so.
So, I will write a new patch that move the "config SH_DMAE" to the "driver/dma/sh/Kconfig", and then, I will add the "config SUDMAC" in the Kconfig.
Quoted 9 lines
drivers/dma/sh/sudmac.h
Quoted 2 lines
drivers/dma/sh/sudmac.h new file +0
-0This is just my opinion.
If there is many driver/code which needs/shares common definition, creating new header is nice idea.
But, in this case,sudmac.huser issudmac.conly.
It is easy to read code, and reduce file if these definitions exist onsudmac.cIMO
I got it, I will remove the drivers/dma/sh/sudmac.h.
Quoted 14 lines
Quoted 1 line
include/linux/sudmac.h +0
-0(snip)
include/linux/sudmac.h line 53
Quoted 8 lines
include/linux/sudmac.h +8
-0+/* Definitions for the sudmac_channel.config */ +#define SUDMAC_SENDBUFM 0x1000/* b12: Transmit Buffer Mode */ +#define SUDMAC_RCVENDM 0x0100/* b8: Receive Data Transfer End Mode */ +#define SUDMAC_LBA_WAIT 0x0030/* b5-4: Local Bus Access Wait */ + +/* Definitions for the sudmac_channel.dint_end_bit */ +#define SUDMAC_CH1ENDE 0x0002/* b1: Ch1 DMA Transfer End Int Enable */ +#define SUDMAC_CH0ENDE 0x0001/* b0: Ch0 DMA Transfer End Int Enable */ I think these are used as register value,
and it comes from board/platform directly ?
Quoted 4 lines
But it is not good approach for me,
because it is easy to break driver.Can you use like this style ?
I got it. I will modify it.
Quoted 4 lines
---
xxxx.h----
#defineSUDMAC_TX_BUFFER_MODE(1 << 0)
#defineSUDMAC_RV_END_MODE(1 << 1)
...
Sergei pointed out this, so I will use the BIT() macro.
Best regards,
Yoshihiro Shimoda
Quoted 12 lines
---
xxx.c----if (
priv->flags&SUDMAC_TX_BUFFER_MODE)val |= 0x1000 if (priv->flags & SUDMAC_RV_END_MODE) val |= 0x0100...
Best regards
---
Kuninori Morimoto