| Message ID | 1362599767-11292-2-git-send-email-mporter@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
[...] > * struct with auxiliary transfer status information, otherwise the call > * will just return a simple status code > * @device_issue_pending: push pending transactions to hardware > + * @device_slave_sg_limits: return the slave SG capabilities > */ > struct dma_device { > > @@ -602,6 +615,9 @@ struct dma_device { > dma_cookie_t cookie, > struct dma_tx_state *txstate); > void (*device_issue_pending)(struct dma_chan *chan); > + struct dma_slave_sg_limits *(*device_slave_sg_limits)( > + struct dma_chan *chan, enum dma_slave_buswidth addr_width, > + u32 maxburst); In my opinion it is better to pass in a pointer to a dma_slave_sg_limits struct and let the driver fill it. Instead of passing back a pointer to an internal structure. It is kind of problematic because you never really know when the user is done using the struct and you don't know when it is safe to free or reuse it. E.g. in your implementation for the edma driver if the function is called with different parameters for the same channel, the previous result will also be overwritten. - Lars
On Wed, Mar 06, 2013 at 02:56:05PM -0500, Matt Porter wrote: > Add a dmaengine API to retrieve slave SG transfer limits. > > The API is optionally implemented by dmaengine drivers and when > unimplemented will return a NULL pointer. A client driver using > this API provides the required dma channel, address width, and > burst size of the transfer. dma_get_slave_sg_limits() returns an > SG limits structure with the maximum number and size of SG segments > that the given channel can handle. > > Signed-off-by: Matt Porter <mporter@ti.com> > --- > +static inline struct dma_slave_sg_limits > +*dma_get_slave_sg_limits(struct dma_chan *chan, > + enum dma_slave_buswidth addr_width, > + u32 maxburst) > +{ > + if (chan->device->device_slave_sg_limits) > + return chan->device->device_slave_sg_limits(chan, > + addr_width, > + maxburst); Hi Matt, Sorry for delayed reply, this series was sent just before i went for vacation :) I agree with Lars, that returning pointer maynot be good idea. So this bit needs work. Also the readblity of above is bad by complying to 80char limit. I would make it easier to read -- ~Vinod > + > + return NULL; > +} > + > enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie); > #ifdef CONFIG_DMA_ENGINE > enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx); >
On 03/06/2013 08:56 PM, Matt Porter wrote: > Add a dmaengine API to retrieve slave SG transfer limits. > > The API is optionally implemented by dmaengine drivers and when > unimplemented will return a NULL pointer. A client driver using > this API provides the required dma channel, address width, and > burst size of the transfer. dma_get_slave_sg_limits() returns an > SG limits structure with the maximum number and size of SG segments > that the given channel can handle. Hi Matt, Are you still working on this patchset? Or do you mind if I pick it up, make the discussed changes and resubmit it? Thanks, - Lars
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 91ac8da..a4a6aac 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -371,6 +371,18 @@ struct dma_slave_config { unsigned int slave_id; }; +/* struct dma_slave_sg_limits - expose SG transfer limits of a channel + * + * @max_seg_nr: maximum number of SG segments supported on a SG/SLAVE + * channel (0 for no maximum or not a SG/SLAVE channel) + * @max_seg_len: maximum length of SG segments supported on a SG/SLAVE + * channel (0 for no maximum or not a SG/SLAVE channel) + */ +struct dma_slave_sg_limits { + u32 max_seg_nr; + u32 max_seg_len; +}; + static inline const char *dma_chan_name(struct dma_chan *chan) { return dev_name(&chan->dev->device); @@ -534,6 +546,7 @@ struct dma_tx_state { * struct with auxiliary transfer status information, otherwise the call * will just return a simple status code * @device_issue_pending: push pending transactions to hardware + * @device_slave_sg_limits: return the slave SG capabilities */ struct dma_device { @@ -602,6 +615,9 @@ struct dma_device { dma_cookie_t cookie, struct dma_tx_state *txstate); void (*device_issue_pending)(struct dma_chan *chan); + struct dma_slave_sg_limits *(*device_slave_sg_limits)( + struct dma_chan *chan, enum dma_slave_buswidth addr_width, + u32 maxburst); }; static inline int dmaengine_device_control(struct dma_chan *chan, @@ -963,6 +979,29 @@ dma_set_tx_state(struct dma_tx_state *st, dma_cookie_t last, dma_cookie_t used, } } +/** + * dma_get_slave_sg_limits - get DMAC SG transfer capabilities + * @chan: target DMA channel + * @addr_width: address width of the DMA transfer + * @maxburst: maximum DMA transfer burst size + * + * Get SG transfer capabilities for a specified channel. If the dmaengine + * driver does not implement SG transfer capabilities then NULL is + * returned. + */ +static inline struct dma_slave_sg_limits +*dma_get_slave_sg_limits(struct dma_chan *chan, + enum dma_slave_buswidth addr_width, + u32 maxburst) +{ + if (chan->device->device_slave_sg_limits) + return chan->device->device_slave_sg_limits(chan, + addr_width, + maxburst); + + return NULL; +} + enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie); #ifdef CONFIG_DMA_ENGINE enum dma_status dma_wait_for_async_tx(struct dma_async_tx_descriptor *tx);
Add a dmaengine API to retrieve slave SG transfer limits. The API is optionally implemented by dmaengine drivers and when unimplemented will return a NULL pointer. A client driver using this API provides the required dma channel, address width, and burst size of the transfer. dma_get_slave_sg_limits() returns an SG limits structure with the maximum number and size of SG segments that the given channel can handle. Signed-off-by: Matt Porter <mporter@ti.com> --- include/linux/dmaengine.h | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)