Re: [PATCH v2] dmaengine: Create symlinks between DMA channels and slaves

[Peter Ujfalusi]

Hi Geert,

On 20/01/2020 12.51, Geert Uytterhoeven wrote:
> Hi Peter,
> 
> On Mon, Jan 20, 2020 at 11:16 AM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote:
>> On 20/01/2020 11.01, Geert Uytterhoeven wrote:
>>> On Fri, Jan 17, 2020 at 9:08 PM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote:
>>>> On 1/17/20 5:30 PM, Geert Uytterhoeven wrote:
>>>>> Currently it is not easy to find out which DMA channels are in use, and
>>>>> which slave devices are using which channels.
>>>>>
>>>>> Fix this by creating two symlinks between the DMA channel and the actual
>>>>> slave device when a channel is requested:
>>>>>   1. A "slave" symlink from DMA channel to slave device,
>>>>
>>>> Have you considered similar link name as on the slave device:
>>>> slave:<name>
>>>>
>>>> That way it would be easier to grasp which channel is used for what
>>>> purpose by only looking under /sys/class/dma/ and no need to check the
>>>> slave device.
>>>
>>> Would this really provide more information?
>>> The device name is already provided in the target of the symlink:
>>>
>>> root@koelsch:~# readlink
>>> /sys/devices/platform/soc/e6720000.dma-controller/dma/dma1chan2/slave
>>> ../../../ee140000.sd
>>
>> e6720000.dma-controller/dma/dma1chan2/slave -> ../../../ee140000.sd
>> e6720000.dma-controller/dma/dma1chan3/slave -> ../../../ee140000.sd
>>
>> It is hard to tell which one is the tx and RX channel without looking
>> under the ee140000.sd:
>>
>> ee140000.sd/dma:rx -> ../e6720000.dma-controller/dma/dma1chan3
>> ee140000.sd/dma:tx -> ../e6720000.dma-controller/dma/dma1chan2
> 
> Oh, you meant the name of the channel, not the name of the device.
> My mistake.
>
> As this name is a property of the slave device, not of the DMA channel,
> I don't think it belongs under dma*chan*.

Right, but it gives me only half the information I need to be a link useful.
I know that device X is using two channels but I need to check the
device X's directory to know which channel is used for what purpose.

>> Another option would be to not have symlinks, but a debugfs file where
>> this information can be extracted and would only compiled if debugfs is
>> enabled.
> 
> Like /proc/interrupts?

More like /sys/kernel/debug/gpio

> That brings the complexity of traversing all channels etc.

Sure, but only when the file is read.
You can add
#ifdef CONFIG_DEBUG_FS
#endif

around the slave_device and name in struct dma_chan {}

and when user reads the file you print out something like this:
cat /sys/kernel/debug/dmaengine

e6700000.dma-controller:
dma0chan0		e6e20000.spi:tx
dma0chan1		e6e20000.spi:rx
dma0chan2		ee100000.sd:tx
dma0chan3		ee100000.sd:rx
...
dma0chan14		non slave
...

e6720000.dma-controller:
dma1chan0		e6b10000.spi:tx
dma1chan1		e6b10000.spi:rx
...

This way we will have all the information in one place, easy to look up
and you don't need to manage symlinks dynamically, just check all
channels if they have slave_device/name when they are in_use (in_use w/o
slave_device is 'non slave')

Some drivers are requesting and releasing the DMA channel per transfer
or when they are opened/closed or other variations.

> What do other people think?
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

- PÃter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki