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@xxxxxxxxx > > 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. -- With Best Regards, Andy Shevchenko
