On Mon, Sep 23, 2024 at 01:01:08AM +0300, Serge Semin wrote:
On Fri, Sep 20, 2024 at 06:56:17PM +0300, Andy Shevchenko wrote:
...
Fix the problem by specifying the default master ID for both memory and peripheral devices in the driver data. Thus the issue noticed for the iDMA 32-bit controllers will be eliminated and the ACPI-probed DW DMA controllers will be configured with the correct master ID by default.
v3: rewrote to use driver_data v2: https://lore.kernel.org/r/20240919185151.7331-1-fancer.lancer@gmail.com
IMO v2 looked better for me.
I disagree, obviously, since I sent a v3. (I can drop your authorship and tags in v4)
I am sure you know, but Master IDs is a platform-specific thing specific for each slave/peripheral device connected to the DMA controller. Depending on the chip design one peripheral device can be accessed over the one master IDs, another device/memory may have another master connected (can be up to four master IDs in general). That's why the master IDs have been declared in the dw_dma_slave structure.
Correct.
So adding them to struct dw_dma_chip_pdata doesn't seem like a good idea seeing it contains the generic DW DMA controller info.
So far there is no evidence that the channels are integrated differently on the same DMA controller over all hardware that uses this IP.
On the contrary my implementation seems a bit more coherent since it just changes the default slave IDs defined in the dw_dma_acpi_filter() method and initialized in the dw_dma_slave instance without adding slave-specific fields to the generic controller data.
The default enumeration for PCI + ACPI or pure ACPI devices is not changed with my patch, but actually makes it better (increases granularity). The defaults are platform specific and that's what driver_data is for.
While you like your solution, the problem with it that it doesn't cover different orders, so it's half-baked solution, I think. Mine doesn't change the status quo about integration (see above) and has better approach regarding different ordering. Both implementations have a flaw regarding per-channel master configuration.
What seems like a much better alternative to the both approaches, is to use the dw_dma_slave instance defined in the mrfld_spi_setup() method for the Intel Merrifield SPI PXA2xx DMA-interface in drivers/spi/spi-pxa2xx-pci.c. But AFAICT that data is left unused since the DMA-engine handle and connection parameters are determined by the channel name. Right? Is it possible to make use of the filter-function specified to the dma_request_slave_channel_compat() method?
Unfortunately no, in ACPI case the only data we use is the name (index) of the channel in the respective resources. Everything else is done automatically.