| Message ID | 1375870770-14263-7-git-send-email-zonque@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 07 August 2013, Daniel Mack wrote: > This patch makes the mmp_pdma controller able to provide DMA resources > in DT environments. > > Signed-off-by: Daniel Mack <zonque@gmail.com> > --- > drivers/dma/mmp_pdma.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c > index 60a1410..d60217a 100644 > --- a/drivers/dma/mmp_pdma.c > +++ b/drivers/dma/mmp_pdma.c > @@ -18,6 +18,7 @@ > #include <linux/platform_data/mmp_dma.h> > #include <linux/dmapool.h> > #include <linux/of_device.h> > +#include <linux/of_dma.h> > #include <linux/of.h> > #include <linux/dma/mmp-pdma.h> > > @@ -783,6 +784,10 @@ static struct of_device_id mmp_pdma_dt_ids[] = { > }; > MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); > > +static struct of_dma_filter_info mmp_pdma_info = { > + .filter_fn = mmp_pdma_filter_fn, > +}; > + > static int mmp_pdma_probe(struct platform_device *op) > { > struct mmp_pdma_device *pdev; > @@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op) > return ret; > } > > + if (op->dev.of_node) { > + mmp_pdma_info.dma_cap = pdev->device.cap_mask; > + > + /* Device-tree DMA controller registration */ > + ret = of_dma_controller_register(op->dev.of_node, > + of_dma_simple_xlate, > + &mmp_pdma_info); of_dma_simple_xlate can not be used if there is a chance that multiple instances of the same dma engine, or multiple different DMA engines are present in the system. I generally advise against using it. Please have a look at the changes that Zhangfei Gao proposed in http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 and see if you can do the same here. Arnd
On 07.08.2013 18:12, Arnd Bergmann wrote: > On Wednesday 07 August 2013, Daniel Mack wrote: >> + if (op->dev.of_node) { >> + mmp_pdma_info.dma_cap = pdev->device.cap_mask; >> + >> + /* Device-tree DMA controller registration */ >> + ret = of_dma_controller_register(op->dev.of_node, >> + of_dma_simple_xlate, >> + &mmp_pdma_info); > > of_dma_simple_xlate can not be used if there is a chance that multiple instances > of the same dma engine, or multiple different DMA engines are present in the > system. Both can't be the case really for PXA, but I see your point. > Please have a look at the changes that Zhangfei Gao proposed in > http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 > and see if you can do the same here. Ok, I can do the same. As I can directly access dma_spec->args[0] from that context, that approach would also solve the problem with the hard-coded filter function, right? Thanks, Daniel
On Wednesday 07 August 2013, Daniel Mack wrote: > > Please have a look at the changes that Zhangfei Gao proposed in > > http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 > > and see if you can do the same here. > > Ok, I can do the same. As I can directly access dma_spec->args[0] from > that context, that approach would also solve the problem with the > hard-coded filter function, right? You mean the problem of using the exported filter function pointer in other drivers? No, since the filter function is used only for the non-DT path, while the xlate function is only used for DT. Arnd
On 07.08.2013 18:12, Arnd Bergmann wrote: > On Wednesday 07 August 2013, Daniel Mack wrote: >> @@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op) >> return ret; >> } >> >> + if (op->dev.of_node) { >> + mmp_pdma_info.dma_cap = pdev->device.cap_mask; >> + >> + /* Device-tree DMA controller registration */ >> + ret = of_dma_controller_register(op->dev.of_node, >> + of_dma_simple_xlate, >> + &mmp_pdma_info); > > of_dma_simple_xlate can not be used if there is a chance that multiple instances > of the same dma engine, or multiple different DMA engines are present in the > system. I generally advise against using it. > > Please have a look at the changes that Zhangfei Gao proposed in > http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 > and see if you can do the same here. Ok, I'll rebase my series on top of that one, hoping that Zhangfei's patch will make it upstream before mine. Daniel
On 07.08.2013 18:12, Arnd Bergmann wrote: > On Wednesday 07 August 2013, Daniel Mack wrote: >> + if (op->dev.of_node) { >> + mmp_pdma_info.dma_cap = pdev->device.cap_mask; >> + >> + /* Device-tree DMA controller registration */ >> + ret = of_dma_controller_register(op->dev.of_node, >> + of_dma_simple_xlate, >> + &mmp_pdma_info); > > of_dma_simple_xlate can not be used if there is a chance that multiple instances > of the same dma engine, or multiple different DMA engines are present in the > system. I generally advise against using it. > > Please have a look at the changes that Zhangfei Gao proposed in > http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 > and see if you can do the same here. I had another look at that and Zhangfei's case is not really applicable to mine, unfortunately. In his case, one specific out of many channels has to be used, depending on the first argument of the phandle. In my case though, the pdma controller may just take any of its channels, and just assign the correct DMA request to it. So if I provide a private xlate function, I need a way to obtain *any* of the channels in my instance. Open-coding that is not easily possible, as I need to hold the dmaengine's local dma_list_mutex for that. I have to dig deeper here, but if anyone has a hint, please let me know :) Daniel
On Fri, Aug 9, 2013 at 9:10 PM, Daniel Mack <zonque@gmail.com> wrote: > On 07.08.2013 18:12, Arnd Bergmann wrote: >> On Wednesday 07 August 2013, Daniel Mack wrote: >>> + if (op->dev.of_node) { >>> + mmp_pdma_info.dma_cap = pdev->device.cap_mask; >>> + >>> + /* Device-tree DMA controller registration */ >>> + ret = of_dma_controller_register(op->dev.of_node, >>> + of_dma_simple_xlate, >>> + &mmp_pdma_info); >> >> of_dma_simple_xlate can not be used if there is a chance that multiple instances >> of the same dma engine, or multiple different DMA engines are present in the >> system. I generally advise against using it. >> >> Please have a look at the changes that Zhangfei Gao proposed in >> http://comments.gmane.org/gmane.linux.ports.arm.kernel/249077 >> and see if you can do the same here. > > I had another look at that and Zhangfei's case is not really applicable > to mine, unfortunately. > > In his case, one specific out of many channels has to be used, depending > on the first argument of the phandle. In my case though, the pdma > controller may just take any of its channels, and just assign the > correct DMA request to it. Dear Daniel Though any physical channel is workable, the virtual channel does not. Each device has to set specific request line. pdma.c chan->drcmr = cfg->slave_id; nand.c conf.slave_id = info->drcmr_dat; The specific virtual channel can be directly specificied by request line. While pdma.c choose the free physical channel inside, which is transparent to client. It should be same. Thanks > > So if I provide a private xlate function, I need a way to obtain *any* > of the channels in my instance. Open-coding that is not easily possible, > as I need to hold the dmaengine's local dma_list_mutex for that. > > I have to dig deeper here, but if anyone has a hint, please let me know :) > > > Daniel > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Friday 09 August 2013, zhangfei gao wrote: > On Fri, Aug 9, 2013 at 9:10 PM, Daniel Mack <zonque@gmail.com> wrote: > > On 07.08.2013 18:12, Arnd Bergmann wrote: > > > > I had another look at that and Zhangfei's case is not really applicable > > to mine, unfortunately. > > > > In his case, one specific out of many channels has to be used, depending > > on the first argument of the phandle. In my case though, the pdma > > controller may just take any of its channels, and just assign the > > correct DMA request to it. This should still be fine since your driver can keep track of which channels are currently in use. The locking can be done inside of the dmaengine core, but you might have to retry if the channel is getting acquired between your lookup and the lock. > Though any physical channel is workable, the virtual channel does not. > Each device has to set specific request line. > > pdma.c > chan->drcmr = cfg->slave_id; > nand.c > conf.slave_id = info->drcmr_dat; > > The specific virtual channel can be directly specificied by request line. > While pdma.c choose the free physical channel inside, which is > transparent to client. It is a bug to override the slave_id value from the dma slave driver when using dma_request_slave_channel, and the dmaengine driver should not allow that. The slave_config function should only be used to set the configuration parts that the slave driver knows about, which does not include the slave_id in this case (since there is no platform data). Arnd
diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c index 60a1410..d60217a 100644 --- a/drivers/dma/mmp_pdma.c +++ b/drivers/dma/mmp_pdma.c @@ -18,6 +18,7 @@ #include <linux/platform_data/mmp_dma.h> #include <linux/dmapool.h> #include <linux/of_device.h> +#include <linux/of_dma.h> #include <linux/of.h> #include <linux/dma/mmp-pdma.h> @@ -783,6 +784,10 @@ static struct of_device_id mmp_pdma_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, mmp_pdma_dt_ids); +static struct of_dma_filter_info mmp_pdma_info = { + .filter_fn = mmp_pdma_filter_fn, +}; + static int mmp_pdma_probe(struct platform_device *op) { struct mmp_pdma_device *pdev; @@ -869,6 +874,19 @@ static int mmp_pdma_probe(struct platform_device *op) return ret; } + if (op->dev.of_node) { + mmp_pdma_info.dma_cap = pdev->device.cap_mask; + + /* Device-tree DMA controller registration */ + ret = of_dma_controller_register(op->dev.of_node, + of_dma_simple_xlate, + &mmp_pdma_info); + if (ret < 0) { + dev_err(&op->dev, "of_dma_controller_register failed\n"); + return ret; + } + } + dev_info(pdev->device.dev, "initialized\n"); return 0; }
This patch makes the mmp_pdma controller able to provide DMA resources in DT environments. Signed-off-by: Daniel Mack <zonque@gmail.com> --- drivers/dma/mmp_pdma.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)