On Fri, 2020-02-21 at 12:21 +0000, Jonathan Cameron wrote: > [External] > > On Thu, 20 Feb 2020 17:03:13 +0200 > Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote: > > > Currently, when using a 'iio_dmaengine_buffer_alloc()', an matching call to > > 'iio_dmaengine_buffer_free()' must be made. > > > > With this change, this can be avoided by using > > 'devm_iio_dmaengine_buffer_alloc()'. The buffer will get free'd via the > > device's devres handling. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > --- > > .../buffer/industrialio-buffer-dmaengine.c | 70 +++++++++++++++++++ > > include/linux/iio/buffer-dmaengine.h | 5 ++ > > 2 files changed, 75 insertions(+) > > > > diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c > > b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > > index b129693af0fd..eff89037e3f5 100644 > > --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c > > +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c > > @@ -229,6 +229,76 @@ void iio_dmaengine_buffer_free(struct iio_buffer > > *buffer) > > } > > EXPORT_SYMBOL_GPL(iio_dmaengine_buffer_free); > > > > +static void __devm_iio_dmaengine_buffer_free(struct device *dev, void *res) > > +{ > > + iio_dmaengine_buffer_free(*(struct iio_buffer **)res); > > +} > > + > > +/** > > + * devm_iio_dmaengine_buffer_alloc() - Resource-managed > > iio_dmaengine_buffer_alloc() > > + * @dev: Parent device for the buffer > > + * @channel: DMA channel name, typically "rx". > > + * > > + * This allocates a new IIO buffer which internally uses the DMAengine > > framework > > + * to perform its transfers. The parent device will be used to request the > > DMA > > + * channel. > > + * > > + * Once done using the buffer iio_dmaengine_buffer_free() should be used to > > + * release it. > > Umm. It really shouldn't! Yeah. Copy+paste. My bad. > > + */ > > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev, > > + const char *channel) > > +{ > > + struct iio_buffer **bufferp, *buffer; > > + > > + bufferp = devres_alloc(__devm_iio_dmaengine_buffer_free, > > + sizeof(*bufferp), GFP_KERNEL); > > + if (!bufferp) > > + return ERR_PTR(-ENOMEM); > > + > > + buffer = iio_dmaengine_buffer_alloc(dev, channel); > > + if (!IS_ERR(buffer)) { > > + *bufferp = buffer; > > + devres_add(dev, bufferp); > > From a flow point of view I'd prefer. > > if (IS_ERR(buffer) { > devres_free(buferp) > return buffer; > } > > *bufferp = buffer; > devres_add(dev, bufferp); > > return buffer; > > > > + } else { > > + devres_free(bufferp); > > + } > > + > > + return buffer; > > +} > > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_alloc); > > + > > +static int devm_iio_dmaengine_buffer_match(struct device *dev, void *res, > > + void *data) > > +{ > > + struct iio_buffer **r = res; > > + > > + if (!r || !*r) { > > + WARN_ON(!r || !*r); > > + return 0; > > + } > > + > > + return *r == data; > > +} > > + > > +/** > > + * devm_iio_dmaengine_buffer_free - iio_dmaengine_buffer_free > > + * @dev: Device this iio_buffer belongs to > > + * @buffer: The iio_buffer associated with the device > > + * > > + * Free buffer allocated with devm_iio_dmaengine_buffer_alloc(). > > + */ > > +void devm_iio_dmaengine_buffer_free(struct device *dev, > > + struct iio_buffer *buffer) > > +{ > > + int rc; > > + > > + rc = devres_release(dev, __devm_iio_dmaengine_buffer_free, > > + devm_iio_dmaengine_buffer_match, buffer); > > + WARN_ON(rc); > > +} > > +EXPORT_SYMBOL_GPL(devm_iio_dmaengine_buffer_free); Should I remove devm_iio_dmaengine_buffer_free() ? There was a comment on the AXI ADC that may apply here as well, about maybe removing it. > > + > > MODULE_AUTHOR("Lars-Peter Clausen <lars@xxxxxxxxxx>"); > > 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 b3a57444a886..8dcd973d76c1 100644 > > --- a/include/linux/iio/buffer-dmaengine.h > > +++ b/include/linux/iio/buffer-dmaengine.h > > @@ -14,4 +14,9 @@ struct iio_buffer *iio_dmaengine_buffer_alloc(struct > > device *dev, > > const char *channel); > > void iio_dmaengine_buffer_free(struct iio_buffer *buffer); > > > > +struct iio_buffer *devm_iio_dmaengine_buffer_alloc(struct device *dev, > > + const char *channel); > > +void devm_iio_dmaengine_buffer_free(struct device *dev, > > + struct iio_buffer *buffer); > Please align parameters with opening bracket where possible. > ack > Thanks, > > Jonathan > > > + > > #endif