Message ID | 20180425200816.GA4662@jerusalem (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > ColdFire mcf5441x implements an edma hw module similar to the Is it similar to to edma ? > one implemented in Vybrid VFxxx controllers, but with a slightly > different register set, more dma channels (64 instead of 32), > a different interrupt mechanism and some other minor differences. > > For the above reasons, modfying fsl-edma.c was too complex and > likely too ugly. From here, the decision to create a different > driver, but starting from fsl-edma. can the common stuff be made into a lib and shared between then two rather than having a same driver or different drivers? > > The driver has been tested with mcf5441x (stmark2 board) and > dspi driver, it worked fine and seems reliable at least as a > first initial version. > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > --- > arch/m68k/configs/stmark2_defconfig | 2 + this should be a separate patch please > multiplexing capability for DMA request sources(slot). > This module can be found on Freescale Vybrid and LS-1 SoCs. > > +config MCF_EDMA Alphabetical sort pls > +// SPDX-License-Identifier: GPL-2.0 Copyright info should be here in c99 style comments > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt why do you need this, why not use dev_xxx > +#define EDMA_CHANNELS 64 > +#define EDMA_MASK_CH(x) ((x) & 0x3F) > +#define EDMA_MASK_ITER(x) ((x) & 0x7FFF) > +#define EDMA_TCD_MEM_ALIGN 32 > + > +#define EDMA_TCD_ATTR_DSZ_8b (0x0000) > +#define EDMA_TCD_ATTR_DSZ_16b (0x0001) > +#define EDMA_TCD_ATTR_DSZ_32b (0x0002) > +#define EDMA_TCD_ATTR_DSZ_16B (0x0004) BIT and GENMASK for these.. > +static unsigned int mcf_edma_get_tcd_attr(enum dma_slave_buswidth addr_width) > +{ > + switch (addr_width) { > + case 1: > + return EDMA_TCD_ATTR_SSZ_8b | EDMA_TCD_ATTR_DSZ_8b; > + case 2: > + return EDMA_TCD_ATTR_SSZ_16b | EDMA_TCD_ATTR_DSZ_16b; > + case 4: > + default: why default not treated as error? > +static int mcf_edma_slave_config(struct dma_chan *chan, > + struct dma_slave_config *cfg) > +{ > + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); > + > + mcf_chan->esc.dir = cfg->direction; > + if (cfg->direction == DMA_DEV_TO_MEM) { > + mcf_chan->esc.dev_addr = cfg->src_addr; > + mcf_chan->esc.addr_width = cfg->src_addr_width; > + mcf_chan->esc.burst = cfg->src_maxburst; > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->src_addr_width); > + } else if (cfg->direction == DMA_MEM_TO_DEV) { > + mcf_chan->esc.dev_addr = cfg->dst_addr; > + mcf_chan->esc.addr_width = cfg->dst_addr_width; > + mcf_chan->esc.burst = cfg->dst_maxburst; > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->dst_addr_width); please save both src/dstn details here, typically we dont at this point about the txn direction... direction comes with prep_xxx call > +static void mcf_edma_free_desc(struct virt_dma_desc *vdesc) > +{ > + struct mcf_edma_desc *mcf_desc; > + int i; > + struct edma_regs *regs; > + > + mcf_desc = to_mcf_edma_desc(vdesc); > + regs = mcf_desc->echan->edma->membase; > + > + //trace_tcd(®s->tcd[mcf_desc->echan->slave_id]); > + //trace_regs(regs); ?? > +static int mcf_edma_irq_init(struct platform_device *pdev, > + struct mcf_edma_engine *mcf_edma) > +{ > + int ret = 0, i; > + struct resource *res; > + > + res = platform_get_resource_byname(pdev, > + IORESOURCE_IRQ, "edma-tx-00-15"); > + if (!res) > + return -1; > + > + for (ret = 0, i = res->start; i <= res->end; ++i) { > + ret |= devm_request_irq(&pdev->dev, i, > + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); you are explicitly freeing irq below, so why use devm_ ?
Hi Vinod, thanks for the review, On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote: > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > > > ColdFire mcf5441x implements an edma hw module similar to the > > Is it similar to to edma ? > It is similar to Freescale "edma" but with a different number of channels, a bit different register set, different interrupt structure, no channel multiplexer. > > one implemented in Vybrid VFxxx controllers, but with a slightly > > different register set, more dma channels (64 instead of 32), > > a different interrupt mechanism and some other minor differences. > > > > For the above reasons, modfying fsl-edma.c was too complex and > > likely too ugly. From here, the decision to create a different > > driver, but starting from fsl-edma. > > can the common stuff be made into a lib and shared between then two rather > than having a same driver or different drivers? > It should be possible to collect some common code in a kind of mcf_edma_core.c common module, but in this case i cannot then test the Vybrid edma after the changes since i miss that hardware. Would be maybe possible for you to diff fsl-edma and this mcf-edma, just to confirm if i can still stay this way, or if moving to a library becomes mandatory ? > > > > The driver has been tested with mcf5441x (stmark2 board) and > > dspi driver, it worked fine and seems reliable at least as a > > first initial version. > > > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > > --- > > arch/m68k/configs/stmark2_defconfig > this should be a separate patch please > Ack. > > multiplexing capability for DMA request sources(slot). > > This module can be found on Freescale Vybrid and LS-1 SoCs. > > > > +config MCF_EDMA > > Alphabetical sort pls > Ack. > > +// SPDX-License-Identifier: GPL-2.0 > > Copyright info should be here in c99 style comments > Seems checkpatch.pl, for C files, does not like the C style initial line comment: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 #87: FILE: drivers/dma/mcf-edma.c:1: +/* SPDX-License-Identifier: GPL-2.0 */ While c++ type is accepted. In contrary, in .h files it wants cpp // style and not C style. > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > why do you need this, why not use dev_xxx > Well, pr_ style seems to simplify the call a bit, should be allowed but if you prefer i can move all to dev_ format. > > +#define EDMA_CHANNELS 64 > > +#define EDMA_MASK_CH(x) ((x) & 0x3F) > > +#define EDMA_MASK_ITER(x) ((x) & 0x7FFF) > > +#define EDMA_TCD_MEM_ALIGN 32 > > + > > +#define EDMA_TCD_ATTR_DSZ_8b (0x0000) > > +#define EDMA_TCD_ATTR_DSZ_16b (0x0001) > > +#define EDMA_TCD_ATTR_DSZ_32b (0x0002) > > +#define EDMA_TCD_ATTR_DSZ_16B (0x0004) > > BIT and GENMASK for these.. > Ack. > > +static unsigned int mcf_edma_get_tcd_attr(enum dma_slave_buswidth addr_width) > > +{ > > + switch (addr_width) { > > + case 1: > > + return EDMA_TCD_ATTR_SSZ_8b | EDMA_TCD_ATTR_DSZ_8b; > > + case 2: > > + return EDMA_TCD_ATTR_SSZ_16b | EDMA_TCD_ATTR_DSZ_16b; > > + case 4: > > + default: > > why default not treated as error? > Ack, will fix that. > > +static int mcf_edma_slave_config(struct dma_chan *chan, > > + struct dma_slave_config *cfg) > > +{ > > + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); > > + > > + mcf_chan->esc.dir = cfg->direction; > > + if (cfg->direction == DMA_DEV_TO_MEM) { > > + mcf_chan->esc.dev_addr = cfg->src_addr; > > + mcf_chan->esc.addr_width = cfg->src_addr_width; > > + mcf_chan->esc.burst = cfg->src_maxburst; > > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->src_addr_width); > > + } else if (cfg->direction == DMA_MEM_TO_DEV) { > > + mcf_chan->esc.dev_addr = cfg->dst_addr; > > + mcf_chan->esc.addr_width = cfg->dst_addr_width; > > + mcf_chan->esc.burst = cfg->dst_maxburst; > > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->dst_addr_width); > > please save both src/dstn details here, typically we dont at this point > about the txn direction... direction comes with prep_xxx call > Ack. > > +static void mcf_edma_free_desc(struct virt_dma_desc *vdesc) > > +{ > > + struct mcf_edma_desc *mcf_desc; > > + int i; > > + struct edma_regs *regs; > > + > > + mcf_desc = to_mcf_edma_desc(vdesc); > > + regs = mcf_desc->echan->edma->membase; > > + > > + //trace_tcd(®s->tcd[mcf_desc->echan->slave_id]); > > + //trace_regs(regs); > > ?? > Sorry, forgot those, will remove them. > > +static int mcf_edma_irq_init(struct platform_device *pdev, > > + struct mcf_edma_engine *mcf_edma) > > +{ > > + int ret = 0, i; > > + struct resource *res; > > + > > + res = platform_get_resource_byname(pdev, > > + IORESOURCE_IRQ, "edma-tx-00-15"); > > + if (!res) > > + return -1; > > + > > + for (ret = 0, i = res->start; i <= res->end; ++i) { > > + ret |= devm_request_irq(&pdev->dev, i, > > + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); > > you are explicitly freeing irq below, so why use devm_ ? Ack, Will fix that. > -- > ~Vinod Many thanks, Regards, Angelo -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote: > Hi Vinod, > > thanks for the review, > > On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote: > > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > > > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > > > > > ColdFire mcf5441x implements an edma hw module similar to the > > > > Is it similar to to edma ? > > > > It is similar to Freescale "edma" but with a different number of > channels, a bit different register set, different interrupt > structure, no channel multiplexer. ok > > > one implemented in Vybrid VFxxx controllers, but with a slightly > > > different register set, more dma channels (64 instead of 32), > > > a different interrupt mechanism and some other minor differences. > > > > > > For the above reasons, modfying fsl-edma.c was too complex and > > > likely too ugly. From here, the decision to create a different > > > driver, but starting from fsl-edma. > > > > can the common stuff be made into a lib and shared between then two rather > > than having a same driver or different drivers? > > It should be possible to collect some common code in a kind of > mcf_edma_core.c common module, but in this case i cannot then test > the Vybrid edma after the changes since i miss that hardware. Sure you should send the patches and folks who care about fsl driver would look it up and test > Would be maybe possible for you to diff fsl-edma and this mcf-edma, > just to confirm if i can still stay this way, or if moving to a > library becomes mandatory ? well since you know the IP you would make a better guess on that, best is to check register sets in drivers > > > +// SPDX-License-Identifier: GPL-2.0 > > > > Copyright info should be here in c99 style comments > > > > Seems checkpatch.pl, for C files, does not like the C style > initial line comment: > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > #87: FILE: drivers/dma/mcf-edma.c:1: > +/* SPDX-License-Identifier: GPL-2.0 */ > > While c++ type is accepted. > > In contrary, in .h files it wants cpp // style and not C style. SC99 comments style is // SPDX-License-Identifier: GPL-2.0 Point is the copyright should be added is same formar i.e., // Copyright 20018 - foo bar this line should follow the spdx line > > > + > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > why do you need this, why not use dev_xxx > > > > Well, pr_ style seems to simplify the call a bit, should be allowed > but if you prefer i can move all to dev_ format. in hindsight dev_ makes better sense, been there done that :)
Hi Vinod, On Mon, May 07, 2018 at 07:45:35PM +0530, Vinod Koul wrote: > On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote: > > Hi Vinod, > > > > thanks for the review, > > > > On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote: > > > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > > > > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > > > > > > > ColdFire mcf5441x implements an edma hw module similar to the > > > > > > Is it similar to to edma ? > > > > > > > It is similar to Freescale "edma" but with a different number of > > channels, a bit different register set, different interrupt > > structure, no channel multiplexer. > > ok > > > > > one implemented in Vybrid VFxxx controllers, but with a slightly > > > > different register set, more dma channels (64 instead of 32), > > > > a different interrupt mechanism and some other minor differences. > > > > > > > > For the above reasons, modfying fsl-edma.c was too complex and > > > > likely too ugly. From here, the decision to create a different > > > > driver, but starting from fsl-edma. > > > > > > can the common stuff be made into a lib and shared between then two rather > > > than having a same driver or different drivers? > > > > It should be possible to collect some common code in a kind of > > mcf_edma_core.c common module, but in this case i cannot then test > > the Vybrid edma after the changes since i miss that hardware. > > Sure you should send the patches and folks who care about fsl driver > would look it up and test > > > Would be maybe possible for you to diff fsl-edma and this mcf-edma, > > just to confirm if i can still stay this way, or if moving to a > > library becomes mandatory ? > > well since you know the IP you would make a better guess on that, best is > to check register sets in drivers > I fixed all the discussed points. Actaully mcf-edma (ColdFire) has a slightly different register set (due to 64 channels in place of 16 of fsl-edma) and, for the same reason, a different DMA interrupt structure. Also, i simplified some parts of the driver considering ColdFire (mcf) big endian architecture. So i would send a rev 2 patch with all the fixes, than eventually in a second phase i may try to create some common code, but at least we have the ColdFire DMA. What do you think ? > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > Copyright info should be here in c99 style comments > > > > > > > Seems checkpatch.pl, for C files, does not like the C style > > initial line comment: > > > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > > #87: FILE: drivers/dma/mcf-edma.c:1: > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > While c++ type is accepted. > > > > In contrary, in .h files it wants cpp // style and not C style. > > SC99 comments style is > // SPDX-License-Identifier: GPL-2.0 > > Point is the copyright should be added is same formar i.e., > > // Copyright 20018 - foo bar > > this line should follow the spdx line > > > > > + > > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > > why do you need this, why not use dev_xxx > > > > > > > Well, pr_ style seems to simplify the call a bit, should be allowed > > but if you prefer i can move all to dev_ format. > > in hindsight dev_ makes better sense, been there done that :) > > -- > ~Vinod > -- > To unsubscribe from this list: send the line "unsubscribe dmaengine" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards, Angelo -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22-05-18, 23:28, Angelo Dureghello wrote: > Hi Vinod, > > On Mon, May 07, 2018 at 07:45:35PM +0530, Vinod Koul wrote: > > On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote: > > > Hi Vinod, > > > > > > thanks for the review, > > > > > > On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote: > > > > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > > > > > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > > > > > > > > > ColdFire mcf5441x implements an edma hw module similar to the > > > > > > > > Is it similar to to edma ? > > > > > > > > > > It is similar to Freescale "edma" but with a different number of > > > channels, a bit different register set, different interrupt > > > structure, no channel multiplexer. > > > > ok > > > > > > > one implemented in Vybrid VFxxx controllers, but with a slightly > > > > > different register set, more dma channels (64 instead of 32), > > > > > a different interrupt mechanism and some other minor differences. > > > > > > > > > > For the above reasons, modfying fsl-edma.c was too complex and > > > > > likely too ugly. From here, the decision to create a different > > > > > driver, but starting from fsl-edma. > > > > > > > > can the common stuff be made into a lib and shared between then two rather > > > > than having a same driver or different drivers? > > > > > > It should be possible to collect some common code in a kind of > > > mcf_edma_core.c common module, but in this case i cannot then test > > > the Vybrid edma after the changes since i miss that hardware. > > > > Sure you should send the patches and folks who care about fsl driver > > would look it up and test > > > > > Would be maybe possible for you to diff fsl-edma and this mcf-edma, > > > just to confirm if i can still stay this way, or if moving to a > > > library becomes mandatory ? > > > > well since you know the IP you would make a better guess on that, best is > > to check register sets in drivers > > > I fixed all the discussed points. > > Actaully mcf-edma (ColdFire) has a slightly different register set (due to 64 > channels in place of 16 of fsl-edma) and, for the same reason, a different > DMA interrupt structure. > Also, i simplified some parts of the driver considering ColdFire (mcf) > big endian architecture. > > So i would send a rev 2 patch with all the fixes, than eventually in a second > phase i may try to create some common code, but at least we have the ColdFire > DMA. What do you think ? wouldn't it be easier to just make common parts and then add edma specific code. If I was doing this it would be my apprach and that way code edma specific will be lesser and faster review
Hi Vinod, thanks for your support. On Wed, May 23, 2018 at 11:07:06AM +0530, Vinod wrote: > On 22-05-18, 23:28, Angelo Dureghello wrote: > > Hi Vinod, > > > > On Mon, May 07, 2018 at 07:45:35PM +0530, Vinod Koul wrote: > > > On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote: > > > > Hi Vinod, > > > > > > > > thanks for the review, > > > > > > > > On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote: > > > > > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > > > > > > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > > > > > > > > > > > ColdFire mcf5441x implements an edma hw module similar to the > > > > > > > > > > Is it similar to to edma ? > > > > > > > > > > > > > It is similar to Freescale "edma" but with a different number of > > > > channels, a bit different register set, different interrupt > > > > structure, no channel multiplexer. > > > > > > ok > > > > > > > > > one implemented in Vybrid VFxxx controllers, but with a slightly > > > > > > different register set, more dma channels (64 instead of 32), > > > > > > a different interrupt mechanism and some other minor differences. > > > > > > > > > > > > For the above reasons, modfying fsl-edma.c was too complex and > > > > > > likely too ugly. From here, the decision to create a different > > > > > > driver, but starting from fsl-edma. > > > > > > > > > > can the common stuff be made into a lib and shared between then two rather > > > > > than having a same driver or different drivers? > > > > > > > > It should be possible to collect some common code in a kind of > > > > mcf_edma_core.c common module, but in this case i cannot then test > > > > the Vybrid edma after the changes since i miss that hardware. > > > > > > Sure you should send the patches and folks who care about fsl driver > > > would look it up and test > > > > > > > Would be maybe possible for you to diff fsl-edma and this mcf-edma, > > > > just to confirm if i can still stay this way, or if moving to a > > > > library becomes mandatory ? > > > > > > well since you know the IP you would make a better guess on that, best is > > > to check register sets in drivers > > > > > I fixed all the discussed points. > > > > Actaully mcf-edma (ColdFire) has a slightly different register set (due to 64 > > channels in place of 16 of fsl-edma) and, for the same reason, a different > > DMA interrupt structure. > > Also, i simplified some parts of the driver considering ColdFire (mcf) > > big endian architecture. > > > > So i would send a rev 2 patch with all the fixes, than eventually in a second > > phase i may try to create some common code, but at least we have the ColdFire > > DMA. What do you think ? > > wouldn't it be easier to just make common parts and then add edma specific code. > If I was doing this it would be my apprach and that way code edma specific will > be lesser and faster review > I tried to set up a common module, but couldn't reach any good point. Issues are: 1) Edma register set between 32 and 64ch is similar, but some offsets/names are not matching between the 2 variants, some registers names are swapped over the reg. address range, 2) interrupt numbers and scheme is still different, handler implementation comes different, 3) as a corollary of the above, all the common functions that needs to access edma registers should use same structure pointers. I could use a union someway but points where register are accessed are many, and i should differentiate the access in each case, referencing to a different structure in each case. If you have any idea on how i could reach a common module, with 2 different registers set, that's welcome. I stay on the thought that a separate 64-channel module is the best way to go here. Currently, as Freescale "edma" variants, i know: Vybrid VFXXX 32ch DMA multiplexer reg.set 1 Kynetis K70 (CortexM4) 32ch DMA multiplexer reg.set 1 imx8xx (coming) 32ch no multiplexer reg.set 1 MPC57xxk 32ch DMA multiplexer reg.set 1 ColdFire mcf5441x 64ch no multiplexer reg.set 2 <--- There may me other cpu using this fsl edma module but not in my knowledge right now. So i still think at the end, to have 2 separate drivers for the 32 and 64 variant is good and probably the most ordered/clean solution. Regards, Angelo > -- > ~Vinod > -- > To unsubscribe from this list: send the line "unsubscribe dmaengine" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Angelo, On 26-05-18, 22:50, Angelo Dureghello wrote: > > wouldn't it be easier to just make common parts and then add edma specific code. > > If I was doing this it would be my apprach and that way code edma specific will > > be lesser and faster review > > > > I tried to set up a common module, but couldn't reach any good point. > > Issues are: > 1) Edma register set between 32 and 64ch is similar, but some offsets/names > are not matching between the 2 variants, some registers names are swapped over > the reg. address range, > 2) interrupt numbers and scheme is still different, handler implementation comes > different, > 3) as a corollary of the above, all the common functions that needs to access > edma registers should use same structure pointers. I could use a union > someway but points where register are accessed are many, and i should > differentiate the access in each case, referencing to a different structure > in each case. > > If you have any idea on how i could reach a common module, with 2 different > registers set, that's welcome. > I stay on the thought that a separate 64-channel module is the best > way to go here. > > Currently, as Freescale "edma" variants, i know: > > Vybrid VFXXX 32ch DMA multiplexer reg.set 1 > Kynetis K70 (CortexM4) 32ch DMA multiplexer reg.set 1 > imx8xx (coming) 32ch no multiplexer reg.set 1 > MPC57xxk 32ch DMA multiplexer reg.set 1 > ColdFire mcf5441x 64ch no multiplexer reg.set 2 <--- > > There may me other cpu using this fsl edma module but not in my knowledge > right now. > > So i still think at the end, to have 2 separate drivers for the 32 and 64 > variant is good and probably the most ordered/clean solution. Okay there are few ways we can do this. One is to use helpers for register access and these helpers are different for the variant you are loaded on. Another is to use register offsets which are set based on the variant loaded.. HTH
Hi Vinod, On Mon, May 28, 2018 at 09:31:23AM +0530, Vinod wrote: > Hi Angelo, > > On 26-05-18, 22:50, Angelo Dureghello wrote: > > > > wouldn't it be easier to just make common parts and then add edma specific code. > > > If I was doing this it would be my apprach and that way code edma specific will > > > be lesser and faster review > > > > > > > I tried to set up a common module, but couldn't reach any good point. > > > > Issues are: > > 1) Edma register set between 32 and 64ch is similar, but some offsets/names > > are not matching between the 2 variants, some registers names are swapped over > > the reg. address range, > > 2) interrupt numbers and scheme is still different, handler implementation comes > > different, > > 3) as a corollary of the above, all the common functions that needs to access > > edma registers should use same structure pointers. I could use a union > > someway but points where register are accessed are many, and i should > > differentiate the access in each case, referencing to a different structure > > in each case. > > > > If you have any idea on how i could reach a common module, with 2 different > > registers set, that's welcome. > > I stay on the thought that a separate 64-channel module is the best > > way to go here. > > > > Currently, as Freescale "edma" variants, i know: > > > > Vybrid VFXXX 32ch DMA multiplexer reg.set 1 > > Kynetis K70 (CortexM4) 32ch DMA multiplexer reg.set 1 > > imx8xx (coming) 32ch no multiplexer reg.set 1 > > MPC57xxk 32ch DMA multiplexer reg.set 1 > > ColdFire mcf5441x 64ch no multiplexer reg.set 2 <--- > > > > There may me other cpu using this fsl edma module but not in my knowledge > > right now. > > > > So i still think at the end, to have 2 separate drivers for the 32 and 64 > > variant is good and probably the most ordered/clean solution. > > Okay there are few ways we can do this. One is to use helpers for register > access and these helpers are different for the variant you are loaded on. > > Another is to use register offsets which are set based on the variant loaded.. > Ok i try with register offsets. Lets' see. Thanks, Angelo > HTH > -- > ~Vinod > -- > To unsubscribe from this list: send the line "unsubscribe linux-m68k" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/m68k/configs/stmark2_defconfig b/arch/m68k/configs/stmark2_defconfig index bf2bfd4ebd2a..2d111e0aeb48 100644 --- a/arch/m68k/configs/stmark2_defconfig +++ b/arch/m68k/configs/stmark2_defconfig @@ -71,6 +71,8 @@ CONFIG_GPIO_GENERIC_PLATFORM=y # CONFIG_HWMON is not set # CONFIG_HID is not set # CONFIG_USB_SUPPORT is not set +CONFIG_DMADEVICES=y +CONFIG_MCF_EDMA=y # CONFIG_FILE_LOCKING is not set # CONFIG_DNOTIFY is not set # CONFIG_INOTIFY_USER is not set diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 6d61cd023633..139626c01ba4 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -225,6 +225,17 @@ config FSL_EDMA multiplexing capability for DMA request sources(slot). This module can be found on Freescale Vybrid and LS-1 SoCs. +config MCF_EDMA + tristate "Freescale eDMA engine support, ColdFire mcf5441x SoCs" + depends on M5441x + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Support the Freescale ColdFire eDMA engine, 64-channel + implementation that performs complex data transfers with + minimal intervention from a host processor. + This module can be found on Freescale ColdFire mcf5441x SoCs. + config FSL_RAID tristate "Freescale RAID engine Support" depends on FSL_SOC && !ASYNC_TX_ENABLE_CHANNEL_SWITCH diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 0f62a4d49aab..db93824441aa 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_DW_DMAC_CORE) += dw/ obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o obj-$(CONFIG_FSL_DMA) += fsldma.o obj-$(CONFIG_FSL_EDMA) += fsl-edma.o +obj-$(CONFIG_MCF_EDMA) += mcf-edma.o obj-$(CONFIG_FSL_RAID) += fsl_raid.o obj-$(CONFIG_HSU_DMA) += hsu/ obj-$(CONFIG_IMG_MDC_DMA) += img-mdc-dma.o diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c new file mode 100644 index 000000000000..8fb6282e922a --- /dev/null +++ b/drivers/dma/mcf-edma.c @@ -0,0 +1,847 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * drivers/dma/mcf-edma.c + * + * Driver for the Freescale ColdFire 64-ch eDMA implementation, + * derived from drivers/dma/fsl-edma.c. + * + * Copyright 2013-2014 Freescale Semiconductor, Inc + * + * Copyright 2017 Sysam, Angelo Dureghello <angelo@sysam.it> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/dmaengine.h> +#include <linux/dmapool.h> +#include <linux/dma-mapping.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/platform_data/dma-mcf-edma.h> + +#include "virt-dma.h" + +#define EDMA_CHANNELS 64 +#define EDMA_MASK_CH(x) ((x) & 0x3F) +#define EDMA_MASK_ITER(x) ((x) & 0x7FFF) +#define EDMA_TCD_MEM_ALIGN 32 + +#define EDMA_TCD_ATTR_DSZ_8b (0x0000) +#define EDMA_TCD_ATTR_DSZ_16b (0x0001) +#define EDMA_TCD_ATTR_DSZ_32b (0x0002) +#define EDMA_TCD_ATTR_DSZ_16B (0x0004) +#define EDMA_TCD_ATTR_SSZ_8b (EDMA_TCD_ATTR_DSZ_8b << 8) +#define EDMA_TCD_ATTR_SSZ_16b (EDMA_TCD_ATTR_DSZ_16b << 8) +#define EDMA_TCD_ATTR_SSZ_32b (EDMA_TCD_ATTR_DSZ_32b << 8) +#define EDMA_TCD_ATTR_SSZ_16B (EDMA_TCD_ATTR_DSZ_16B << 8) + +#define MCF_EDMA_BUSWIDTHS (BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \ + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \ + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \ + BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)) + +#define EDMA_CR_ERCA BIT(2) +#define EDMA_CR_ERGA BIT(3) + +#define EDMA_TCD_CSR_INT_MAJOR BIT(1) +#define EDMA_TCD_CSR_D_REQ BIT(3) +#define EDMA_TCD_CSR_E_SG BIT(4) + +#define EDMA_CERR_CERR(x) ((x) & 0x1F) + +struct edma_tcd { + u32 saddr; + u16 attr; + u16 soff; + u32 nbytes; + u32 slast; + u32 daddr; + u16 citer; + u16 doff; + u32 dlast_sga; + u16 biter; + u16 csr; +}; + +struct edma_regs { + u32 cr; + u32 es; + u32 erqh; + u32 erql; + u32 eeih; + u32 eeil; + u8 serq; + u8 cerq; + u8 seei; + u8 ceei; + u8 cint; + u8 cerr; + u8 ssrt; + u8 cdne; + u32 inth; + u32 intl; + u32 errh; + u32 errl; + u32 rsh; + u32 rsl; + u8 res[200]; + u8 dch_pri[EDMA_CHANNELS]; + u8 res2[3776]; + struct edma_tcd tcd[EDMA_CHANNELS]; +}; + +struct mcf_edma_sw_tcd { + dma_addr_t ptcd; + struct edma_tcd *vtcd; +}; + +struct mcf_edma_desc { + struct virt_dma_desc vdesc; + struct mcf_edma_chan *echan; + bool iscyclic; + unsigned int n_tcds; + struct mcf_edma_sw_tcd tcd[]; +}; + +struct mcf_edma_slave_config { + enum dma_transfer_direction dir; + enum dma_slave_buswidth addr_width; + u32 dev_addr; + u32 burst; + u32 attr; +}; + +struct mcf_edma_chan { + struct virt_dma_chan vchan; + enum dma_status status; + bool idle; + struct mcf_edma_engine *edma; + struct dma_pool *tcd_pool; + struct mcf_edma_desc *edesc; + struct mcf_edma_slave_config esc; + u32 slave_id; +}; + +struct mcf_edma_engine { + struct dma_device dma_dev; + int n_chans; + void __iomem *membase; + struct mutex mcf_edma_mutex; + struct mcf_edma_chan chans[]; +}; + +static struct mcf_edma_chan *to_mcf_edma_chan(struct dma_chan *chan) +{ + return container_of(chan, struct mcf_edma_chan, vchan.chan); +} + +static struct mcf_edma_desc *to_mcf_edma_desc(struct virt_dma_desc *vd) +{ + return container_of(vd, struct mcf_edma_desc, vdesc); +} + +static unsigned int mcf_edma_get_tcd_attr(enum dma_slave_buswidth addr_width) +{ + switch (addr_width) { + case 1: + return EDMA_TCD_ATTR_SSZ_8b | EDMA_TCD_ATTR_DSZ_8b; + case 2: + return EDMA_TCD_ATTR_SSZ_16b | EDMA_TCD_ATTR_DSZ_16b; + case 4: + default: + return EDMA_TCD_ATTR_SSZ_32b | EDMA_TCD_ATTR_DSZ_32b; + } +} + +static void mcf_edma_enable_request(struct mcf_edma_chan *mcf_chan) +{ + struct edma_regs *regs = mcf_chan->edma->membase; + u32 ch = mcf_chan->vchan.chan.chan_id; + + iowrite8(EDMA_MASK_CH(ch), ®s->seei); + iowrite8(EDMA_MASK_CH(ch), ®s->serq); +} + +static void mcf_edma_disable_request(struct mcf_edma_chan *mcf_chan) +{ + struct edma_regs *regs = mcf_chan->edma->membase; + u32 ch = mcf_chan->vchan.chan.chan_id; + + iowrite8(EDMA_MASK_CH(ch), ®s->cerq); + iowrite8(EDMA_MASK_CH(ch), ®s->ceei); +} + +static void mcf_edma_set_tcd_regs(struct mcf_edma_chan *mcf_chan, + struct edma_tcd *tcd) +{ + struct edma_regs *regs = mcf_chan->edma->membase; + u32 ch = mcf_chan->vchan.chan.chan_id; + + iowrite16(0, ®s->tcd[ch].csr); + iowrite32(tcd->saddr, ®s->tcd[ch].saddr); + iowrite16(tcd->attr, ®s->tcd[ch].attr); + iowrite16(tcd->soff, ®s->tcd[ch].soff); + iowrite32(tcd->nbytes, ®s->tcd[ch].nbytes); + iowrite32(tcd->slast, ®s->tcd[ch].slast); + iowrite32(tcd->daddr, ®s->tcd[ch].daddr); + iowrite16(tcd->citer, ®s->tcd[ch].citer); + iowrite16(tcd->doff, ®s->tcd[ch].doff); + iowrite32(tcd->dlast_sga, ®s->tcd[ch].dlast_sga); + iowrite16(tcd->biter, ®s->tcd[ch].biter); + iowrite16(tcd->csr, ®s->tcd[ch].csr); +} + +static void mcf_edma_xfer_desc(struct mcf_edma_chan *mcf_chan) +{ + struct virt_dma_desc *vdesc; + + vdesc = vchan_next_desc(&mcf_chan->vchan); + if (!vdesc) + return; + + mcf_chan->edesc = to_mcf_edma_desc(vdesc); + + mcf_edma_set_tcd_regs(mcf_chan, mcf_chan->edesc->tcd[0].vtcd); + mcf_edma_enable_request(mcf_chan); + + mcf_chan->status = DMA_IN_PROGRESS; + mcf_chan->idle = false; +} + +static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id) +{ + struct mcf_edma_engine *mcf_edma = dev_id; + struct edma_regs *regs = mcf_edma->membase; + unsigned int ch; + struct mcf_edma_chan *mcf_chan; + u64 intmap; + + intmap = ioread32(®s->inth); + intmap <<= 32; + intmap |= ioread32(®s->intl); + if (!intmap) + return IRQ_NONE; + + for (ch = 0; ch < mcf_edma->n_chans; ch++) { + if (intmap & (0x1 << ch)) { + iowrite8(EDMA_MASK_CH(ch), ®s->cint); + + mcf_chan = &mcf_edma->chans[ch]; + + spin_lock(&mcf_chan->vchan.lock); + if (!mcf_chan->edesc->iscyclic) { + list_del(&mcf_chan->edesc->vdesc.node); + vchan_cookie_complete(&mcf_chan->edesc->vdesc); + mcf_chan->edesc = NULL; + mcf_chan->status = DMA_COMPLETE; + mcf_chan->idle = true; + } else { + vchan_cyclic_callback(&mcf_chan->edesc->vdesc); + } + + if (!mcf_chan->edesc) + mcf_edma_xfer_desc(mcf_chan); + + spin_unlock(&mcf_chan->vchan.lock); + } + } + + return IRQ_HANDLED; +} + +static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id) +{ + struct mcf_edma_engine *mcf_edma = dev_id; + struct edma_regs *regs = mcf_edma->membase; + unsigned int err, ch; + + err = ioread32(®s->errl); + if (!err) + return IRQ_NONE; + + for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) { + if (err & (0x1 << ch)) { + mcf_edma_disable_request(&mcf_edma->chans[ch]); + iowrite8(EDMA_CERR_CERR(ch), ®s->cerr); + mcf_edma->chans[ch].status = DMA_ERROR; + mcf_edma->chans[ch].idle = true; + } + } + + err = ioread32(®s->errh); + if (!err) + return IRQ_NONE; + + for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) { + if (err & (0x1 << (ch - (EDMA_CHANNELS / 2)))) { + mcf_edma_disable_request(&mcf_edma->chans[ch]); + iowrite8(EDMA_CERR_CERR(ch), ®s->cerr); + mcf_edma->chans[ch].status = DMA_ERROR; + mcf_edma->chans[ch].idle = true; + } + } + + return IRQ_HANDLED; +} + +static inline void mcf_edma_fill_tcd(struct edma_tcd *tcd, + u32 src, u32 dst, u16 attr, u16 soff, u32 nbytes, + u32 slast, u16 citer, u16 biter, u16 doff, + u32 dlast_sga, bool major_int, + bool disable_req, bool enable_sg) +{ + u16 csr = 0; + + tcd->saddr = src; + tcd->daddr = dst; + tcd->attr = attr; + tcd->soff = soff; + tcd->nbytes = nbytes; + tcd->slast = slast; + tcd->citer = EDMA_MASK_ITER(citer); + tcd->doff = doff; + tcd->dlast_sga = dlast_sga; + tcd->biter = EDMA_MASK_ITER(biter); + + if (major_int) + csr |= EDMA_TCD_CSR_INT_MAJOR; + + if (disable_req) + csr |= EDMA_TCD_CSR_D_REQ; + + if (enable_sg) + csr |= EDMA_TCD_CSR_E_SG; + + tcd->csr = csr; +} + +static int mcf_edma_slave_config(struct dma_chan *chan, + struct dma_slave_config *cfg) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + + mcf_chan->esc.dir = cfg->direction; + if (cfg->direction == DMA_DEV_TO_MEM) { + mcf_chan->esc.dev_addr = cfg->src_addr; + mcf_chan->esc.addr_width = cfg->src_addr_width; + mcf_chan->esc.burst = cfg->src_maxburst; + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->src_addr_width); + } else if (cfg->direction == DMA_MEM_TO_DEV) { + mcf_chan->esc.dev_addr = cfg->dst_addr; + mcf_chan->esc.addr_width = cfg->dst_addr_width; + mcf_chan->esc.burst = cfg->dst_maxburst; + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->dst_addr_width); + } else { + return -EINVAL; + } + + return 0; +} + +static struct mcf_edma_desc *mcf_edma_alloc_desc( + struct mcf_edma_chan *mcf_chan, int sg_len) +{ + struct mcf_edma_desc *mcf_desc; + int i; + + mcf_desc = kzalloc(sizeof(*mcf_desc) + sizeof(struct mcf_edma_sw_tcd) + * sg_len, GFP_NOWAIT); + if (!mcf_desc) + return NULL; + + mcf_desc->echan = mcf_chan; + mcf_desc->n_tcds = sg_len; + for (i = 0; i < sg_len; i++) { + mcf_desc->tcd[i].vtcd = dma_pool_alloc(mcf_chan->tcd_pool, + GFP_NOWAIT, &mcf_desc->tcd[i].ptcd); + if (!mcf_desc->tcd[i].vtcd) + goto err; + } + + return mcf_desc; + +err: + while (--i >= 0) + dma_pool_free(mcf_chan->tcd_pool, mcf_desc->tcd[i].vtcd, + mcf_desc->tcd[i].ptcd); + kfree(mcf_desc); + + return NULL; +} + +static struct dma_async_tx_descriptor *mcf_edma_prep_slave_sg( + struct dma_chan *chan, struct scatterlist *sgl, + unsigned int sg_len, enum dma_transfer_direction direction, + unsigned long flags, void *context) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + struct mcf_edma_desc *mcf_desc; + struct scatterlist *sg; + u32 src_addr, dst_addr, last_sg, nbytes; + u16 soff, doff, iter; + int i; + + if (!is_slave_direction(mcf_chan->esc.dir)) + return NULL; + + mcf_desc = mcf_edma_alloc_desc(mcf_chan, sg_len); + if (!mcf_desc) + return NULL; + + mcf_desc->iscyclic = false; + + nbytes = mcf_chan->esc.addr_width * mcf_chan->esc.burst; + for_each_sg(sgl, sg, sg_len, i) { + /* get next sg's physical address */ + last_sg = mcf_desc->tcd[(i + 1) % sg_len].ptcd; + + if (mcf_chan->esc.dir == DMA_MEM_TO_DEV) { + src_addr = sg_dma_address(sg); + dst_addr = mcf_chan->esc.dev_addr; + soff = mcf_chan->esc.addr_width; + doff = 0; + } else { + src_addr = mcf_chan->esc.dev_addr; + dst_addr = sg_dma_address(sg); + soff = 0; + doff = mcf_chan->esc.addr_width; + } + + iter = sg_dma_len(sg) / nbytes; + if (i < sg_len - 1) { + last_sg = mcf_desc->tcd[(i + 1)].ptcd; + mcf_edma_fill_tcd(mcf_desc->tcd[i].vtcd, src_addr, + dst_addr, mcf_chan->esc.attr, soff, + nbytes, 0, iter, iter, doff, last_sg, + false, false, true); + } else { + last_sg = 0; + mcf_edma_fill_tcd(mcf_desc->tcd[i].vtcd, src_addr, + dst_addr, mcf_chan->esc.attr, soff, + nbytes, 0, iter, iter, doff, last_sg, + true, true, false); + } + } + + return vchan_tx_prep(&mcf_chan->vchan, &mcf_desc->vdesc, flags); +} + +static struct dma_async_tx_descriptor *mcf_edma_prep_dma_cyclic( + struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len, + size_t period_len, enum dma_transfer_direction direction, + unsigned long flags) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + struct mcf_edma_desc *mcf_desc; + dma_addr_t dma_buf_next; + int sg_len, i; + u32 src_addr, dst_addr, last_sg, nbytes; + u16 soff, doff, iter; + + if (!is_slave_direction(mcf_chan->esc.dir)) + return NULL; + + sg_len = buf_len / period_len; + mcf_desc = mcf_edma_alloc_desc(mcf_chan, sg_len); + if (!mcf_desc) + return NULL; + mcf_desc->iscyclic = true; + + dma_buf_next = dma_addr; + nbytes = mcf_chan->esc.addr_width * mcf_chan->esc.burst; + iter = period_len / nbytes; + + for (i = 0; i < sg_len; i++) { + if (dma_buf_next >= dma_addr + buf_len) + dma_buf_next = dma_addr; + + /* get next sg's physical address */ + last_sg = mcf_desc->tcd[(i + 1) % sg_len].ptcd; + + if (mcf_chan->esc.dir == DMA_MEM_TO_DEV) { + src_addr = dma_buf_next; + dst_addr = mcf_chan->esc.dev_addr; + soff = mcf_chan->esc.addr_width; + doff = 0; + } else { + src_addr = mcf_chan->esc.dev_addr; + dst_addr = dma_buf_next; + soff = 0; + doff = mcf_chan->esc.addr_width; + } + + mcf_edma_fill_tcd(mcf_desc->tcd[i].vtcd, src_addr, dst_addr, + mcf_chan->esc.attr, soff, nbytes, 0, iter, + iter, doff, last_sg, true, false, true); + dma_buf_next += period_len; + } + + return vchan_tx_prep(&mcf_chan->vchan, &mcf_desc->vdesc, flags); +} + +static int mcf_edma_alloc_chan_resources(struct dma_chan *chan) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + + mcf_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev, + sizeof(struct edma_tcd), EDMA_TCD_MEM_ALIGN, 0); + + return 0; +} + +static void mcf_edma_free_chan_resources(struct dma_chan *chan) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + unsigned long flags; + LIST_HEAD(head); + + spin_lock_irqsave(&mcf_chan->vchan.lock, flags); + mcf_edma_disable_request(mcf_chan); + mcf_chan->edesc = NULL; + vchan_get_all_descriptors(&mcf_chan->vchan, &head); + spin_unlock_irqrestore(&mcf_chan->vchan.lock, flags); + + vchan_dma_desc_free_list(&mcf_chan->vchan, &head); + dma_pool_destroy(mcf_chan->tcd_pool); + mcf_chan->tcd_pool = NULL; +} + +static size_t mcf_edma_desc_residue(struct mcf_edma_chan *mcf_chan, + struct virt_dma_desc *vdesc, bool in_progress) +{ + struct mcf_edma_desc *edesc = mcf_chan->edesc; + struct edma_regs *regs = mcf_chan->edma->membase; + u32 ch = mcf_chan->vchan.chan.chan_id; + enum dma_transfer_direction dir = mcf_chan->esc.dir; + dma_addr_t cur_addr, dma_addr; + size_t len, size; + int i; + + /* calculate the total size in this desc */ + for (len = i = 0; i < mcf_chan->edesc->n_tcds; i++) + len += edesc->tcd[i].vtcd->nbytes * edesc->tcd[i].vtcd->biter; + + if (!in_progress) + return len; + + cur_addr = (dir == DMA_MEM_TO_DEV) ? + ioread32(®s->tcd[ch].saddr) : + ioread32(®s->tcd[ch].daddr); + + /* figure out the finished and calculate the residue */ + for (i = 0; i < mcf_chan->edesc->n_tcds; i++) { + size = edesc->tcd[i].vtcd->nbytes * edesc->tcd[i].vtcd->biter; + if (dir == DMA_MEM_TO_DEV) + dma_addr = edesc->tcd[i].vtcd->saddr; + else + dma_addr = edesc->tcd[i].vtcd->daddr; + + len -= size; + if (cur_addr >= dma_addr && cur_addr < dma_addr + size) { + len += dma_addr + size - cur_addr; + break; + } + } + + return len; +} + +static enum dma_status mcf_edma_tx_status(struct dma_chan *chan, + dma_cookie_t cookie, struct dma_tx_state *txstate) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + struct virt_dma_desc *vdesc; + enum dma_status status; + unsigned long flags; + + status = dma_cookie_status(chan, cookie, txstate); + if (status == DMA_COMPLETE) + return status; + + if (!txstate) + return mcf_chan->status; + + spin_lock_irqsave(&mcf_chan->vchan.lock, flags); + vdesc = vchan_find_desc(&mcf_chan->vchan, cookie); + if (mcf_chan->edesc && cookie == mcf_chan->edesc->vdesc.tx.cookie) + txstate->residue = + mcf_edma_desc_residue(mcf_chan, vdesc, true); + else if (vdesc) + txstate->residue = + mcf_edma_desc_residue(mcf_chan, vdesc, false); + else + txstate->residue = 0; + + spin_unlock_irqrestore(&mcf_chan->vchan.lock, flags); + + return mcf_chan->status; +} + +static void mcf_edma_issue_pending(struct dma_chan *chan) +{ + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + unsigned long flags; + + spin_lock_irqsave(&mcf_chan->vchan.lock, flags); + + if (vchan_issue_pending(&mcf_chan->vchan) && !mcf_chan->edesc) + mcf_edma_xfer_desc(mcf_chan); + + spin_unlock_irqrestore(&mcf_chan->vchan.lock, flags); +} + +static void mcf_edma_free_desc(struct virt_dma_desc *vdesc) +{ + struct mcf_edma_desc *mcf_desc; + int i; + struct edma_regs *regs; + + mcf_desc = to_mcf_edma_desc(vdesc); + regs = mcf_desc->echan->edma->membase; + + //trace_tcd(®s->tcd[mcf_desc->echan->slave_id]); + //trace_regs(regs); + + for (i = 0; i < mcf_desc->n_tcds; i++) + dma_pool_free(mcf_desc->echan->tcd_pool, mcf_desc->tcd[i].vtcd, + mcf_desc->tcd[i].ptcd); + kfree(mcf_desc); +} + +static int mcf_edma_irq_init(struct platform_device *pdev, + struct mcf_edma_engine *mcf_edma) +{ + int ret = 0, i; + struct resource *res; + + res = platform_get_resource_byname(pdev, + IORESOURCE_IRQ, "edma-tx-00-15"); + if (!res) + return -1; + + for (ret = 0, i = res->start; i <= res->end; ++i) { + ret |= devm_request_irq(&pdev->dev, i, + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); + } + if (ret) + return ret; + + res = platform_get_resource_byname(pdev, + IORESOURCE_IRQ, "edma-tx-16-55"); + if (!res) + return -1; + + for (ret = 0, i = res->start; i <= res->end; ++i) { + ret |= devm_request_irq(&pdev->dev, i, + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); + } + if (ret) + return ret; + + ret = platform_get_irq_byname(pdev, "edma-tx-56-63"); + if (ret != -ENXIO) { + ret = devm_request_irq(&pdev->dev, ret, + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); + if (ret) + return ret; + } + + ret = platform_get_irq_byname(pdev, "edma-err"); + if (ret != -ENXIO) { + ret = devm_request_irq(&pdev->dev, ret, + mcf_edma_err_handler, 0, "eDMA", mcf_edma); + if (ret) + return ret; + } + + return 0; +} + +static void mcf_edma_irq_free(struct platform_device *pdev, + struct mcf_edma_engine *mcf_edma) +{ + int irq; + struct resource *res; + + res = platform_get_resource_byname(pdev, + IORESOURCE_IRQ, "edma-tx-00-15"); + if (res) { + for (irq = res->start; irq <= res->end; irq++) + devm_free_irq(&pdev->dev, irq, mcf_edma); + } + + res = platform_get_resource_byname(pdev, + IORESOURCE_IRQ, "edma-tx-16-55"); + if (res) { + for (irq = res->start; irq <= res->end; irq++) + devm_free_irq(&pdev->dev, irq, mcf_edma); + } + + irq = platform_get_irq_byname(pdev, "edma-tx-56-63"); + if (irq != -ENXIO) + devm_free_irq(&pdev->dev, irq, mcf_edma); + + irq = platform_get_irq_byname(pdev, "edma-err"); + if (irq != -ENXIO) + devm_free_irq(&pdev->dev, irq, mcf_edma); +} + +static int mcf_edma_probe(struct platform_device *pdev) +{ + struct mcf_edma_platform_data *pdata; + struct mcf_edma_engine *mcf_edma; + struct mcf_edma_chan *mcf_chan; + struct edma_regs *regs; + struct resource *res; + int ret, i, len, chans; + + pdata = dev_get_platdata(&pdev->dev); + if (!pdata) + return PTR_ERR(pdata); + + chans = pdata->dma_channels; + len = sizeof(*mcf_edma) + sizeof(*mcf_chan) * chans; + mcf_edma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL); + if (!mcf_edma) + return -ENOMEM; + + mcf_edma->n_chans = chans; + + if (!mcf_edma->n_chans) { + pr_info("setting default channel number to 64"); + mcf_edma->n_chans = 64; + } + + mutex_init(&mcf_edma->mcf_edma_mutex); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + + mcf_edma->membase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(mcf_edma->membase)) + return PTR_ERR(mcf_edma->membase); + + regs = mcf_edma->membase; + + INIT_LIST_HEAD(&mcf_edma->dma_dev.channels); + for (i = 0; i < mcf_edma->n_chans; i++) { + struct mcf_edma_chan *mcf_chan = &mcf_edma->chans[i]; + + mcf_chan->edma = mcf_edma; + mcf_chan->slave_id = i; + mcf_chan->idle = true; + mcf_chan->vchan.desc_free = mcf_edma_free_desc; + vchan_init(&mcf_chan->vchan, &mcf_edma->dma_dev); + iowrite32(0x0, ®s->tcd[i].csr); + } + + iowrite32(~0, ®s->inth); + iowrite32(~0, ®s->intl); + + ret = mcf_edma_irq_init(pdev, mcf_edma); + if (ret) + return ret; + + dma_cap_set(DMA_PRIVATE, mcf_edma->dma_dev.cap_mask); + dma_cap_set(DMA_SLAVE, mcf_edma->dma_dev.cap_mask); + dma_cap_set(DMA_CYCLIC, mcf_edma->dma_dev.cap_mask); + + mcf_edma->dma_dev.dev = &pdev->dev; + mcf_edma->dma_dev.device_alloc_chan_resources = + mcf_edma_alloc_chan_resources; + mcf_edma->dma_dev.device_free_chan_resources = + mcf_edma_free_chan_resources; + mcf_edma->dma_dev.device_config = mcf_edma_slave_config; + mcf_edma->dma_dev.device_prep_dma_cyclic = + mcf_edma_prep_dma_cyclic; + mcf_edma->dma_dev.device_prep_slave_sg = mcf_edma_prep_slave_sg; + mcf_edma->dma_dev.device_tx_status = mcf_edma_tx_status; + mcf_edma->dma_dev.device_issue_pending = mcf_edma_issue_pending; + + mcf_edma->dma_dev.src_addr_widths = MCF_EDMA_BUSWIDTHS; + mcf_edma->dma_dev.dst_addr_widths = MCF_EDMA_BUSWIDTHS; + mcf_edma->dma_dev.directions = + BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV); + + mcf_edma->dma_dev.filter.fn = mcf_edma_filter_fn; + mcf_edma->dma_dev.filter.map = pdata->slave_map; + mcf_edma->dma_dev.filter.mapcnt = pdata->slavecnt; + + platform_set_drvdata(pdev, mcf_edma); + + ret = dma_async_device_register(&mcf_edma->dma_dev); + if (ret) { + pr_err("Can't register Freescale eDMA engine. (%d)\n", ret); + return ret; + } + + /* Enable round robin arbitration */ + iowrite32(EDMA_CR_ERGA | EDMA_CR_ERCA, ®s->cr); + + return 0; +} + +static void mcf_edma_cleanup_vchan(struct dma_device *dmadev) +{ + struct mcf_edma_chan *chan, *_chan; + + list_for_each_entry_safe(chan, _chan, + &dmadev->channels, vchan.chan.device_node) { + list_del(&chan->vchan.chan.device_node); + tasklet_kill(&chan->vchan.task); + } +} + +static int mcf_edma_remove(struct platform_device *pdev) +{ + struct mcf_edma_engine *mcf_edma = platform_get_drvdata(pdev); + + mcf_edma_irq_free(pdev, mcf_edma); + mcf_edma_cleanup_vchan(&mcf_edma->dma_dev); + dma_async_device_unregister(&mcf_edma->dma_dev); + + return 0; +} + +static struct platform_driver mcf_edma_driver = { + .driver = { + .name = "mcf-edma", + }, + .probe = mcf_edma_probe, + .remove = mcf_edma_remove, +}; + +bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) +{ + if (chan->device->dev->driver == &mcf_edma_driver.driver) { + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); + + return (mcf_chan->slave_id == (int)param); + } + + return false; +} +EXPORT_SYMBOL(mcf_edma_filter_fn); + +static int __init mcf_edma_init(void) +{ + return platform_driver_register(&mcf_edma_driver); +} +subsys_initcall(mcf_edma_init); + +static void __exit mcf_edma_exit(void) +{ + platform_driver_unregister(&mcf_edma_driver); +} +module_exit(mcf_edma_exit); + +MODULE_ALIAS("platform:mcf-edma"); +MODULE_DESCRIPTION("Freescale eDMA engine driver, ColdFire family"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h new file mode 100644 index 000000000000..9a1819acb28f --- /dev/null +++ b/include/linux/platform_data/dma-mcf-edma.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Freescale eDMA platform data, ColdFire SoC's family. + * + * Copyright (c) 2017 Angelo Dureghello <angelo@sysam.it> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __MACH_MCF_EDMA_H__ +#define __MACH_MCF_EDMA_H__ + +struct dma_slave_map; + +bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); + +#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) + +/** + * struct mcf_edma_platform_data - platform specific data for eDMA engine + * + * @ver The eDMA module version. + * @dma_channels The number of eDMA channels. + */ +struct mcf_edma_platform_data { + int dma_channels; + const struct dma_slave_map *slave_map; + int slavecnt; +}; + +#endif /* __MACH_MCF_EDMA_H__ */
This patch adds dma support for NXP mcf5441x (ColdFire) family. ColdFire mcf5441x implements an edma hw module similar to the one implemented in Vybrid VFxxx controllers, but with a slightly different register set, more dma channels (64 instead of 32), a different interrupt mechanism and some other minor differences. For the above reasons, modfying fsl-edma.c was too complex and likely too ugly. From here, the decision to create a different driver, but starting from fsl-edma. The driver has been tested with mcf5441x (stmark2 board) and dspi driver, it worked fine and seems reliable at least as a first initial version. Signed-off-by: Angelo Dureghello <angelo@sysam.it> --- arch/m68k/configs/stmark2_defconfig | 2 + drivers/dma/Kconfig | 11 + drivers/dma/Makefile | 1 + drivers/dma/mcf-edma.c | 847 +++++++++++++++++++++ include/linux/platform_data/dma-mcf-edma.h | 38 + 5 files changed, 899 insertions(+) create mode 100644 drivers/dma/mcf-edma.c create mode 100644 include/linux/platform_data/dma-mcf-edma.h