Re: [PATCH v7 23/25] PCI: dwc: Restore DMA-mask after MSI-data allocation

[Robin Murphy <robin.murphy@arm.com>]

On 2022-12-16 10:18, Serge Semin wrote:
> On Fri, Dec 16, 2022 at 01:49:38AM -0800, Christoph Hellwig wrote:
>> On Fri, Dec 16, 2022 at 12:34:23PM +0300, Serge Semin wrote:
>>> What about instead of save/restore pattern I'll just change the
>>> dma_set_mask_and_coherent() method with the dma_set_coherent_mask()
>>> function call? It seems cleaner. Like this:
>>
>>> Thus the platform-specific streaming DMA mask would be preserved.
>>> Since it's PCIe then having the streaming DMA-mask less than 32-bits
>>> wide is very much improbable. Moreover DW PCIe AXI-interface can be
>>> synthesize only with one out of two address bus widths: 32 and 64.
>>
> 
>> Where platform-specific means the dwc subdriver?
> 
> Right. I meant the streaming DMA-mask set by the low-level DWC PCIe drivers
> (like pcie-qcom(-ep)?.c, pcie-bt1.c, etc). It's very much important to
> have the real DMA-mask (at least the streaming one) set for the eDMA-capable
> controllers so the DMA-engine clients would work with the best performance.
> 
>> Yes, that seems to work.
> 
> Ok. I'll just use the direct dma_set_coherent_mask() method here then.
> 
>> Alternatively have a flag that says which streaming mask
>> to set.
> 
> I'd prefer to have more flexibility here relying on the low-level
> drivers to set the mask(s) instead of adding the new flag, just in case
> if there is vendor-specific IP-core/platform changes in the address
> bus width.

Presumably the low-level glue drivers could pass a bus size or mask 
value in struct dw_pcie_rp/dw_pcie, so the actual dma_set_mask() call 
itself could be centralised? I guess there's also an argument that only 
glue drivers which care about eDMA need to care about setting a mask at 
all, so I don't have a string preference either way. If you'd rather 
stick with that approach then it might be worth a brief comment at each 
site to clarify why the other mask is being set from an entirely 
different place, just in case anyone comes along and tries to "fix" it.

Cheers,
Robin.