Messages in this thread | | | From | Howard Yen <> | Date | Fri, 2 Feb 2024 12:40:59 +0800 | Subject | Re: [PATCH] dma-coherent: add support for multi coherent rmems per dev |
| |
On Thu, Feb 1, 2024 at 11:41 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Feb 01, 2024 at 10:45:30AM +0000, Robin Murphy wrote: > > On 2024-02-01 9:35 am, Howard Yen wrote: > > > Add support for multiple coherent rmems per device. This patch addes > > > dma_mem_list to device structure to store multiple rmems. > > > > > > These multiple rmems can be assigned to the device one by one by > > > of_reserved_mem_device_init_by_idx() with the memory-region > > > declaration in device tree as below and store the rmem to the dma_mem_list. > > > > > > device1@0 { > > > ... > > > memory-region = <&reserved_mem0>, <&reserved_mem1>; > > > ... > > > }; > > > > > > When driver tries to allocate memory from the rmems, looks for the first > > > available rmem and allocates the memory from this rmem. > > > > > > Then if driver removed, of_reserved_mem_device_release() needs to be > > > invoked to release all the rmems assigned to the device. > > > > > > Signed-off-by: Howard Yen <howardyen@google.com> > > > --- > > > include/linux/device.h | 1 + > > > kernel/dma/coherent.c | 66 +++++++++++++++++++++++++++++++++++------- > > > 2 files changed, 56 insertions(+), 11 deletions(-) > > > > > > diff --git a/include/linux/device.h b/include/linux/device.h > > > index 97c4b046c09d..c8682ee507cf 100644 > > > --- a/include/linux/device.h > > > +++ b/include/linux/device.h > > > @@ -751,6 +751,7 @@ struct device { > > > #ifdef CONFIG_DMA_DECLARE_COHERENT > > > struct dma_coherent_mem *dma_mem; /* internal for coherent mem > > > override */ > > > + struct list_head dma_mem_list; > > > > I'm not necessarily against the idea, but only if it's implemented sensibly. > > If we're going to have a list of these it should *replace* the existing > > pointer, not do this weird thing with both. > > Agreed, it should be one pointer max for this structure for this type of > thing. Why not move it into the dma_coherent_mem structure? > > thanks, > > greg k-h
I'm considering to modify the change to
1. Move it into the dma_coherent_mem structure, like
HEAD mem0->node
This case, if I check list_empty(mem0->node), it would give me the list is empty, but actually there is one rmem.
2. Replace the pointer to a list_head.
HEAD dma_mems ---> mem0->node
This case, if I check list_empty(dma_mems), it would give me the list is non-empty, it matches the actual status.
So, the 2nd looks reasonable, I'm going to upload a v2 patch with the 2nd approach, does that make sense?
-- Regards,
Howard
| |