| Message ID | 20241115-dlech-mainline-spi-engine-offload-2-v5-10-bea815bd5ea5@baylibre.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: axi-spi-engine: add offload support | expand |
On Fri, 15 Nov 2024 14:18:49 -0600 David Lechner <dlechner@baylibre.com> wrote: > Add a new devm_iio_dmaengine_buffer_setup_ext2() function to handle > cases where the DMA channel is managed by the caller rather than being > requested and released by the iio_dmaengine module. > > Signed-off-by: David Lechner <dlechner@baylibre.com> Fresh read and I'm wondering if the lifetimes in here can be managed more simply either by pushing the DMA channel get down, or dragging the release up. Basically I'd like to see them at the same level of nesting in the code. If it ends up being necessary to duplicate more code that is fine if it makes this easier to reason about. Jonathan > --- > > v5 changes: none > > v4 changes: > * This replaces "iio: buffer-dmaengine: generalize requesting DMA channel" > --- > drivers/iio/buffer/industrialio-buffer-dmaengine.c | 107 +++++++++++++++------ > include/linux/iio/buffer-dmaengine.h | 5 + > 2 files changed, 81 insertions(+), 31 deletions(-) > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > index 054af21dfa65..602cb2e147a6 100644 > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > @@ -33,6 +33,7 @@ struct dmaengine_buffer { > struct iio_dma_buffer_queue queue; > > struct dma_chan *chan; > + bool owns_chan; > struct list_head active; > > size_t align; > @@ -216,28 +217,23 @@ static const struct iio_dev_attr *iio_dmaengine_buffer_attrs[] = { > * Once done using the buffer iio_dmaengine_buffer_free() should be used to > * release it. > */ > -static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, > - const char *channel) > +static struct iio_buffer *iio_dmaengine_buffer_alloc(struct dma_chan *chan, > + bool owns_chan) > { > struct dmaengine_buffer *dmaengine_buffer; > unsigned int width, src_width, dest_width; > struct dma_slave_caps caps; > - struct dma_chan *chan; > int ret; > > dmaengine_buffer = kzalloc(sizeof(*dmaengine_buffer), GFP_KERNEL); > - if (!dmaengine_buffer) > - return ERR_PTR(-ENOMEM); > - > - chan = dma_request_chan(dev, channel); > - if (IS_ERR(chan)) { > - ret = PTR_ERR(chan); > - goto err_free; > + if (!dmaengine_buffer) { > + ret = -ENOMEM; > + goto err_release; > } > > ret = dma_get_slave_caps(chan, &caps); > if (ret < 0) > - goto err_release; > + goto err_free; > > /* Needs to be aligned to the maximum of the minimums */ > if (caps.src_addr_widths) > @@ -252,6 +248,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, > > INIT_LIST_HEAD(&dmaengine_buffer->active); > dmaengine_buffer->chan = chan; > + dmaengine_buffer->owns_chan = owns_chan; > dmaengine_buffer->align = width; > dmaengine_buffer->max_size = dma_get_max_seg_size(chan->device->dev); > > @@ -263,10 +260,12 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, > > return &dmaengine_buffer->queue.buffer; > > -err_release: > - dma_release_channel(chan); > err_free: > kfree(dmaengine_buffer); > +err_release: This ends up oddly miss balanced. If you get an error here, why not just do the release at the caller instead? > + if (owns_chan) > + dma_release_channel(chan); > + > return ERR_PTR(ret); > } > > @@ -282,12 +281,38 @@ void iio_dmaengine_buffer_free(struct iio_buffer *buffer) > iio_buffer_to_dmaengine_buffer(buffer); > > iio_dma_buffer_exit(&dmaengine_buffer->queue); > - dma_release_channel(dmaengine_buffer->chan); > - > iio_buffer_put(buffer); > + > + if (dmaengine_buffer->owns_chan) > + dma_release_channel(dmaengine_buffer->chan); > } > EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_free, IIO_DMAENGINE_BUFFER); > > +static struct iio_buffer > +*__iio_dmaengine_buffer_setup_ext(struct iio_dev *indio_dev, > + struct dma_chan *chan, bool owns_chan, > + enum iio_buffer_direction dir) > +{ > + struct iio_buffer *buffer; > + int ret; > + > + buffer = iio_dmaengine_buffer_alloc(chan, owns_chan); > + if (IS_ERR(buffer)) > + return ERR_CAST(buffer); > + > + indio_dev->modes |= INDIO_BUFFER_HARDWARE; > + > + buffer->direction = dir; > + > + ret = iio_device_attach_buffer(indio_dev, buffer); > + if (ret) { > + iio_dmaengine_buffer_free(buffer); Note that if you did push the free out to the caller for owns_chan as suggested above this would need to never free the buffer as that would be done if necessary at the caller for this... > + return ERR_PTR(ret); > + } > + > + return buffer; > +} > + > /** > * iio_dmaengine_buffer_setup_ext() - Setup a DMA buffer for an IIO device > * @dev: DMA channel consumer device > @@ -308,24 +333,13 @@ struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev, > const char *channel, > enum iio_buffer_direction dir) > { > - struct iio_buffer *buffer; > - int ret; > - > - buffer = iio_dmaengine_buffer_alloc(dev, channel); > - if (IS_ERR(buffer)) > - return ERR_CAST(buffer); > - > - indio_dev->modes |= INDIO_BUFFER_HARDWARE; > - > - buffer->direction = dir; > + struct dma_chan *chan; > > - ret = iio_device_attach_buffer(indio_dev, buffer); > - if (ret) { > - iio_dmaengine_buffer_free(buffer); > - return ERR_PTR(ret); > - } > + chan = dma_request_chan(dev, channel); > + if (IS_ERR(chan)) > + return ERR_CAST(chan); > > - return buffer; > + return __iio_dmaengine_buffer_setup_ext(indio_dev, chan, true, dir); As above, why not just free the channel here if this fails? Thus matching the dma_request_chan just above. > } > EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_setup_ext, IIO_DMAENGINE_BUFFER); > > @@ -362,6 +376,37 @@ int devm_iio_dmaengine_buffer_setup_ext(struct device *dev, > } > EXPORT_SYMBOL_NS_GPL(devm_iio_dmaengine_buffer_setup_ext, IIO_DMAENGINE_BUFFER); > > +/** > + * devm_iio_dmaengine_buffer_setup_ext2() - Setup a DMA buffer for an IIO device > + * @dev: Device for devm ownership > + * @indio_dev: IIO device to which to attach this buffer. > + * @chan: DMA channel > + * @dir: Direction of buffer (in or out) > + * > + * This allocates a new IIO buffer with devm_iio_dmaengine_buffer_alloc() > + * and attaches it to an IIO device with iio_device_attach_buffer(). > + * It also appends the INDIO_BUFFER_HARDWARE mode to the supported modes of the > + * IIO device. > + * > + * This is the same as devm_iio_dmaengine_buffer_setup_ext() except that the > + * caller manages requesting and releasing the DMA channel. I'd like the name to make that clearer. ext2 is too vague. > + */ > +int devm_iio_dmaengine_buffer_setup_ext2(struct device *dev, > + struct iio_dev *indio_dev, > + struct dma_chan *chan, > + enum iio_buffer_direction dir) > +{ > + struct iio_buffer *buffer; > + > + buffer = __iio_dmaengine_buffer_setup_ext(indio_dev, chan, false, dir); > + if (IS_ERR(buffer)) > + return PTR_ERR(buffer); > + > + return devm_add_action_or_reset(dev, __devm_iio_dmaengine_buffer_free, > + buffer); > +} > +EXPORT_SYMBOL_NS_GPL(devm_iio_dmaengine_buffer_setup_ext2, IIO_DMAENGINE_BUFFER); > + > MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); > MODULE_DESCRIPTION("DMA buffer for the IIO framework"); > MODULE_LICENSE("GPL"); > diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h > index 81d9a19aeb91..7bdb979b59f2 100644 > --- a/include/linux/iio/buffer-dmaengine.h > +++ b/include/linux/iio/buffer-dmaengine.h > @@ -11,6 +11,7 @@ > > struct iio_dev; > struct device; > +struct dma_chan; > > void iio_dmaengine_buffer_free(struct iio_buffer *buffer); > struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev, > @@ -26,6 +27,10 @@ int devm_iio_dmaengine_buffer_setup_ext(struct device *dev, > struct iio_dev *indio_dev, > const char *channel, > enum iio_buffer_direction dir); > +int devm_iio_dmaengine_buffer_setup_ext2(struct device *dev, > + struct iio_dev *indio_dev, > + struct dma_chan *chan, > + enum iio_buffer_direction dir); > > #define devm_iio_dmaengine_buffer_setup(dev, indio_dev, channel) \ > devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, \ >
On 11/24/24 11:16 AM, Jonathan Cameron wrote: > On Fri, 15 Nov 2024 14:18:49 -0600 > David Lechner <dlechner@baylibre.com> wrote: > >> Add a new devm_iio_dmaengine_buffer_setup_ext2() function to handle >> cases where the DMA channel is managed by the caller rather than being >> requested and released by the iio_dmaengine module. >> >> Signed-off-by: David Lechner <dlechner@baylibre.com> > Fresh read and I'm wondering if the lifetimes in here can be managed > more simply either by pushing the DMA channel get down, or dragging > the release up. Basically I'd like to see them at the same level > of nesting in the code. If it ends up being necessary to duplicate > more code that is fine if it makes this easier to reason about. > One option could be instead of introducing a 2nd function, change the existing iio_dmaengine_buffer_setup_ext() to use the signature of the proposed devm_iio_dmaengine_buffer_setup_ext2(). There are only two users of these functions. So we could move the dma chan request/release out to the drivers for those. Otherwise, we can't completly get rid of the owns_chan bit. > Jonathan > >> --- >> >> v5 changes: none >> >> v4 changes: >> * This replaces "iio: buffer-dmaengine: generalize requesting DMA channel" >> --- >> drivers/iio/buffer/industrialio-buffer-dmaengine.c | 107 +++++++++++++++------ >> include/linux/iio/buffer-dmaengine.h | 5 + >> 2 files changed, 81 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c >> index 054af21dfa65..602cb2e147a6 100644 >> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c >> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c >> @@ -33,6 +33,7 @@ struct dmaengine_buffer { >> struct iio_dma_buffer_queue queue; >> >> struct dma_chan *chan; >> + bool owns_chan; >> struct list_head active; >> >> size_t align; >> @@ -216,28 +217,23 @@ static const struct iio_dev_attr *iio_dmaengine_buffer_attrs[] = { >> * Once done using the buffer iio_dmaengine_buffer_free() should be used to >> * release it. >> */ >> -static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, >> - const char *channel) >> +static struct iio_buffer *iio_dmaengine_buffer_alloc(struct dma_chan *chan, >> + bool owns_chan) >> { >> struct dmaengine_buffer *dmaengine_buffer; >> unsigned int width, src_width, dest_width; >> struct dma_slave_caps caps; >> - struct dma_chan *chan; >> int ret; >> >> dmaengine_buffer = kzalloc(sizeof(*dmaengine_buffer), GFP_KERNEL); >> - if (!dmaengine_buffer) >> - return ERR_PTR(-ENOMEM); >> - >> - chan = dma_request_chan(dev, channel); >> - if (IS_ERR(chan)) { >> - ret = PTR_ERR(chan); >> - goto err_free; >> + if (!dmaengine_buffer) { >> + ret = -ENOMEM; >> + goto err_release; >> } >> >> ret = dma_get_slave_caps(chan, &caps); >> if (ret < 0) >> - goto err_release; >> + goto err_free; >> >> /* Needs to be aligned to the maximum of the minimums */ >> if (caps.src_addr_widths) >> @@ -252,6 +248,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, >> >> INIT_LIST_HEAD(&dmaengine_buffer->active); >> dmaengine_buffer->chan = chan; >> + dmaengine_buffer->owns_chan = owns_chan; >> dmaengine_buffer->align = width; >> dmaengine_buffer->max_size = dma_get_max_seg_size(chan->device->dev); >> >> @@ -263,10 +260,12 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, >> >> return &dmaengine_buffer->queue.buffer; >> >> -err_release: >> - dma_release_channel(chan); >> err_free: >> kfree(dmaengine_buffer); >> +err_release: > This ends up oddly miss balanced. If you get an error here, why > not just do the release at the caller instead? Yes, we can do that. > >> + if (owns_chan) >> + dma_release_channel(chan); >> + >> return ERR_PTR(ret); >> } >> >> @@ -282,12 +281,38 @@ void iio_dmaengine_buffer_free(struct iio_buffer *buffer) >> iio_buffer_to_dmaengine_buffer(buffer); >> >> iio_dma_buffer_exit(&dmaengine_buffer->queue); >> - dma_release_channel(dmaengine_buffer->chan); >> - >> iio_buffer_put(buffer); >> + >> + if (dmaengine_buffer->owns_chan) >> + dma_release_channel(dmaengine_buffer->chan); >> } >> EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_free, IIO_DMAENGINE_BUFFER); >> >> +static struct iio_buffer >> +*__iio_dmaengine_buffer_setup_ext(struct iio_dev *indio_dev, >> + struct dma_chan *chan, bool owns_chan, >> + enum iio_buffer_direction dir) >> +{ >> + struct iio_buffer *buffer; >> + int ret; >> + >> + buffer = iio_dmaengine_buffer_alloc(chan, owns_chan); >> + if (IS_ERR(buffer)) >> + return ERR_CAST(buffer); >> + >> + indio_dev->modes |= INDIO_BUFFER_HARDWARE; >> + >> + buffer->direction = dir; >> + >> + ret = iio_device_attach_buffer(indio_dev, buffer); >> + if (ret) { >> + iio_dmaengine_buffer_free(buffer); > Note that if you did push the free out to the caller for owns_chan > as suggested above this would need to never free the buffer as > that would be done if necessary at the caller for this... We can't push iio_dmaengine_buffer_free() out here since it is balancing the call to iio_dmaengine_buffer_alloc() which is called just above in this function. But we do have a problem then with the caller not knowing if the channel was released or not on error. So perhaps we get rid of this helper function and duplicate some code as you suggested. > >> + return ERR_PTR(ret); >> + } >> + >> + return buffer; >> +} >> + >> /** >> * iio_dmaengine_buffer_setup_ext() - Setup a DMA buffer for an IIO device >> * @dev: DMA channel consumer device >> @@ -308,24 +333,13 @@ struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev, >> const char *channel, >> enum iio_buffer_direction dir) >> { >> - struct iio_buffer *buffer; >> - int ret; >> - >> - buffer = iio_dmaengine_buffer_alloc(dev, channel); >> - if (IS_ERR(buffer)) >> - return ERR_CAST(buffer); >> - >> - indio_dev->modes |= INDIO_BUFFER_HARDWARE; >> - >> - buffer->direction = dir; >> + struct dma_chan *chan; >> >> - ret = iio_device_attach_buffer(indio_dev, buffer); >> - if (ret) { >> - iio_dmaengine_buffer_free(buffer); >> - return ERR_PTR(ret); >> - } >> + chan = dma_request_chan(dev, channel); >> + if (IS_ERR(chan)) >> + return ERR_CAST(chan); >> >> - return buffer; >> + return __iio_dmaengine_buffer_setup_ext(indio_dev, chan, true, dir); > As above, why not just free the channel here if this fails? Thus matching > the dma_request_chan just above. This one is manageable. > > >> } >> EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_setup_ext, IIO_DMAENGINE_BUFFER); >>
On 12/6/24 3:36 PM, David Lechner wrote: > On 11/24/24 11:16 AM, Jonathan Cameron wrote: >> On Fri, 15 Nov 2024 14:18:49 -0600 >> David Lechner <dlechner@baylibre.com> wrote: >> >>> Add a new devm_iio_dmaengine_buffer_setup_ext2() function to handle >>> cases where the DMA channel is managed by the caller rather than being >>> requested and released by the iio_dmaengine module. >>> >>> Signed-off-by: David Lechner <dlechner@baylibre.com> >> Fresh read and I'm wondering if the lifetimes in here can be managed >> more simply either by pushing the DMA channel get down, or dragging >> the release up. Basically I'd like to see them at the same level >> of nesting in the code. If it ends up being necessary to duplicate >> more code that is fine if it makes this easier to reason about. >> > > One option could be instead of introducing a 2nd function, change Oops. The new function is devm_ so would still need a 2nd function but changing iio_dmaengine_buffer_setup_ext() to have basically the same signature would still avoid the asymmetry. > the existing iio_dmaengine_buffer_setup_ext() to use the signature > of the proposed devm_iio_dmaengine_buffer_setup_ext2(). There are > only two users of these functions. So we could move the dma chan > request/release out to the drivers for those. > > Otherwise, we can't completely get rid of the owns_chan bit. >
On Fri, 6 Dec 2024 16:04:40 -0600 David Lechner <dlechner@baylibre.com> wrote: > On 12/6/24 3:36 PM, David Lechner wrote: > > On 11/24/24 11:16 AM, Jonathan Cameron wrote: > >> On Fri, 15 Nov 2024 14:18:49 -0600 > >> David Lechner <dlechner@baylibre.com> wrote: > >> > >>> Add a new devm_iio_dmaengine_buffer_setup_ext2() function to handle > >>> cases where the DMA channel is managed by the caller rather than being > >>> requested and released by the iio_dmaengine module. > >>> > >>> Signed-off-by: David Lechner <dlechner@baylibre.com> > >> Fresh read and I'm wondering if the lifetimes in here can be managed > >> more simply either by pushing the DMA channel get down, or dragging > >> the release up. Basically I'd like to see them at the same level > >> of nesting in the code. If it ends up being necessary to duplicate > >> more code that is fine if it makes this easier to reason about. > >> > > > > One option could be instead of introducing a 2nd function, change > > Oops. The new function is devm_ so would still need a 2nd function > but changing iio_dmaengine_buffer_setup_ext() to have basically > the same signature would still avoid the asymmetry. That sounds sensible. (though I've mostly forgotten the background ;) > > > the existing iio_dmaengine_buffer_setup_ext() to use the signature > > of the proposed devm_iio_dmaengine_buffer_setup_ext2(). There are > > only two users of these functions. So we could move the dma chan > > request/release out to the drivers for those. > > > > Otherwise, we can't completely get rid of the owns_chan bit. > >
diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c index 054af21dfa65..602cb2e147a6 100644 --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c @@ -33,6 +33,7 @@ struct dmaengine_buffer { struct iio_dma_buffer_queue queue; struct dma_chan *chan; + bool owns_chan; struct list_head active; size_t align; @@ -216,28 +217,23 @@ static const struct iio_dev_attr *iio_dmaengine_buffer_attrs[] = { * Once done using the buffer iio_dmaengine_buffer_free() should be used to * release it. */ -static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, - const char *channel) +static struct iio_buffer *iio_dmaengine_buffer_alloc(struct dma_chan *chan, + bool owns_chan) { struct dmaengine_buffer *dmaengine_buffer; unsigned int width, src_width, dest_width; struct dma_slave_caps caps; - struct dma_chan *chan; int ret; dmaengine_buffer = kzalloc(sizeof(*dmaengine_buffer), GFP_KERNEL); - if (!dmaengine_buffer) - return ERR_PTR(-ENOMEM); - - chan = dma_request_chan(dev, channel); - if (IS_ERR(chan)) { - ret = PTR_ERR(chan); - goto err_free; + if (!dmaengine_buffer) { + ret = -ENOMEM; + goto err_release; } ret = dma_get_slave_caps(chan, &caps); if (ret < 0) - goto err_release; + goto err_free; /* Needs to be aligned to the maximum of the minimums */ if (caps.src_addr_widths) @@ -252,6 +248,7 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, INIT_LIST_HEAD(&dmaengine_buffer->active); dmaengine_buffer->chan = chan; + dmaengine_buffer->owns_chan = owns_chan; dmaengine_buffer->align = width; dmaengine_buffer->max_size = dma_get_max_seg_size(chan->device->dev); @@ -263,10 +260,12 @@ static struct iio_buffer *iio_dmaengine_buffer_alloc(struct device *dev, return &dmaengine_buffer->queue.buffer; -err_release: - dma_release_channel(chan); err_free: kfree(dmaengine_buffer); +err_release: + if (owns_chan) + dma_release_channel(chan); + return ERR_PTR(ret); } @@ -282,12 +281,38 @@ void iio_dmaengine_buffer_free(struct iio_buffer *buffer) iio_buffer_to_dmaengine_buffer(buffer); iio_dma_buffer_exit(&dmaengine_buffer->queue); - dma_release_channel(dmaengine_buffer->chan); - iio_buffer_put(buffer); + + if (dmaengine_buffer->owns_chan) + dma_release_channel(dmaengine_buffer->chan); } EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_free, IIO_DMAENGINE_BUFFER); +static struct iio_buffer +*__iio_dmaengine_buffer_setup_ext(struct iio_dev *indio_dev, + struct dma_chan *chan, bool owns_chan, + enum iio_buffer_direction dir) +{ + struct iio_buffer *buffer; + int ret; + + buffer = iio_dmaengine_buffer_alloc(chan, owns_chan); + if (IS_ERR(buffer)) + return ERR_CAST(buffer); + + indio_dev->modes |= INDIO_BUFFER_HARDWARE; + + buffer->direction = dir; + + ret = iio_device_attach_buffer(indio_dev, buffer); + if (ret) { + iio_dmaengine_buffer_free(buffer); + return ERR_PTR(ret); + } + + return buffer; +} + /** * iio_dmaengine_buffer_setup_ext() - Setup a DMA buffer for an IIO device * @dev: DMA channel consumer device @@ -308,24 +333,13 @@ struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev, const char *channel, enum iio_buffer_direction dir) { - struct iio_buffer *buffer; - int ret; - - buffer = iio_dmaengine_buffer_alloc(dev, channel); - if (IS_ERR(buffer)) - return ERR_CAST(buffer); - - indio_dev->modes |= INDIO_BUFFER_HARDWARE; - - buffer->direction = dir; + struct dma_chan *chan; - ret = iio_device_attach_buffer(indio_dev, buffer); - if (ret) { - iio_dmaengine_buffer_free(buffer); - return ERR_PTR(ret); - } + chan = dma_request_chan(dev, channel); + if (IS_ERR(chan)) + return ERR_CAST(chan); - return buffer; + return __iio_dmaengine_buffer_setup_ext(indio_dev, chan, true, dir); } EXPORT_SYMBOL_NS_GPL(iio_dmaengine_buffer_setup_ext, IIO_DMAENGINE_BUFFER); @@ -362,6 +376,37 @@ int devm_iio_dmaengine_buffer_setup_ext(struct device *dev, } EXPORT_SYMBOL_NS_GPL(devm_iio_dmaengine_buffer_setup_ext, IIO_DMAENGINE_BUFFER); +/** + * devm_iio_dmaengine_buffer_setup_ext2() - Setup a DMA buffer for an IIO device + * @dev: Device for devm ownership + * @indio_dev: IIO device to which to attach this buffer. + * @chan: DMA channel + * @dir: Direction of buffer (in or out) + * + * This allocates a new IIO buffer with devm_iio_dmaengine_buffer_alloc() + * and attaches it to an IIO device with iio_device_attach_buffer(). + * It also appends the INDIO_BUFFER_HARDWARE mode to the supported modes of the + * IIO device. + * + * This is the same as devm_iio_dmaengine_buffer_setup_ext() except that the + * caller manages requesting and releasing the DMA channel. + */ +int devm_iio_dmaengine_buffer_setup_ext2(struct device *dev, + struct iio_dev *indio_dev, + struct dma_chan *chan, + enum iio_buffer_direction dir) +{ + struct iio_buffer *buffer; + + buffer = __iio_dmaengine_buffer_setup_ext(indio_dev, chan, false, dir); + if (IS_ERR(buffer)) + return PTR_ERR(buffer); + + return devm_add_action_or_reset(dev, __devm_iio_dmaengine_buffer_free, + buffer); +} +EXPORT_SYMBOL_NS_GPL(devm_iio_dmaengine_buffer_setup_ext2, IIO_DMAENGINE_BUFFER); + MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>"); MODULE_DESCRIPTION("DMA buffer for the IIO framework"); MODULE_LICENSE("GPL"); diff --git a/include/linux/iio/buffer-dmaengine.h b/include/linux/iio/buffer-dmaengine.h index 81d9a19aeb91..7bdb979b59f2 100644 --- a/include/linux/iio/buffer-dmaengine.h +++ b/include/linux/iio/buffer-dmaengine.h @@ -11,6 +11,7 @@ struct iio_dev; struct device; +struct dma_chan; void iio_dmaengine_buffer_free(struct iio_buffer *buffer); struct iio_buffer *iio_dmaengine_buffer_setup_ext(struct device *dev, @@ -26,6 +27,10 @@ int devm_iio_dmaengine_buffer_setup_ext(struct device *dev, struct iio_dev *indio_dev, const char *channel, enum iio_buffer_direction dir); +int devm_iio_dmaengine_buffer_setup_ext2(struct device *dev, + struct iio_dev *indio_dev, + struct dma_chan *chan, + enum iio_buffer_direction dir); #define devm_iio_dmaengine_buffer_setup(dev, indio_dev, channel) \ devm_iio_dmaengine_buffer_setup_ext(dev, indio_dev, channel, \
Add a new devm_iio_dmaengine_buffer_setup_ext2() function to handle cases where the DMA channel is managed by the caller rather than being requested and released by the iio_dmaengine module. Signed-off-by: David Lechner <dlechner@baylibre.com> --- v5 changes: none v4 changes: * This replaces "iio: buffer-dmaengine: generalize requesting DMA channel" --- drivers/iio/buffer/industrialio-buffer-dmaengine.c | 107 +++++++++++++++------ include/linux/iio/buffer-dmaengine.h | 5 + 2 files changed, 81 insertions(+), 31 deletions(-)