All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Geert Uytterhoeven <geert+renesas@glider.be>,
	Vinod Koul <vkoul@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>
Cc: <dmaengine@vger.kernel.org>, <linux-renesas-soc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] dmaengine: Create symlinks between DMA channels and slaves
Date: Thu, 30 Jan 2020 11:43:21 +0200	[thread overview]
Message-ID: <e219bc58-0d30-582f-4872-559097f212d2@ti.com> (raw)
In-Reply-To: <20200117153056.31363-1-geert+renesas@glider.be>

Geert,

On 17/01/2020 17.30, 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,
>   2. A "dma:<name>" symlink slave device to DMA channel.
> When the channel is released, the symlinks are removed again.
> The latter requires keeping track of the slave device and the channel
> name in the dma_chan structure.
> 
> Note that this is limited to channel request functions for requesting an
> exclusive slave channel that take a device pointer (dma_request_chan()
> and dma_request_slave_channel*()).
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - Add DMA_SLAVE_NAME macro,
>   - Also handle channels from FIXME,
>   - Add backlinks from slave device to DMA channel,
> 
> On r8a7791/koelsch, the following new symlinks are created:
> 
>     /sys/devices/platform/soc/
>     ├── e6700000.dma-controller/dma/dma0chan0/slave -> ../../../e6e20000.spi
>     ├── e6700000.dma-controller/dma/dma0chan1/slave -> ../../../e6e20000.spi
>     ├── e6700000.dma-controller/dma/dma0chan2/slave -> ../../../ee100000.sd
>     ├── e6700000.dma-controller/dma/dma0chan3/slave -> ../../../ee100000.sd
>     ├── e6700000.dma-controller/dma/dma0chan4/slave -> ../../../ee160000.sd
>     ├── e6700000.dma-controller/dma/dma0chan5/slave -> ../../../ee160000.sd
>     ├── e6700000.dma-controller/dma/dma0chan6/slave -> ../../../e6e68000.serial
>     ├── e6700000.dma-controller/dma/dma0chan7/slave -> ../../../e6e68000.serial
>     ├── e6720000.dma-controller/dma/dma1chan0/slave -> ../../../e6b10000.spi
>     ├── e6720000.dma-controller/dma/dma1chan1/slave -> ../../../e6b10000.spi
>     ├── e6720000.dma-controller/dma/dma1chan2/slave -> ../../../ee140000.sd
>     ├── e6720000.dma-controller/dma/dma1chan3/slave -> ../../../ee140000.sd
>     ├── e6b10000.spi/dma:rx -> ../e6720000.dma-controller/dma/dma1chan1
>     ├── e6b10000.spi/dma:tx -> ../e6720000.dma-controller/dma/dma1chan0
>     ├── e6e20000.spi/dma:rx -> ../e6700000.dma-controller/dma/dma0chan1
>     ├── e6e20000.spi/dma:tx -> ../e6700000.dma-controller/dma/dma0chan0
>     ├── e6e68000.serial/dma:rx -> ../e6700000.dma-controller/dma/dma0chan7
>     ├── e6e68000.serial/dma:tx -> ../e6700000.dma-controller/dma/dma0chan6
>     ├── ee100000.sd/dma:rx -> ../e6700000.dma-controller/dma/dma0chan3
>     ├── ee100000.sd/dma:tx -> ../e6700000.dma-controller/dma/dma0chan2
>     ├── ee140000.sd/dma:rx -> ../e6720000.dma-controller/dma/dma1chan3
>     ├── ee140000.sd/dma:tx -> ../e6720000.dma-controller/dma/dma1chan2
>     ├── ee160000.sd/dma:rx -> ../e6700000.dma-controller/dma/dma0chan5
>     └── ee160000.sd/dma:tx -> ../e6700000.dma-controller/dma/dma0chan4
> 
> On r8a77951/salvator-xs:
> 
>     /sys/devices/platform/soc/
>     ├── e6460000.dma-controller/dma/dma4chan0/slave -> ../../../e659c000.usb
>     ├── e6460000.dma-controller/dma/dma4chan1/slave -> ../../../e659c000.usb
>     ├── e6470000.dma-controller/dma/dma5chan0/slave -> ../../../e659c000.usb
>     ├── e6470000.dma-controller/dma/dma5chan1/slave -> ../../../e659c000.usb
>     ├── e6510000.i2c/dma:tx -> ../e7300000.dma-controller/dma/dma7chan0
>     ├── e6550000.serial/dma:rx -> ../e7310000.dma-controller/dma/dma8chan1
>     ├── e6550000.serial/dma:tx -> ../e7310000.dma-controller/dma/dma8chan0
>     ├── e6590000.usb/dma:ch0 -> ../e65a0000.dma-controller/dma/dma2chan0
>     ├── e6590000.usb/dma:ch1 -> ../e65a0000.dma-controller/dma/dma2chan1
>     ├── e6590000.usb/dma:ch2 -> ../e65b0000.dma-controller/dma/dma3chan0
>     ├── e6590000.usb/dma:ch3 -> ../e65b0000.dma-controller/dma/dma3chan1
>     ├── e659c000.usb/dma:ch0 -> ../e6460000.dma-controller/dma/dma4chan0
>     ├── e659c000.usb/dma:ch1 -> ../e6460000.dma-controller/dma/dma4chan1
>     ├── e659c000.usb/dma:ch2 -> ../e6470000.dma-controller/dma/dma5chan0
>     ├── e659c000.usb/dma:ch3 -> ../e6470000.dma-controller/dma/dma5chan1
>     ├── e65a0000.dma-controller/dma/dma2chan0/slave -> ../../../e6590000.usb
>     ├── e65a0000.dma-controller/dma/dma2chan1/slave -> ../../../e6590000.usb
>     ├── e65b0000.dma-controller/dma/dma3chan0/slave -> ../../../e6590000.usb
>     ├── e65b0000.dma-controller/dma/dma3chan1/slave -> ../../../e6590000.usb
>     ├── e7300000.dma-controller/dma/dma7chan0/slave -> ../../../e6510000.i2c
>     ├── e7310000.dma-controller/dma/dma8chan0/slave -> ../../../e6550000.serial
>     └── e7310000.dma-controller/dma/dma8chan1/slave -> ../../../e6550000.serial
> ---
>  drivers/dma/dmaengine.c   | 37 +++++++++++++++++++++++++++++++------
>  include/linux/dmaengine.h |  4 ++++
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 56a8420c388679d3..617c84cf6800962b 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -60,6 +60,8 @@ static long dmaengine_ref_count;
>  
>  /* --- sysfs implementation --- */
>  
> +#define DMA_SLAVE_NAME	"slave"
> +
>  /**
>   * dev_to_dma_chan - convert a device pointer to its sysfs container object
>   * @dev - device node
> @@ -730,11 +732,11 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>  	if (has_acpi_companion(dev) && !chan)
>  		chan = acpi_dma_request_slave_chan_by_name(dev, name);
>  
> -	if (chan) {
> -		/* Valid channel found or requester needs to be deferred */
> -		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> -			return chan;
> -	}
> +	if (PTR_ERR(chan) == -EPROBE_DEFER)
> +		return chan;
> +
> +	if (!IS_ERR_OR_NULL(chan))
> +		goto found;
>  
>  	/* Try to find the channel via the DMA filter map(s) */
>  	mutex_lock(&dma_list_mutex);
> @@ -754,7 +756,23 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>  	}
>  	mutex_unlock(&dma_list_mutex);
>  
> -	return chan ? chan : ERR_PTR(-EPROBE_DEFER);
> +	if (!IS_ERR_OR_NULL(chan))
> +		goto found;
> +
> +	return ERR_PTR(-EPROBE_DEFER);
> +
> +found:
> +	chan->slave = dev;
> +	chan->name = kasprintf(GFP_KERNEL, "dma:%s", name);
> +	if (!chan->name)
> +		return ERR_PTR(-ENOMEM);

