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@xxxxxxxxxxx> wroe:
On Fri, 7 Apr 2023 07:57:04 +0200
Christoph Hellwig <hch@xxxxxx> 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
