On Tue, Mar 10, 2026 at 3:38 PM Andrew Davis <[email protected]> wrote:
>
> On 3/6/26 4:36 AM, Albert Esteve wrote:
> > From: John Stultz <[email protected]>
> >
> > Keep track of the heap device struct.
> >
> > This will be useful for special DMA allocations
> > and actions.
> >
> > Signed-off-by: John Stultz <[email protected]>
> > Reviewed-by: Maxime Ripard <[email protected]>
> > Signed-off-by: Albert Esteve <[email protected]>
> > ---
> > drivers/dma-buf/dma-heap.c | 34 ++++++++++++++++++++++++++--------
> > include/linux/dma-heap.h | 2 ++
> > 2 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> > index ac5f8685a6494..1124d63eb1398 100644
> > --- a/drivers/dma-buf/dma-heap.c
> > +++ b/drivers/dma-buf/dma-heap.c
> > @@ -31,6 +31,7 @@
> > * @heap_devt: heap device node
> > * @list: list head connecting to list of heaps
> > * @heap_cdev: heap char device
> > + * @heap_dev: heap device
> > *
> > * Represents a heap of memory from which buffers can be made.
> > */
> > @@ -41,6 +42,7 @@ struct dma_heap {
> > dev_t heap_devt;
> > struct list_head list;
> > struct cdev heap_cdev;
> > + struct device *heap_dev;
> > };
> >
> > static LIST_HEAD(heap_list);
> > @@ -223,6 +225,19 @@ const char *dma_heap_get_name(struct dma_heap *heap)
> > }
> > EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
> >
> > +/**
> > + * dma_heap_get_dev() - get device struct for the heap
> > + * @heap: DMA-Heap to retrieve device struct from
> > + *
> > + * Returns:
> > + * The device struct for the heap.
> > + */
> > +struct device *dma_heap_get_dev(struct dma_heap *heap)
> > +{
> > + return heap->heap_dev;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(dma_heap_get_dev, "DMA_BUF_HEAP");
> > +
> > /**
> > * dma_heap_add - adds a heap to dmabuf heaps
> > * @exp_info: information needed to register this heap
> > @@ -230,7 +245,6 @@ EXPORT_SYMBOL_NS_GPL(dma_heap_get_name, "DMA_BUF_HEAP");
> > struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> > {
> > struct dma_heap *heap, *h, *err_ret;
> > - struct device *dev_ret;
> > unsigned int minor;
> > int ret;
> >
> > @@ -272,14 +286,14 @@ struct dma_heap *dma_heap_add(const struct
> > dma_heap_export_info *exp_info)
> > goto err1;
> > }
> >
> > - dev_ret = device_create(dma_heap_class,
> > - NULL,
> > - heap->heap_devt,
> > - NULL,
> > - heap->name);
> > - if (IS_ERR(dev_ret)) {
> > + heap->heap_dev = device_create(dma_heap_class,
> > + NULL,
> > + heap->heap_devt,
> > + NULL,
> > + heap->name);
> > + if (IS_ERR(heap->heap_dev)) {
> > pr_err("dma_heap: Unable to create device\n");
> > - err_ret = ERR_CAST(dev_ret);
> > + err_ret = ERR_CAST(heap->heap_dev);
> > goto err2;
> > }
> >
> > @@ -295,6 +309,10 @@ struct dma_heap *dma_heap_add(const struct
> > dma_heap_export_info *exp_info)
> > }
> > }
> >
> > + /* Make sure it doesn't disappear on us */
> > + heap->heap_dev = get_device(heap->heap_dev);
> > +
> > +
>
> Is this actually something that matters or could happen? Seems you
> just remove it in the next patch anyway.
Good question. Technically, device_add() (and therefore, also
device_create()) already increments the refcount by calling
get_device() internally. So I'm not sure if this is necessary, I just
carried it over when I picked the patch. It feels like a safeguard to
have the device owner hold an extra reference so that if other code
decreases the reference count, the heap device won't be destroyed. So,
having the extra reference causes no harm.
I dropped it in the next patch because otherwise, I would have to
account for (and drop) both references when implementing
dma_heap_destroy().
So, I'm not sure what the best pattern is here. But I do agree that I
should either remove it from both patches or keep it for both.
BR,
Albert.
>
> Andrew
>
> > /* Add heap to the list */
> > list_add(&heap->list, &heap_list);
> > mutex_unlock(&heap_list_lock);
> > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> > index 648328a64b27e..493085e69b70e 100644
> > --- a/include/linux/dma-heap.h
> > +++ b/include/linux/dma-heap.h
> > @@ -12,6 +12,7 @@
> > #include <linux/types.h>
> >
> > struct dma_heap;
> > +struct device;
> >
> > /**
> > * struct dma_heap_ops - ops to operate on a given heap
> > @@ -43,6 +44,7 @@ struct dma_heap_export_info {
> > void *dma_heap_get_drvdata(struct dma_heap *heap);
> >
> > const char *dma_heap_get_name(struct dma_heap *heap);
> > +struct device *dma_heap_get_dev(struct dma_heap *heap);
> >
> > struct dma_heap *dma_heap_add(const struct dma_heap_export_info
> > *exp_info);
> >
> >
>