You will lock the channel... It is requested, but it is not released in
case of failure.

> +
> +	if (sysfs_create_link(&chan->dev->device.kobj, &dev->kobj,
> +			      DMA_SLAVE_NAME))
> +		dev_err(dev, "Cannot create DMA %s symlink\n", DMA_SLAVE_NAME);
> +	if (sysfs_create_link(&dev->kobj, &chan->dev->device.kobj, chan->name))
> +		dev_err(dev, "Cannot create DMA %s symlink\n", chan->name);

It is not a problem if these fail?

> +	return chan;
>  }
>  EXPORT_SYMBOL_GPL(dma_request_chan);
>  
> @@ -812,6 +830,13 @@ void dma_release_channel(struct dma_chan *chan)
>  	/* drop PRIVATE cap enabled by __dma_request_channel() */
>  	if (--chan->device->privatecnt == 0)
>  		dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
> +	if (chan->slave) {
> +		sysfs_remove_link(&chan->slave->kobj, chan->name);
> +		kfree(chan->name);
> +		chan->name = NULL;
> +		chan->slave = NULL;
> +	}
> +	sysfs_remove_link(&chan->dev->device.kobj, DMA_SLAVE_NAME);

If a non slave channel is released, then you remove the link you have
never created?

What happens if the link creation fails and here you attempt to remove
the failed ones?

