Messages in this thread | | | Date | Fri, 21 Apr 2023 15:58:18 +0100 | Subject | Re: [RFC v1 3/4] swiotlb: Allow dynamic allocation of bounce buffers | From | Robin Murphy <> |
| |
On 2023-04-21 14:03, Petr Tesařík wrote: > Hi Christoph! > > I'd like to follow up on this sub-thread: > > On Fri, 7 Apr 2023 12:15:55 +0200 > Petr Tesařík <petr@tesarici.cz> wroe: > >> On Fri, 7 Apr 2023 07:57:04 +0200 >> Christoph Hellwig <hch@lst.de> wrote: >> [...] >>> (Btw, in case anyone is interested, we really need to get started >>> on moving the dma fields out of struct device into a sub-struct >>> only allocated for DMA capable busses) >> >> I like this idea. In fact, my WIP topic branch now moves the swiotlb >> fields into a separate struct, > > As you have noticed, I have removed that commit again in v2. > > The reason is that I'm not sure about the intended goal. I have looked > around for examples of moving fields out of struct device and found > different approaches: > > A. struct dev_msi_info > The MSI fields are merely grouped in a separate struct, which is > defined in device.h and embedded in struct device. I don't see much > benefit. > > B. struct dev_pm_info > This struct is also embedded in struct device, but it is defined in > <linux/pm.h>, which is mentioned in MAINTAINERS. The benefit is that > further changes are reviewed by this maintainer. The downside is > that device.h includes pm.h. > > C. struct dev_pin_info > This struct is merely declared in device.h and defined > pinctrl/devinfo.h (which is not included). Only a pointer to this > struct is stored in struct device. Of course, the pointer must be > initialized (and released) somehow. > > Here my question: What did you want for DMA fields? > > A. Only grouping those fields in their own struct? > B. Or move the definition to another include file (cf. MAINTAINERS)? > C. Or store a pointer in struct device?
dev->dma_parms is already this, and IIRC still has some very old comments somewhere about consolidating the other DMA-related fields in there.
> Since you mentioned "allocated", it sounds like you want to achieve C, > but: > > 1. Is it worth the extra dereference for every use? > 2. How should the struct be allocated? Presumably not with kmalloc() in > device_initialize(), because I don't know how to determine if a > device is DMA capable this low in the call stack. So, should it be > allocated together with the containing structure? AFAICS this would > mean changing nearly all device drivers...
The bus code knows whether it's a DMA-capable bus or not, and as such should already be providing a .dma_configure method and/or performing some initialisation of DMA fields. Many of the ones that would need to are already providing dma_parms, in fact.
Thanks, Robin.
> > As you can see, I need some more guidance from you before I can start > working on this. ;-) > > Petr T
| |