| Message ID | 1347662517-4210-3-git-send-email-jon-hunter@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2012-09-14 at 17:41 -0500, Jon Hunter wrote: > +/** > + * dma_request_slave_channel - try to allocate an exclusive slave > channel > + * @dev: pointer to client device structure > + * @name: slave channel name > + */ > +struct dma_chan *dma_request_slave_channel(struct device *dev, char *name) > +{ > + /* If device-tree is present get slave info from here */ > + if (dev->of_node) > + return of_dma_request_slave_channel(dev->of_node, name); > + Shouldn't this be conditionally compiled only when OF is built. I think this might be problematic for systems which doesn't have device tree. Or perhaps you can declare these symbols as dummy in of_dma.h when device tree is not selected. > + return NULL; > +} > +EXPORT_SYMBOL_GPL(dma_request_slave_channel);
On Monday 17 September 2012, Vinod Koul wrote: > On Fri, 2012-09-14 at 17:41 -0500, Jon Hunter wrote: > > +/** > > + * dma_request_slave_channel - try to allocate an exclusive slave > > channel > > + * @dev: pointer to client device structure > > + * @name: slave channel name > > + */ > > +struct dma_chan *dma_request_slave_channel(struct device *dev, char *name) > > +{ > > + /* If device-tree is present get slave info from here */ > > + if (dev->of_node) > > + return of_dma_request_slave_channel(dev->of_node, name); > > + > Shouldn't this be conditionally compiled only when OF is built. I think > this might be problematic for systems which doesn't have device tree. > Or perhaps you can declare these symbols as dummy in of_dma.h when > device tree is not selected. Right, good point. I'd prefer the dummy functions, since that is in line with what a lot of other subsystems do: #ifdef CONFIG_OF extern int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) (struct of_phandle_args *, struct of_dma *), void *data); extern void of_dma_controller_free(struct device_node *np); extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, char *name); extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma); #else static inline int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) (struct of_phandle_args *, struct of_dma *), void *data) { return -ENODEV; } static inline void of_dma_controller_free(struct device_node *np) { } static inline struct dma_chan *of_dma_request_slave_channel(struct device_node *np, char *name) { return NULL; } static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma) { return NULL; } #endif I believe that Jon is on vacation this week, so if this is the only issue holding up the merge, maybe you can change this in his patch directly, or I can send an updated version if you prefer. Arnd
On Mon, Sep 17, 2012 at 11:59:27AM +0000, Arnd Bergmann wrote: > On Monday 17 September 2012, Vinod Koul wrote: > > On Fri, 2012-09-14 at 17:41 -0500, Jon Hunter wrote: > > > +/** > > > + * dma_request_slave_channel - try to allocate an exclusive slave > > > channel > > > + * @dev: pointer to client device structure > > > + * @name: slave channel name > > > + */ > > > +struct dma_chan *dma_request_slave_channel(struct device *dev, char *name) > > > +{ > > > + /* If device-tree is present get slave info from here */ > > > + if (dev->of_node) > > > + return of_dma_request_slave_channel(dev->of_node, name); > > > + > > Shouldn't this be conditionally compiled only when OF is built. I think > > this might be problematic for systems which doesn't have device tree. > > Or perhaps you can declare these symbols as dummy in of_dma.h when > > device tree is not selected. > > Right, good point. I'd prefer the dummy functions, since that is in line > with what a lot of other subsystems do: > > #ifdef CONFIG_OF > extern int of_dma_controller_register(struct device_node *np, > struct dma_chan *(*of_dma_xlate) > (struct of_phandle_args *, struct of_dma *), > void *data); > extern void of_dma_controller_free(struct device_node *np); > extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, > char *name); > extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, > struct of_dma *ofdma); > #else > static inline int of_dma_controller_register(struct device_node *np, > struct dma_chan *(*of_dma_xlate) > (struct of_phandle_args *, struct of_dma *), > void *data) > { > return -ENODEV; > } > static inline void of_dma_controller_free(struct device_node *np) > { > } > static inline struct dma_chan *of_dma_request_slave_channel(struct device_node *np, > char *name) > { > return NULL; > } > static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, > struct of_dma *ofdma) > { > return NULL; > } > #endif > > I believe that Jon is on vacation this week, so if this is the only issue > holding up the merge, maybe you can change this in his patch directly, or > I can send an updated version if you prefer. I worry that too much is going on here too quickly. We have some people working on changing the way DMA engine selects channels. Meanwhile we have other people trying to create an OF DMA engine API. It seems that Vinod's working on a way for platforms to specify bindings to the DMA engine code, and the DMA engine code itself selects the appropriate channel. This patch, on the other hand, introduces a set of translation functions which need to be provided by platform code, which returns the dma_chan pointer. This sounds like a recipe for a total abortion of interfaces. Only one of those two activities should be going on at any one time, or if they have to occur, they need coordination so that the we don't end up with two totally different schemes. In the mad rush to DTify everything, don't make hasty decisions, because it is very difficult to change it later - especially something like this which defines how DT encodes this information.
On Mon, 2012-09-17 at 11:59 +0000, Arnd Bergmann wrote: > On Monday 17 September 2012, Vinod Koul wrote: > > I believe that Jon is on vacation this week, so if this is the only issue > holding up the merge, maybe you can change this in his patch directly, or > I can send an updated version if you prefer. Yes that was my idea as well, we do similar stuff in dmaengine. Okay I think we are good for merging this, I will add the patch for this as well. If anyone has objection please speak up _now_
On Mon, 2012-09-17 at 23:36 +0100, Russell King - ARM Linux wrote: > > > > I believe that Jon is on vacation this week, so if this is the only issue > > holding up the merge, maybe you can change this in his patch directly, or > > I can send an updated version if you prefer. > > I worry that too much is going on here too quickly. We have some people > working on changing the way DMA engine selects channels. Meanwhile we > have other people trying to create an OF DMA engine API. > > It seems that Vinod's working on a way for platforms to specify bindings > to the DMA engine code, and the DMA engine code itself selects the > appropriate channel. This patch, on the other hand, introduces a set of > translation functions which need to be provided by platform code, > which returns the dma_chan pointer. > > This sounds like a recipe for a total abortion of interfaces. Only one > of those two activities should be going on at any one time, or if they > have to occur, they need coordination so that the we don't end up with > two totally different schemes. > > In the mad rush to DTify everything, don't make hasty decisions, because > it is very difficult to change it later - especially something like this > which defines how DT encodes this information. We discussed this in KS and IMHO we need to merge these two approaches. For DT bindings, I think the binding itself shouldn't change based on my work but I would like these same bindings to help build the DMA engine code mappings. Now would it make sense to NOT merge these changes for 3.7 and postpone to 3.8. I can host these patches on a topic branch and merge them when we are ready. I plan to spend some good amount of time on my work this week so we should be ready pretty soon. One these changes are merged, users can start moving to this scheme.
On Tue, Sep 18, 2012 at 08:43:55AM +0530, Vinod Koul wrote: > On Mon, 2012-09-17 at 23:36 +0100, Russell King - ARM Linux wrote: > > > > > > I believe that Jon is on vacation this week, so if this is the only issue > > > holding up the merge, maybe you can change this in his patch directly, or > > > I can send an updated version if you prefer. > > > > I worry that too much is going on here too quickly. We have some people > > working on changing the way DMA engine selects channels. Meanwhile we > > have other people trying to create an OF DMA engine API. > > > > It seems that Vinod's working on a way for platforms to specify bindings > > to the DMA engine code, and the DMA engine code itself selects the > > appropriate channel. This patch, on the other hand, introduces a set of > > translation functions which need to be provided by platform code, > > which returns the dma_chan pointer. > > > > This sounds like a recipe for a total abortion of interfaces. Only one > > of those two activities should be going on at any one time, or if they > > have to occur, they need coordination so that the we don't end up with > > two totally different schemes. > > > > In the mad rush to DTify everything, don't make hasty decisions, because > > it is very difficult to change it later - especially something like this > > which defines how DT encodes this information. > We discussed this in KS and IMHO we need to merge these two approaches. > > For DT bindings, I think the binding itself shouldn't change based on my > work but I would like these same bindings to help build the DMA engine > code mappings. > > Now would it make sense to NOT merge these changes for 3.7 and postpone > to 3.8. I can host these patches on a topic branch and merge them when > we are ready. I plan to spend some good amount of time on my work this > week so we should be ready pretty soon. > One these changes are merged, users can start moving to this scheme. FWIW, I'm already basing the EDMA dmaengine support for OMAP (specifically for AM335x) on using these helpers since AM335x *only* boots from DT. -Matt
On Tuesday 18 September 2012, Matt Porter wrote: > On Tue, Sep 18, 2012 at 08:43:55AM +0530, Vinod Koul wrote: > > > > Now would it make sense to NOT merge these changes for 3.7 and postpone > > to 3.8. I can host these patches on a topic branch and merge them when > > we are ready. I plan to spend some good amount of time on my work this > > week so we should be ready pretty soon. > > One these changes are merged, users can start moving to this scheme. > > FWIW, I'm already basing the EDMA dmaengine support for OMAP (specifically > for AM335x) on using these helpers since AM335x only boots from DT. I suspect the same thing will be happening for a lot of platforms after 3.7-rc1: ux500, lpc32xx, integrator, imx, mxs and probably others are all waiting for this so they can complete the move to DT-only booting. Having some support for the new binding in 3.7 would be a great help to reduce interdependencies for the 3.8 merge cycle. Both the driver interface and the binding should actually be in a good shape now, so I'd very much appriciate it getting merged for 3.7, even if the implementation may have to change again in 3.8. Arnd
On Tue, Sep 18, 2012 at 03:20:08PM +0000, Arnd Bergmann wrote: > On Tuesday 18 September 2012, Matt Porter wrote: > > FWIW, I'm already basing the EDMA dmaengine support for OMAP (specifically > > for AM335x) on using these helpers since AM335x only boots from DT. > > I suspect the same thing will be happening for a lot of platforms > after 3.7-rc1: ux500, lpc32xx, integrator, imx, mxs and probably > others are all waiting for this so they can complete the move > to DT-only booting. Integrator won't be, because no integrator platforms have DMA support. That only came in with the Versatile range, of which it's extremely dodgy and unreliable to use. So in general, ARMs platforms do not have working DMA support. (I actually carry a private patch set which does enable full support on Versatile platforms, but given the general brokenness of the platform I see no reason to merge it and cause people issues with the platform when they find out that DMA is broken in weird and wonderful ways depending on the FPGA build they've happened to end up with...)
Hi Vinod, On 09/17/2012 10:13 PM, Vinod Koul wrote: > On Mon, 2012-09-17 at 23:36 +0100, Russell King - ARM Linux wrote: >>> >>> I believe that Jon is on vacation this week, so if this is the only issue >>> holding up the merge, maybe you can change this in his patch directly, or >>> I can send an updated version if you prefer. >> >> I worry that too much is going on here too quickly. We have some people >> working on changing the way DMA engine selects channels. Meanwhile we >> have other people trying to create an OF DMA engine API. >> >> It seems that Vinod's working on a way for platforms to specify bindings >> to the DMA engine code, and the DMA engine code itself selects the >> appropriate channel. This patch, on the other hand, introduces a set of >> translation functions which need to be provided by platform code, >> which returns the dma_chan pointer. >> >> This sounds like a recipe for a total abortion of interfaces. Only one >> of those two activities should be going on at any one time, or if they >> have to occur, they need coordination so that the we don't end up with >> two totally different schemes. >> >> In the mad rush to DTify everything, don't make hasty decisions, because >> it is very difficult to change it later - especially something like this >> which defines how DT encodes this information. > We discussed this in KS and IMHO we need to merge these two approaches. > > For DT bindings, I think the binding itself shouldn't change based on my > work but I would like these same bindings to help build the DMA engine > code mappings. > > Now would it make sense to NOT merge these changes for 3.7 and postpone > to 3.8. I can host these patches on a topic branch and merge them when > we are ready. I plan to spend some good amount of time on my work this > week so we should be ready pretty soon. > One these changes are merged, users can start moving to this scheme. I just wanted to see how things are progressing your side. Did you create a topic branch for this? If so let me know where I can find it, I did not find a branch on your infradead git tree. I wanted to pull in the latest patches for some testing. Cheers Jon
On Mon, 2012-09-24 at 17:25 -0500, Jon Hunter wrote: > > For DT bindings, I think the binding itself shouldn't change based on my > > work but I would like these same bindings to help build the DMA engine > > code mappings. > > > > Now would it make sense to NOT merge these changes for 3.7 and postpone > > to 3.8. I can host these patches on a topic branch and merge them when > > we are ready. I plan to spend some good amount of time on my work this > > week so we should be ready pretty soon. > > One these changes are merged, users can start moving to this scheme. > > I just wanted to see how things are progressing your side. Did you > create a topic branch for this? If so let me know where I can find it, I > did not find a branch on your infradead git tree. > > I wanted to pull in the latest patches for some testing. Sorry for delay, I had everything ready, but couldn't manage to commit and push. I have pushed to topic/dmaengine_dt, it is pushing out now... This has your two patches, my fix for build breakage, and Matt's typo patch. This is based on my next.
On Tue, 2012-10-16 at 10:43 +0800, Shawn Guo wrote: > On Tue, Sep 25, 2012 at 10:05:01AM +0530, Vinod Koul wrote: > > On Mon, 2012-09-24 at 17:25 -0500, Jon Hunter wrote: > > > > For DT bindings, I think the binding itself shouldn't change based on my > > > > work but I would like these same bindings to help build the DMA engine > > > > code mappings. > > > > > > > > Now would it make sense to NOT merge these changes for 3.7 and postpone > > > > to 3.8. I can host these patches on a topic branch and merge them when > > > > we are ready. I plan to spend some good amount of time on my work this > > > > week so we should be ready pretty soon. > > > > One these changes are merged, users can start moving to this scheme. > > > > > > I just wanted to see how things are progressing your side. Did you > > > create a topic branch for this? If so let me know where I can find it, I > > > did not find a branch on your infradead git tree. > > > > > > I wanted to pull in the latest patches for some testing. > > Sorry for delay, I had everything ready, but couldn't manage to commit > > and push. I have pushed to topic/dmaengine_dt, it is pushing out now... > > > Vinod, > > Looks it seemed 3.7-rc1. Does that mean we have to wait for 3.8, or > will it show on later -rc for 3.7? Yes this will be merged once we work out the common interface, otherwise we will end up redoing interfaces for all drivers.
On Tue, Sep 25, 2012 at 10:05:01AM +0530, Vinod Koul wrote: > On Mon, 2012-09-24 at 17:25 -0500, Jon Hunter wrote: > > > For DT bindings, I think the binding itself shouldn't change based on my > > > work but I would like these same bindings to help build the DMA engine > > > code mappings. > > > > > > Now would it make sense to NOT merge these changes for 3.7 and postpone > > > to 3.8. I can host these patches on a topic branch and merge them when > > > we are ready. I plan to spend some good amount of time on my work this > > > week so we should be ready pretty soon. > > > One these changes are merged, users can start moving to this scheme. > > > > I just wanted to see how things are progressing your side. Did you > > create a topic branch for this? If so let me know where I can find it, I > > did not find a branch on your infradead git tree. > > > > I wanted to pull in the latest patches for some testing. > Sorry for delay, I had everything ready, but couldn't manage to commit > and push. I have pushed to topic/dmaengine_dt, it is pushing out now... > Vinod, Looks it seemed 3.7-rc1. Does that mean we have to wait for 3.8, or will it show on later -rc for 3.7? Shawn
On 10/15/2012 09:39 PM, Vinod Koul wrote: > On Tue, 2012-10-16 at 10:43 +0800, Shawn Guo wrote: >> On Tue, Sep 25, 2012 at 10:05:01AM +0530, Vinod Koul wrote: >>> On Mon, 2012-09-24 at 17:25 -0500, Jon Hunter wrote: >>>>> For DT bindings, I think the binding itself shouldn't change based on my >>>>> work but I would like these same bindings to help build the DMA engine >>>>> code mappings. >>>>> >>>>> Now would it make sense to NOT merge these changes for 3.7 and postpone >>>>> to 3.8. I can host these patches on a topic branch and merge them when >>>>> we are ready. I plan to spend some good amount of time on my work this >>>>> week so we should be ready pretty soon. >>>>> One these changes are merged, users can start moving to this scheme. >>>> >>>> I just wanted to see how things are progressing your side. Did you >>>> create a topic branch for this? If so let me know where I can find it, I >>>> did not find a branch on your infradead git tree. >>>> >>>> I wanted to pull in the latest patches for some testing. >>> Sorry for delay, I had everything ready, but couldn't manage to commit >>> and push. I have pushed to topic/dmaengine_dt, it is pushing out now... >>> >> Vinod, >> >> Looks it seemed 3.7-rc1. Does that mean we have to wait for 3.8, or >> will it show on later -rc for 3.7? > Yes this will be merged once we work out the common interface, otherwise > we will end up redoing interfaces for all drivers. Hi Vinod, A few people have been asking me if getting device-tree support for DMA engine is plan for record for v3.8. I know that you were working through implementing a common interface and so I wanted to check how that is going. Do you have any insight to when device-tree support may get added? Cheers Jon
On Fri, 2012-11-09 at 14:01 -0600, Jon Hunter wrote: > Hi Vinod, > > A few people have been asking me if getting device-tree support for DMA > engine is plan for record for v3.8. I know that you were working through > implementing a common interface and so I wanted to check how that is > going. Do you have any insight to when device-tree support may get added? > I have not been able to do much work on this for last couple of weeks. I hope to do it in by this weekend. If not I will merge yours and then uppdate. Anyway, DT would be there in 3.8 with or without my changes.
On 11/16/2012 02:37 AM, Vinod Koul : > On Fri, 2012-11-09 at 14:01 -0600, Jon Hunter wrote: >> Hi Vinod, >> >> A few people have been asking me if getting device-tree support for DMA >> engine is plan for record for v3.8. I know that you were working through >> implementing a common interface and so I wanted to check how that is >> going. Do you have any insight to when device-tree support may get added? >> > I have not been able to do much work on this for last couple of weeks. I > hope to do it in by this weekend. If not I will merge yours and then > uppdate. > > Anyway, DT would be there in 3.8 with or without my changes. Vinod, So, can we imagine having this tree in linux-next rapidly (with or without your changes)? Thanks, best regards,
On 11/15/2012 07:37 PM, Vinod Koul wrote: > On Fri, 2012-11-09 at 14:01 -0600, Jon Hunter wrote: >> Hi Vinod, >> >> A few people have been asking me if getting device-tree support for DMA >> engine is plan for record for v3.8. I know that you were working through >> implementing a common interface and so I wanted to check how that is >> going. Do you have any insight to when device-tree support may get added? >> > I have not been able to do much work on this for last couple of weeks. I > hope to do it in by this weekend. If not I will merge yours and then > uppdate. > > Anyway, DT would be there in 3.8 with or without my changes. Thanks, Vinod. Can you make sure you also pick up the two fixes [1][2] I sent out? Cheers Jon [1] http://marc.info/?l=linux-omap&m=134859981920598&w=2 [2] http://marc.info/?l=linux-omap&m=134998461526129&w=2
On Fri, 2012-11-16 at 09:45 -0600, Jon Hunter wrote: > > Thanks, Vinod. Can you make sure you also pick up the two fixes [1][2] > I sent out? > > Cheers > Jon > > [1] http://marc.info/?l=linux-omap&m=134859981920598&w=2 > [2] http://marc.info/?l=linux-omap&m=134998461526129&w=2 > Applied both and merged the branch to my next. It should show up in linux-next tomorrow Please check Thanks
Hi Vinod, On 11/15/2012 07:37 PM, Vinod Koul wrote: > On Fri, 2012-11-09 at 14:01 -0600, Jon Hunter wrote: >> Hi Vinod, >> >> A few people have been asking me if getting device-tree support for DMA >> engine is plan for record for v3.8. I know that you were working through >> implementing a common interface and so I wanted to check how that is >> going. Do you have any insight to when device-tree support may get added? >> > I have not been able to do much work on this for last couple of weeks. I > hope to do it in by this weekend. If not I will merge yours and then > uppdate. > > Anyway, DT would be there in 3.8 with or without my changes. I am not sure if I have missed your pull request, but wanted to see if you had or were going to send a pull request for the DT changes for v3.8? I believe that the merge window ends this week. Cheers Jon
On Wed, 2012-12-19 at 11:12 -0600, Jon Hunter wrote: > I am not sure if I have missed your pull request, but wanted to see if > you had or were going to send a pull request for the DT changes for > v3.8? I believe that the merge window ends this week. Not yet sent. I was waiting on Dan's changes, just merged and pushed those. Pull request should show up in couple of days. Thanks
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 3491654..9b466da 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -62,6 +62,7 @@ #include <linux/rculist.h> #include <linux/idr.h> #include <linux/slab.h> +#include <linux/of_dma.h> static DEFINE_MUTEX(dma_list_mutex); static DEFINE_IDR(dma_idr); @@ -546,6 +547,21 @@ struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, v } EXPORT_SYMBOL_GPL(__dma_request_channel); +/** + * dma_request_slave_channel - try to allocate an exclusive slave channel + * @dev: pointer to client device structure + * @name: slave channel name + */ +struct dma_chan *dma_request_slave_channel(struct device *dev, char *name) +{ + /* If device-tree is present get slave info from here */ + if (dev->of_node) + return of_dma_request_slave_channel(dev->of_node, name); + + return NULL; +} +EXPORT_SYMBOL_GPL(dma_request_slave_channel); + void dma_release_channel(struct dma_chan *chan) { mutex_lock(&dma_list_mutex); diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 9c02a45..9500aa5 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -973,6 +973,7 @@ enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie); enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx); void dma_issue_pending_all(void); struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, dma_filter_fn fn, void *fn_param); +struct dma_chan *dma_request_slave_channel(struct device *dev, char *name); void dma_release_channel(struct dma_chan *chan); #else static inline enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx) @@ -987,6 +988,11 @@ static inline struct dma_chan *__dma_request_channel(dma_cap_mask_t *mask, { return NULL; } +static inline struct dma_chan *dma_request_slave_channel(struct device *dev, + char *name) +{ + return NULL +} static inline void dma_release_channel(struct dma_chan *chan) { }