>  	mutex_unlock(&dma_list_mutex);
>  }
>  EXPORT_SYMBOL_GPL(dma_release_channel);
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 2cd1d6d7ef0fcce5..2804da93e27e114b 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -238,10 +238,12 @@ struct dma_router {
>  /**
>   * struct dma_chan - devices supply DMA channels, clients use them
>   * @device: ptr to the dma device who supplies this channel, always !%NULL
> + * @slave: ptr to the device using this channel
>   * @cookie: last cookie value returned to client
>   * @completed_cookie: last completed cookie for this channel
>   * @chan_id: channel ID for sysfs
>   * @dev: class device for sysfs
> + * @name: backlink name for sysfs
>   * @device_node: used to add this to the device chan list
>   * @local: per-cpu pointer to a struct dma_chan_percpu
>   * @client_count: how many clients are using this channel
> @@ -252,12 +254,14 @@ struct dma_router {
>   */
>  struct dma_chan {
>  	struct dma_device *device;
> +	struct device *slave;
>  	dma_cookie_t cookie;
>  	dma_cookie_t completed_cookie;
>  
>  	/* sysfs */
>  	int chan_id;
>  	struct dma_chan_dev *dev;
> +	const char *name;
>  
>  	struct list_head device_node;
>  	struct dma_chan_percpu __percpu *local;
> 

- Péter

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

  parent reply	other threads:[~2020-01-30  9:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-17 15:30 [PATCH v2] dmaengine: Create symlinks between DMA channels and slaves Geert Uytterhoeven
2020-01-17 16:26 ` Niklas Söderlund
2020-01-17 20:10 ` Peter Ujfalusi
2020-01-20  9:01   ` Geert Uytterhoeven
2020-01-20 10:16     ` Peter Ujfalusi
2020-01-20 10:51       ` Geert Uytterhoeven
2020-01-20 12:06         ` Peter Ujfalusi
2020-01-21 20:22           ` Geert Uytterhoeven
2020-01-22  9:40             ` Vinod Koul
2020-01-24  6:13               ` Vinod Koul
2020-01-24  7:31                 ` Peter Ujfalusi
2020-01-27  5:08                   ` Vinod Koul
     [not found] ` <CGME20200129174723eucas1p1fe4f76325f463fc9e3645ce18740d2eb@eucas1p1.samsung.com>
2020-01-29 17:47   ` Marek Szyprowski
2020-01-30  8:30     ` Geert Uytterhoeven
2020-01-30 10:33       ` Marek Szyprowski
2020-01-30 10:47         ` Geert Uytterhoeven
2020-01-30  9:43 ` Peter Ujfalusi [this message]
2020-01-30  9:51   ` Geert Uytterhoeven
2020-01-30 10:22     ` Peter Ujfalusi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e219bc58-0d30-582f-4872-559097f212d2@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.