* [PATCH] dma: of-dma: return error when 'dma-cells' not found @ 2013-03-05 9:25 Padmavathi Venna 2013-03-05 9:25 ` [PATCH V2] DMA: PL330: Add check if device tree compatible Padmavathi Venna 2013-03-05 9:43 ` [PATCH] dma: of-dma: return error when 'dma-cells' not found Rob Herring 0 siblings, 2 replies; 12+ messages in thread From: Padmavathi Venna @ 2013-03-05 9:25 UTC (permalink / raw) To: linux-arm-kernel This patch returns error when 'dma-cells' property not found in the corresponding device node. With out this change there is a crash in the generic dma incompatible platforms. Signed-off-by: Padmavathi Venna <padma.v@samsung.com> --- Based on Vinod Koul next branch. drivers/dma/of-dma.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 69d04d2..46aca0d 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -92,6 +92,7 @@ int of_dma_controller_register(struct device_node *np, void *data) { struct of_dma *ofdma; + const __be32 *ip; int nbcells; if (!np || !of_dma_xlate) { @@ -103,7 +104,12 @@ int of_dma_controller_register(struct device_node *np, if (!ofdma) return -ENOMEM; - nbcells = be32_to_cpup(of_get_property(np, "#dma-cells", NULL)); + ip = of_get_property(np, "#dma-cells", NULL); + if (!ip) + return -ENXIO; + + nbcells = be32_to_cpup(ip); + if (!nbcells) { pr_err("%s: #dma-cells property is missing or invalid\n", __func__); -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-03-05 9:25 [PATCH] dma: of-dma: return error when 'dma-cells' not found Padmavathi Venna @ 2013-03-05 9:25 ` Padmavathi Venna 2013-03-13 5:36 ` Padma Venkat 2013-03-21 9:39 ` Vinod Koul 2013-03-05 9:43 ` [PATCH] dma: of-dma: return error when 'dma-cells' not found Rob Herring 1 sibling, 2 replies; 12+ messages in thread From: Padmavathi Venna @ 2013-03-05 9:25 UTC (permalink / raw) To: linux-arm-kernel This patch register the dma controller with generic dma helpers only in DT case. This also adds some extra error handling in the driver. Signed-off-by: Padmavathi Venna <padma.v@samsung.com> Reported-by: Sachin Kamat <sachin.kamat@linaro.org> --- Based on Vinod Koul next branch. Changes since V1: - return silently when of_dma_controller_register fails, as suggested by Arnd. drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- 1 files changed, 27 insertions(+), 11 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 7181531..5dbc594 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) { struct dma_pl330_platdata *pdat; struct dma_pl330_dmac *pdmac; - struct dma_pl330_chan *pch; + struct dma_pl330_chan *pch, *_p; struct pl330_info *pi; struct dma_device *pd; struct resource *res; @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) ret = dma_async_device_register(pd); if (ret) { dev_err(&adev->dev, "unable to register DMAC\n"); - goto probe_err2; + goto probe_err3; + } + + if (adev->dev.of_node) { + ret = of_dma_controller_register(adev->dev.of_node, + of_dma_pl330_xlate, pdmac); + if (ret) { + dev_err(&adev->dev, + "unable to register DMA to the generic DT DMA helpers\n"); + } } dev_info(&adev->dev, @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, pi->pcfg.num_peri, pi->pcfg.num_events); - ret = of_dma_controller_register(adev->dev.of_node, - of_dma_pl330_xlate, pdmac); - if (ret) { - dev_err(&adev->dev, - "unable to register DMA to the generic DT DMA helpers\n"); - goto probe_err2; - } - return 0; +probe_err3: + amba_set_drvdata(adev, NULL); + /* Idle the DMAC */ + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, + chan.device_node) { + + /* Remove the channel */ + list_del(&pch->chan.device_node); + + /* Flush the channel */ + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); + pl330_free_chan_resources(&pch->chan); + } probe_err2: pl330_del(pi); probe_err1: @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) if (!pdmac) return 0; - of_dma_controller_free(adev->dev.of_node); + if (adev->dev.of_node) + of_dma_controller_free(adev->dev.of_node); + dma_async_device_unregister(&pdmac->ddma); amba_set_drvdata(adev, NULL); /* Idle the DMAC */ -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-03-05 9:25 ` [PATCH V2] DMA: PL330: Add check if device tree compatible Padmavathi Venna @ 2013-03-13 5:36 ` Padma Venkat 2013-03-21 9:39 ` Vinod Koul 1 sibling, 0 replies; 12+ messages in thread From: Padma Venkat @ 2013-03-13 5:36 UTC (permalink / raw) To: linux-arm-kernel Hi, On Tue, Mar 5, 2013 at 2:55 PM, Padmavathi Venna <padma.v@samsung.com> wrote: > This patch register the dma controller with generic dma helpers only > in DT case. This also adds some extra error handling in the driver. > > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > Reported-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > > Based on Vinod Koul next branch. > > Changes since V1: > - return silently when of_dma_controller_register fails, as > suggested by Arnd. > > drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- > 1 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > index 7181531..5dbc594 100644 > --- a/drivers/dma/pl330.c > +++ b/drivers/dma/pl330.c > @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > { > struct dma_pl330_platdata *pdat; > struct dma_pl330_dmac *pdmac; > - struct dma_pl330_chan *pch; > + struct dma_pl330_chan *pch, *_p; > struct pl330_info *pi; > struct dma_device *pd; > struct resource *res; > @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > ret = dma_async_device_register(pd); > if (ret) { > dev_err(&adev->dev, "unable to register DMAC\n"); > - goto probe_err2; > + goto probe_err3; > + } > + > + if (adev->dev.of_node) { > + ret = of_dma_controller_register(adev->dev.of_node, > + of_dma_pl330_xlate, pdmac); > + if (ret) { > + dev_err(&adev->dev, > + "unable to register DMA to the generic DT DMA helpers\n"); > + } > } > > dev_info(&adev->dev, > @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, > pi->pcfg.num_peri, pi->pcfg.num_events); > > - ret = of_dma_controller_register(adev->dev.of_node, > - of_dma_pl330_xlate, pdmac); > - if (ret) { > - dev_err(&adev->dev, > - "unable to register DMA to the generic DT DMA helpers\n"); > - goto probe_err2; > - } > - > return 0; > +probe_err3: > + amba_set_drvdata(adev, NULL); > > + /* Idle the DMAC */ > + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, > + chan.device_node) { > + > + /* Remove the channel */ > + list_del(&pch->chan.device_node); > + > + /* Flush the channel */ > + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); > + pl330_free_chan_resources(&pch->chan); > + } > probe_err2: > pl330_del(pi); > probe_err1: > @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) > if (!pdmac) > return 0; > > - of_dma_controller_free(adev->dev.of_node); > + if (adev->dev.of_node) > + of_dma_controller_free(adev->dev.of_node); > > + dma_async_device_unregister(&pdmac->ddma); Any comment on this patch? Vinod, Can you please take this patch into your tree? > amba_set_drvdata(adev, NULL); > > /* Idle the DMAC */ > -- > 1.7.4.4 > Thanks Padma ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-03-05 9:25 ` [PATCH V2] DMA: PL330: Add check if device tree compatible Padmavathi Venna 2013-03-13 5:36 ` Padma Venkat @ 2013-03-21 9:39 ` Vinod Koul 2013-04-01 13:13 ` Rob Herring 1 sibling, 1 reply; 12+ messages in thread From: Vinod Koul @ 2013-03-21 9:39 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 05, 2013 at 02:55:31PM +0530, Padmavathi Venna wrote: > This patch register the dma controller with generic dma helpers only > in DT case. This also adds some extra error handling in the driver. > > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > Reported-by: Sachin Kamat <sachin.kamat@linaro.org> > --- > > Based on Vinod Koul next branch. > > Changes since V1: > - return silently when of_dma_controller_register fails, as > suggested by Arnd. > > drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- > 1 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > index 7181531..5dbc594 100644 > --- a/drivers/dma/pl330.c > +++ b/drivers/dma/pl330.c > @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > { > struct dma_pl330_platdata *pdat; > struct dma_pl330_dmac *pdmac; > - struct dma_pl330_chan *pch; > + struct dma_pl330_chan *pch, *_p; > struct pl330_info *pi; > struct dma_device *pd; > struct resource *res; > @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > ret = dma_async_device_register(pd); > if (ret) { > dev_err(&adev->dev, "unable to register DMAC\n"); > - goto probe_err2; > + goto probe_err3; > + } > + > + if (adev->dev.of_node) { > + ret = of_dma_controller_register(adev->dev.of_node, > + of_dma_pl330_xlate, pdmac); > + if (ret) { > + dev_err(&adev->dev, > + "unable to register DMA to the generic DT DMA helpers\n"); Indent? > + } > } > > dev_info(&adev->dev, > @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, > pi->pcfg.num_peri, pi->pcfg.num_events); > > - ret = of_dma_controller_register(adev->dev.of_node, > - of_dma_pl330_xlate, pdmac); > - if (ret) { > - dev_err(&adev->dev, > - "unable to register DMA to the generic DT DMA helpers\n"); > - goto probe_err2; > - } > - > return 0; > +probe_err3: > + amba_set_drvdata(adev, NULL); > > + /* Idle the DMAC */ > + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, > + chan.device_node) { > + > + /* Remove the channel */ > + list_del(&pch->chan.device_node); > + > + /* Flush the channel */ > + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); > + pl330_free_chan_resources(&pch->chan); free_chan for error handling in probe? > + } > probe_err2: > pl330_del(pi); > probe_err1: > @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) > if (!pdmac) > return 0; > > - of_dma_controller_free(adev->dev.of_node); > + if (adev->dev.of_node) > + of_dma_controller_free(adev->dev.of_node); > > + dma_async_device_unregister(&pdmac->ddma); > amba_set_drvdata(adev, NULL); > > /* Idle the DMAC */ > -- > 1.7.4.4 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-03-21 9:39 ` Vinod Koul @ 2013-04-01 13:13 ` Rob Herring 2013-04-01 18:21 ` Vinod Koul 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2013-04-01 13:13 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 21, 2013 at 4:39 AM, Vinod Koul <vinod.koul@intel.com> wrote: > On Tue, Mar 05, 2013 at 02:55:31PM +0530, Padmavathi Venna wrote: >> This patch register the dma controller with generic dma helpers only >> in DT case. This also adds some extra error handling in the driver. >> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> Reported-by: Sachin Kamat <sachin.kamat@linaro.org> What's the status on this? It is needed for 3.9 to fix pl330 on highbank. Rob >> --- >> >> Based on Vinod Koul next branch. >> >> Changes since V1: >> - return silently when of_dma_controller_register fails, as >> suggested by Arnd. >> >> drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- >> 1 files changed, 27 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c >> index 7181531..5dbc594 100644 >> --- a/drivers/dma/pl330.c >> +++ b/drivers/dma/pl330.c >> @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) >> { >> struct dma_pl330_platdata *pdat; >> struct dma_pl330_dmac *pdmac; >> - struct dma_pl330_chan *pch; >> + struct dma_pl330_chan *pch, *_p; >> struct pl330_info *pi; >> struct dma_device *pd; >> struct resource *res; >> @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) >> ret = dma_async_device_register(pd); >> if (ret) { >> dev_err(&adev->dev, "unable to register DMAC\n"); >> - goto probe_err2; >> + goto probe_err3; >> + } >> + >> + if (adev->dev.of_node) { >> + ret = of_dma_controller_register(adev->dev.of_node, >> + of_dma_pl330_xlate, pdmac); >> + if (ret) { >> + dev_err(&adev->dev, >> + "unable to register DMA to the generic DT DMA helpers\n"); > Indent? >> + } >> } >> >> dev_info(&adev->dev, >> @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) >> pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, >> pi->pcfg.num_peri, pi->pcfg.num_events); >> >> - ret = of_dma_controller_register(adev->dev.of_node, >> - of_dma_pl330_xlate, pdmac); >> - if (ret) { >> - dev_err(&adev->dev, >> - "unable to register DMA to the generic DT DMA helpers\n"); >> - goto probe_err2; >> - } >> - >> return 0; >> +probe_err3: >> + amba_set_drvdata(adev, NULL); >> >> + /* Idle the DMAC */ >> + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, >> + chan.device_node) { >> + >> + /* Remove the channel */ >> + list_del(&pch->chan.device_node); >> + >> + /* Flush the channel */ >> + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); >> + pl330_free_chan_resources(&pch->chan); > free_chan for error handling in probe? > >> + } >> probe_err2: >> pl330_del(pi); >> probe_err1: >> @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) >> if (!pdmac) >> return 0; >> >> - of_dma_controller_free(adev->dev.of_node); >> + if (adev->dev.of_node) >> + of_dma_controller_free(adev->dev.of_node); >> >> + dma_async_device_unregister(&pdmac->ddma); >> amba_set_drvdata(adev, NULL); >> >> /* Idle the DMAC */ >> -- >> 1.7.4.4 >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-04-01 13:13 ` Rob Herring @ 2013-04-01 18:21 ` Vinod Koul 2013-04-02 2:58 ` Padma Venkat 0 siblings, 1 reply; 12+ messages in thread From: Vinod Koul @ 2013-04-01 18:21 UTC (permalink / raw) To: linux-arm-kernel On Mon, Apr 01, 2013 at 08:13:31AM -0500, Rob Herring wrote: > On Thu, Mar 21, 2013 at 4:39 AM, Vinod Koul <vinod.koul@intel.com> wrote: > > On Tue, Mar 05, 2013 at 02:55:31PM +0530, Padmavathi Venna wrote: > >> This patch register the dma controller with generic dma helpers only > >> in DT case. This also adds some extra error handling in the driver. > >> > >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > >> Reported-by: Sachin Kamat <sachin.kamat@linaro.org> > > What's the status on this? It is needed for 3.9 to fix pl330 on highbank. Well the status is that submitter has been rude. I had few questions and comments on this patch and they are yet to be addressed. -- ~Vinod > > Rob > > >> --- > >> > >> Based on Vinod Koul next branch. > >> > >> Changes since V1: > >> - return silently when of_dma_controller_register fails, as > >> suggested by Arnd. > >> > >> drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- > >> 1 files changed, 27 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > >> index 7181531..5dbc594 100644 > >> --- a/drivers/dma/pl330.c > >> +++ b/drivers/dma/pl330.c > >> @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > >> { > >> struct dma_pl330_platdata *pdat; > >> struct dma_pl330_dmac *pdmac; > >> - struct dma_pl330_chan *pch; > >> + struct dma_pl330_chan *pch, *_p; > >> struct pl330_info *pi; > >> struct dma_device *pd; > >> struct resource *res; > >> @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > >> ret = dma_async_device_register(pd); > >> if (ret) { > >> dev_err(&adev->dev, "unable to register DMAC\n"); > >> - goto probe_err2; > >> + goto probe_err3; > >> + } > >> + > >> + if (adev->dev.of_node) { > >> + ret = of_dma_controller_register(adev->dev.of_node, > >> + of_dma_pl330_xlate, pdmac); > >> + if (ret) { > >> + dev_err(&adev->dev, > >> + "unable to register DMA to the generic DT DMA helpers\n"); > > Indent? > >> + } > >> } > >> > >> dev_info(&adev->dev, > >> @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > >> pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, > >> pi->pcfg.num_peri, pi->pcfg.num_events); > >> > >> - ret = of_dma_controller_register(adev->dev.of_node, > >> - of_dma_pl330_xlate, pdmac); > >> - if (ret) { > >> - dev_err(&adev->dev, > >> - "unable to register DMA to the generic DT DMA helpers\n"); > >> - goto probe_err2; > >> - } > >> - > >> return 0; > >> +probe_err3: > >> + amba_set_drvdata(adev, NULL); > >> > >> + /* Idle the DMAC */ > >> + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, > >> + chan.device_node) { > >> + > >> + /* Remove the channel */ > >> + list_del(&pch->chan.device_node); > >> + > >> + /* Flush the channel */ > >> + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); > >> + pl330_free_chan_resources(&pch->chan); > > free_chan for error handling in probe? > > > >> + } > >> probe_err2: > >> pl330_del(pi); > >> probe_err1: > >> @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) > >> if (!pdmac) > >> return 0; > >> > >> - of_dma_controller_free(adev->dev.of_node); > >> + if (adev->dev.of_node) > >> + of_dma_controller_free(adev->dev.of_node); > >> > >> + dma_async_device_unregister(&pdmac->ddma); > >> amba_set_drvdata(adev, NULL); > >> > >> /* Idle the DMAC */ > >> -- > >> 1.7.4.4 > >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2] DMA: PL330: Add check if device tree compatible 2013-04-01 18:21 ` Vinod Koul @ 2013-04-02 2:58 ` Padma Venkat 0 siblings, 0 replies; 12+ messages in thread From: Padma Venkat @ 2013-04-02 2:58 UTC (permalink / raw) To: linux-arm-kernel Hi Vinod, I apologies for the delayed reply. Last week I was out of station and no access to mails. I will send another patch addressing your comments. Thanks Padma On Mon, Apr 1, 2013 at 11:51 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > On Mon, Apr 01, 2013 at 08:13:31AM -0500, Rob Herring wrote: > > On Thu, Mar 21, 2013 at 4:39 AM, Vinod Koul <vinod.koul@intel.com> wrote: > > > On Tue, Mar 05, 2013 at 02:55:31PM +0530, Padmavathi Venna wrote: > > >> This patch register the dma controller with generic dma helpers only > > >> in DT case. This also adds some extra error handling in the driver. > > >> > > >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > > >> Reported-by: Sachin Kamat <sachin.kamat@linaro.org> > > > > What's the status on this? It is needed for 3.9 to fix pl330 on highbank. > Well the status is that submitter has been rude. I had few questions and > comments on this patch and they are yet to be addressed. > > -- > ~Vinod > > > > Rob > > > > >> --- > > >> > > >> Based on Vinod Koul next branch. > > >> > > >> Changes since V1: > > >> - return silently when of_dma_controller_register fails, as > > >> suggested by Arnd. > > >> > > >> drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++----------- > > >> 1 files changed, 27 insertions(+), 11 deletions(-) > > >> > > >> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c > > >> index 7181531..5dbc594 100644 > > >> --- a/drivers/dma/pl330.c > > >> +++ b/drivers/dma/pl330.c > > >> @@ -2882,7 +2882,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > > >> { > > >> struct dma_pl330_platdata *pdat; > > >> struct dma_pl330_dmac *pdmac; > > >> - struct dma_pl330_chan *pch; > > >> + struct dma_pl330_chan *pch, *_p; > > >> struct pl330_info *pi; > > >> struct dma_device *pd; > > >> struct resource *res; > > >> @@ -2984,7 +2984,16 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > > >> ret = dma_async_device_register(pd); > > >> if (ret) { > > >> dev_err(&adev->dev, "unable to register DMAC\n"); > > >> - goto probe_err2; > > >> + goto probe_err3; > > >> + } > > >> + > > >> + if (adev->dev.of_node) { > > >> + ret = of_dma_controller_register(adev->dev.of_node, > > >> + of_dma_pl330_xlate, pdmac); > > >> + if (ret) { > > >> + dev_err(&adev->dev, > > >> + "unable to register DMA to the generic DT DMA helpers\n"); > > > Indent? Okey. > > > >> + } > > >> } > > >> > > >> dev_info(&adev->dev, > > >> @@ -2995,16 +3004,21 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id) > > >> pi->pcfg.data_bus_width / 8, pi->pcfg.num_chan, > > >> pi->pcfg.num_peri, pi->pcfg.num_events); > > >> > > >> - ret = of_dma_controller_register(adev->dev.of_node, > > >> - of_dma_pl330_xlate, pdmac); > > >> - if (ret) { > > >> - dev_err(&adev->dev, > > >> - "unable to register DMA to the generic DT DMA helpers\n"); > > >> - goto probe_err2; > > >> - } > > >> - > > >> return 0; > > >> +probe_err3: > > >> + amba_set_drvdata(adev, NULL); > > >> > > >> + /* Idle the DMAC */ > > >> + list_for_each_entry_safe(pch, _p, &pdmac->ddma.channels, > > >> + chan.device_node) { > > >> + > > >> + /* Remove the channel */ > > >> + list_del(&pch->chan.device_node); > > >> + > > >> + /* Flush the channel */ > > >> + pl330_control(&pch->chan, DMA_TERMINATE_ALL, 0); > > >> + pl330_free_chan_resources(&pch->chan); > > > free_chan for error handling in probe? Will remove this. > > > > > > >> + } > > >> probe_err2: > > >> pl330_del(pi); > > >> probe_err1: > > >> @@ -3023,8 +3037,10 @@ static int pl330_remove(struct amba_device *adev) > > >> if (!pdmac) > > >> return 0; > > >> > > >> - of_dma_controller_free(adev->dev.of_node); > > >> + if (adev->dev.of_node) > > >> + of_dma_controller_free(adev->dev.of_node); > > >> > > >> + dma_async_device_unregister(&pdmac->ddma); > > >> amba_set_drvdata(adev, NULL); > > >> > > >> /* Idle the DMAC */ > > >> -- > > >> 1.7.4.4 > > >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] dma: of-dma: return error when 'dma-cells' not found 2013-03-05 9:25 [PATCH] dma: of-dma: return error when 'dma-cells' not found Padmavathi Venna 2013-03-05 9:25 ` [PATCH V2] DMA: PL330: Add check if device tree compatible Padmavathi Venna @ 2013-03-05 9:43 ` Rob Herring 2013-03-05 10:04 ` Padma Venkat 2013-03-05 10:15 ` Arnd Bergmann 1 sibling, 2 replies; 12+ messages in thread From: Rob Herring @ 2013-03-05 9:43 UTC (permalink / raw) To: linux-arm-kernel On 03/05/2013 03:25 AM, Padmavathi Venna wrote: > This patch returns error when 'dma-cells' property not found > in the corresponding device node. With out this change there > is a crash in the generic dma incompatible platforms. > > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> NAK. #dma-cells should be optional. It is not needed for platforms supporting memory to memory transfers only and should therefore be optional. You cannot assume the dtb can be updated and kernel changes need to work with old dtbs. I've submitted patches to address this and fix the crash: https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-February/028769.html Rob > --- > > Based on Vinod Koul next branch. > > drivers/dma/of-dma.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c > index 69d04d2..46aca0d 100644 > --- a/drivers/dma/of-dma.c > +++ b/drivers/dma/of-dma.c > @@ -92,6 +92,7 @@ int of_dma_controller_register(struct device_node *np, > void *data) > { > struct of_dma *ofdma; > + const __be32 *ip; > int nbcells; > > if (!np || !of_dma_xlate) { > @@ -103,7 +104,12 @@ int of_dma_controller_register(struct device_node *np, > if (!ofdma) > return -ENOMEM; > > - nbcells = be32_to_cpup(of_get_property(np, "#dma-cells", NULL)); > + ip = of_get_property(np, "#dma-cells", NULL); > + if (!ip) > + return -ENXIO; > + > + nbcells = be32_to_cpup(ip); > + > if (!nbcells) { > pr_err("%s: #dma-cells property is missing or invalid\n", > __func__); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] dma: of-dma: return error when 'dma-cells' not found 2013-03-05 9:43 ` [PATCH] dma: of-dma: return error when 'dma-cells' not found Rob Herring @ 2013-03-05 10:04 ` Padma Venkat 2013-03-05 10:15 ` Arnd Bergmann 1 sibling, 0 replies; 12+ messages in thread From: Padma Venkat @ 2013-03-05 10:04 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 5, 2013 at 3:13 PM, Rob Herring <robherring2@gmail.com> wrote: > On 03/05/2013 03:25 AM, Padmavathi Venna wrote: >> This patch returns error when 'dma-cells' property not found >> in the corresponding device node. With out this change there >> is a crash in the generic dma incompatible platforms. >> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > > NAK. > > #dma-cells should be optional. It is not needed for platforms supporting > memory to memory transfers only and should therefore be optional. You > cannot assume the dtb can be updated and kernel changes need to work > with old dtbs. I've submitted patches to address this and fix the crash: > > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-February/028769.html > Okay. Thanks Padma > Rob >> --- >> >> Based on Vinod Koul next branch. >> >> drivers/dma/of-dma.c | 8 +++++++- >> 1 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c >> index 69d04d2..46aca0d 100644 >> --- a/drivers/dma/of-dma.c >> +++ b/drivers/dma/of-dma.c >> @@ -92,6 +92,7 @@ int of_dma_controller_register(struct device_node *np, >> void *data) >> { >> struct of_dma *ofdma; >> + const __be32 *ip; >> int nbcells; >> >> if (!np || !of_dma_xlate) { >> @@ -103,7 +104,12 @@ int of_dma_controller_register(struct device_node *np, >> if (!ofdma) >> return -ENOMEM; >> >> - nbcells = be32_to_cpup(of_get_property(np, "#dma-cells", NULL)); >> + ip = of_get_property(np, "#dma-cells", NULL); >> + if (!ip) >> + return -ENXIO; >> + >> + nbcells = be32_to_cpup(ip); >> + >> if (!nbcells) { >> pr_err("%s: #dma-cells property is missing or invalid\n", >> __func__); >> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] dma: of-dma: return error when 'dma-cells' not found 2013-03-05 9:43 ` [PATCH] dma: of-dma: return error when 'dma-cells' not found Rob Herring 2013-03-05 10:04 ` Padma Venkat @ 2013-03-05 10:15 ` Arnd Bergmann 2013-03-05 14:56 ` Rob Herring 1 sibling, 1 reply; 12+ messages in thread From: Arnd Bergmann @ 2013-03-05 10:15 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 05 March 2013 03:43:52 Rob Herring wrote: > On 03/05/2013 03:25 AM, Padmavathi Venna wrote: > > This patch returns error when 'dma-cells' property not found > > in the corresponding device node. With out this change there > > is a crash in the generic dma incompatible platforms. > > > > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > > NAK. > > #dma-cells should be optional. It is not needed for platforms supporting > memory to memory transfers only and should therefore be optional. You > cannot assume the dtb can be updated and kernel changes need to work > with old dtbs. I've submitted patches to address this and fix the crash: > > https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-February/028769.html Why would you call of_dma_controller_register() for a dma engine that does not support slave channels, when that is the only purpose of that interface? Note that the binding defines #dma-cells as required, and it does not make any sense otherwise. Arnd ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] dma: of-dma: return error when 'dma-cells' not found 2013-03-05 10:15 ` Arnd Bergmann @ 2013-03-05 14:56 ` Rob Herring 2013-03-05 19:48 ` Arnd Bergmann 0 siblings, 1 reply; 12+ messages in thread From: Rob Herring @ 2013-03-05 14:56 UTC (permalink / raw) To: linux-arm-kernel On 03/05/2013 04:15 AM, Arnd Bergmann wrote: > On Tuesday 05 March 2013 03:43:52 Rob Herring wrote: >> On 03/05/2013 03:25 AM, Padmavathi Venna wrote: >>> This patch returns error when 'dma-cells' property not found >>> in the corresponding device node. With out this change there >>> is a crash in the generic dma incompatible platforms. >>> >>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> >> NAK. >> >> #dma-cells should be optional. It is not needed for platforms supporting >> memory to memory transfers only and should therefore be optional. You >> cannot assume the dtb can be updated and kernel changes need to work >> with old dtbs. I've submitted patches to address this and fix the crash: >> >> https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-February/028769.html > > Why would you call of_dma_controller_register() for a dma > engine that does not support slave channels, when that is the > only purpose of that interface? Well maybe then that function should be allowed to fail without erroring out. I just fixed it a the line that failed. Doing be32_to_cpup directly on a function return that can be NULL is not correct either. > Note that the binding defines #dma-cells as required, and it > does not make any sense otherwise. The 2nd patch I submitted changes that. It does not make sense to require it if you have no requests and hence will never have a phandle reference in a slave device. The simple fact is that the pl330 had an existing binding that worked for the memory to memory only case and kernel changes broke this. Kernel changes should not break existing device-trees. Rob ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] dma: of-dma: return error when 'dma-cells' not found 2013-03-05 14:56 ` Rob Herring @ 2013-03-05 19:48 ` Arnd Bergmann 0 siblings, 0 replies; 12+ messages in thread From: Arnd Bergmann @ 2013-03-05 19:48 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 05 March 2013, Rob Herring wrote: > > Why would you call of_dma_controller_register() for a dma > > engine that does not support slave channels, when that is the > > only purpose of that interface? > > Well maybe then that function should be allowed to fail without erroring > out. I just fixed it a the line that failed. We have a lot of interfaces that fall back to silently doing nothing when there is no need. > Doing be32_to_cpup directly > on a function return that can be NULL is not correct either. Agreed. > > Note that the binding defines #dma-cells as required, and it > > does not make any sense otherwise. > > The 2nd patch I submitted changes that. It does not make sense to > require it if you have no requests and hence will never have a phandle > reference in a slave device. I still don't see why you would want to use the binding for dma slaves to describe a dmaengine that does not have slaves. > The simple fact is that the pl330 had an > existing binding that worked for the memory to memory only case and > kernel changes broke this. Kernel changes should not break existing > device-trees. Of course we should not break the existing device tree, but I think it would be more sensible to change the pl330 specific binding in this case to require the use of the generic dma slave binding only when there are slaves connected to it. If the #dma-cells property is absent, the pl330 driver can be changed not to call of_dma_controller_register. Arnd ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-04-02 2:58 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-05 9:25 [PATCH] dma: of-dma: return error when 'dma-cells' not found Padmavathi Venna 2013-03-05 9:25 ` [PATCH V2] DMA: PL330: Add check if device tree compatible Padmavathi Venna 2013-03-13 5:36 ` Padma Venkat 2013-03-21 9:39 ` Vinod Koul 2013-04-01 13:13 ` Rob Herring 2013-04-01 18:21 ` Vinod Koul 2013-04-02 2:58 ` Padma Venkat 2013-03-05 9:43 ` [PATCH] dma: of-dma: return error when 'dma-cells' not found Rob Herring 2013-03-05 10:04 ` Padma Venkat 2013-03-05 10:15 ` Arnd Bergmann 2013-03-05 14:56 ` Rob Herring 2013-03-05 19:48 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).