* [PATCH 0/2] dmaengine: Cleanups for symlink handling and debugfs support @ 2020-01-30 11:42 Peter Ujfalusi 2020-01-30 11:42 ` [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support Peter Ujfalusi 2020-01-30 11:42 ` [PATCH 2/2] dmaengine: Add basic debugfs support Peter Ujfalusi 0 siblings, 2 replies; 7+ messages in thread From: Peter Ujfalusi @ 2020-01-30 11:42 UTC (permalink / raw) To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams, geert Hi, As I have mentioned on the symlink patch earlier I like how the gpio's debugfs shows in one place information. These patches are on top of Vinod's next (with the v2 fix for the symlink support). The first patch fixes and cleans up the symlink handling code a bit and the second adds support for debugfs file: On my board with audio and after a run with dmatest on 6 channels this is how the information is presented about the DMA drivers: # cat /sys/kernel/debug/dmaengine dma0 (285c0000.dma-controller): number of channels: 96 dma1 (31150000.dma-controller): number of channels: 267 dma1chan0: 2b00000.mcasp:tx dma1chan1: 2b00000.mcasp:rx dma1chan2: in-use dma1chan3: in-use dma1chan4: in-use dma1chan5: in-use dma1chan6: in-use dma1chan7: in-use It shows the users (device name + channel name) of the channels. If it is not a slave channel, then it only prints 'in-use' as no other information is available for non save channels. DMA drivers can implement the dbg_show callback to provide custom information for their channels if needed. Regards, Peter --- Peter Ujfalusi (2): dmaengine: Cleanups for the slave <-> channel symlink support dmaengine: Add basic debugfs support drivers/dma/dmaengine.c | 143 +++++++++++++++++++++++++++++++++++--- include/linux/dmaengine.h | 12 +++- 2 files changed, 144 insertions(+), 11 deletions(-) -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support 2020-01-30 11:42 [PATCH 0/2] dmaengine: Cleanups for symlink handling and debugfs support Peter Ujfalusi @ 2020-01-30 11:42 ` Peter Ujfalusi 2020-01-30 15:20 ` Geert Uytterhoeven 2020-01-30 11:42 ` [PATCH 2/2] dmaengine: Add basic debugfs support Peter Ujfalusi 1 sibling, 1 reply; 7+ messages in thread From: Peter Ujfalusi @ 2020-01-30 11:42 UTC (permalink / raw) To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams, geert No need to use goto to jump over the return chan ? chan : ERR_PTR(-EPROBE_DEFER); We can just revert the check and return right there. Do not fail the channel request if the chan->name allocation fails, but print a warning about it. Change the dev_err to dev_warn if sysfs_create_link() fails as it is not fatal. Only attempt to remove the DMA_SLAVE_NAME symlink if it is created - or it was attempted to be created. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/dma/dmaengine.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 7b1cefc3213a..75516f9fbab4 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -756,22 +756,24 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) } mutex_unlock(&dma_list_mutex); - if (!IS_ERR_OR_NULL(chan)) - goto found; - - return chan ? chan : ERR_PTR(-EPROBE_DEFER); + if (IS_ERR_OR_NULL(chan)) + return chan ? chan : ERR_PTR(-EPROBE_DEFER); found: - chan->slave = dev; chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); - if (!chan->name) - return ERR_PTR(-ENOMEM); + if (!chan->name) { + dev_warn(dev, + "Cannot allocate memory for slave symlink name\n"); + return chan; + } + chan->slave = dev; 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); + dev_warn(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); + dev_warn(dev, "Cannot create DMA %s symlink\n", chan->name); + return chan; } EXPORT_SYMBOL_GPL(dma_request_chan); @@ -830,13 +832,14 @@ 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->dev->device.kobj, DMA_SLAVE_NAME); 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); mutex_unlock(&dma_list_mutex); } EXPORT_SYMBOL_GPL(dma_release_channel); -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support 2020-01-30 11:42 ` [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support Peter Ujfalusi @ 2020-01-30 15:20 ` Geert Uytterhoeven 2020-01-31 7:14 ` Peter Ujfalusi 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2020-01-30 15:20 UTC (permalink / raw) To: Peter Ujfalusi; +Cc: Vinod, dmaengine, Linux Kernel Mailing List, Dan Williams Hi Peter, On Thu, Jan 30, 2020 at 12:41 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > No need to use goto to jump over the > return chan ? chan : ERR_PTR(-EPROBE_DEFER); > We can just revert the check and return right there. > > Do not fail the channel request if the chan->name allocation fails, but > print a warning about it. > > Change the dev_err to dev_warn if sysfs_create_link() fails as it is not > fatal. > > Only attempt to remove the DMA_SLAVE_NAME symlink if it is created - or it > was attempted to be created. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Thanks for your patch! > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -756,22 +756,24 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) > } > mutex_unlock(&dma_list_mutex); > > - if (!IS_ERR_OR_NULL(chan)) > - goto found; > - > - return chan ? chan : ERR_PTR(-EPROBE_DEFER); > + if (IS_ERR_OR_NULL(chan)) > + return chan ? chan : ERR_PTR(-EPROBE_DEFER); > > found: > - chan->slave = dev; > chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); > - if (!chan->name) > - return ERR_PTR(-ENOMEM); > + if (!chan->name) { > + dev_warn(dev, > + "Cannot allocate memory for slave symlink name\n"); No need to print a message, as the memory allocator core will have screamed already. > + return chan; > + } > + chan->slave = dev; > > 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); > + dev_warn(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); > + dev_warn(dev, "Cannot create DMA %s symlink\n", chan->name); > + > return chan; > } > EXPORT_SYMBOL_GPL(dma_request_chan); With the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support 2020-01-30 15:20 ` Geert Uytterhoeven @ 2020-01-31 7:14 ` Peter Ujfalusi 0 siblings, 0 replies; 7+ messages in thread From: Peter Ujfalusi @ 2020-01-31 7:14 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Vinod, dmaengine, Linux Kernel Mailing List, Dan Williams Hi Geert, On 30/01/2020 17.20, Geert Uytterhoeven wrote: > Hi Peter, > > On Thu, Jan 30, 2020 at 12:41 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >> No need to use goto to jump over the >> return chan ? chan : ERR_PTR(-EPROBE_DEFER); >> We can just revert the check and return right there. >> >> Do not fail the channel request if the chan->name allocation fails, but >> print a warning about it. >> >> Change the dev_err to dev_warn if sysfs_create_link() fails as it is not >> fatal. >> >> Only attempt to remove the DMA_SLAVE_NAME symlink if it is created - or it >> was attempted to be created. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > Thanks for your patch! > >> --- a/drivers/dma/dmaengine.c >> +++ b/drivers/dma/dmaengine.c >> @@ -756,22 +756,24 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) >> } >> mutex_unlock(&dma_list_mutex); >> >> - if (!IS_ERR_OR_NULL(chan)) >> - goto found; >> - >> - return chan ? chan : ERR_PTR(-EPROBE_DEFER); >> + if (IS_ERR_OR_NULL(chan)) >> + return chan ? chan : ERR_PTR(-EPROBE_DEFER); >> >> found: >> - chan->slave = dev; >> chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); >> - if (!chan->name) >> - return ERR_PTR(-ENOMEM); >> + if (!chan->name) { >> + dev_warn(dev, >> + "Cannot allocate memory for slave symlink name\n"); > > No need to print a message, as the memory allocator core will have > screamed already. Right, I tend to forget this ;) > >> + return chan; >> + } >> + chan->slave = dev; >> >> 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); >> + dev_warn(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); >> + dev_warn(dev, "Cannot create DMA %s symlink\n", chan->name); >> + >> return chan; >> } >> EXPORT_SYMBOL_GPL(dma_request_chan); > > With the above fixed: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks, - Péter > > Gr{oetje,eeting}s, > > Geert > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] dmaengine: Add basic debugfs support 2020-01-30 11:42 [PATCH 0/2] dmaengine: Cleanups for symlink handling and debugfs support Peter Ujfalusi 2020-01-30 11:42 ` [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support Peter Ujfalusi @ 2020-01-30 11:42 ` Peter Ujfalusi 2020-01-30 15:42 ` Geert Uytterhoeven 1 sibling, 1 reply; 7+ messages in thread From: Peter Ujfalusi @ 2020-01-30 11:42 UTC (permalink / raw) To: vkoul; +Cc: dmaengine, linux-kernel, dan.j.williams, geert Via the /sys/kernel/debug/dmaengine users can get information about the DMA devices and the used channels. Example output on am654-evm with audio using two channels and after running dmatest on 6 channels: # cat /sys/kernel/debug/dmaengine dma0 (285c0000.dma-controller): number of channels: 96 dma1 (31150000.dma-controller): number of channels: 267 dma1chan0: 2b00000.mcasp:tx dma1chan1: 2b00000.mcasp:rx dma1chan2: in-use dma1chan3: in-use dma1chan4: in-use dma1chan5: in-use dma1chan6: in-use dma1chan7: in-use For slave channels we can show the device and the channel name a given channel is requested. For non slave devices the only information we know is that the channel is in use. DMA drivers can implement the optional dbg_show callback to provide controller specific information instead of the generic one. It is easy to extend the generic dmaengine_dbg_show() to print additional information about the used channels. I have taken the idea from gpiolib. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/dma/dmaengine.c | 120 ++++++++++++++++++++++++++++++++++++++ include/linux/dmaengine.h | 12 +++- 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 75516f9fbab4..7573a4d0f9d7 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -32,6 +32,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/platform_device.h> +#include <linux/debugfs.h> #include <linux/dma-mapping.h> #include <linux/init.h> #include <linux/module.h> @@ -760,6 +761,13 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) return chan ? chan : ERR_PTR(-EPROBE_DEFER); found: +#ifdef CONFIG_DEBUG_FS + chan->slave_name = kasprintf(GFP_KERNEL, "%s:%s", dev_name(dev), name); + if (!chan->slave_name) + dev_warn(dev, + "Cannot allocate memory for slave name (debugfs)\n"); +#endif + chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); if (!chan->name) { dev_warn(dev, @@ -840,6 +848,13 @@ void dma_release_channel(struct dma_chan *chan) chan->name = NULL; chan->slave = NULL; } + +#ifdef CONFIG_DEBUG_FS + if (chan->slave_name) { + kfree(chan->slave_name); + chan->slave_name = NULL; + } +#endif mutex_unlock(&dma_list_mutex); } EXPORT_SYMBOL_GPL(dma_release_channel); @@ -1562,3 +1577,108 @@ static int __init dma_bus_init(void) return class_register(&dma_devclass); } arch_initcall(dma_bus_init); + +#ifdef CONFIG_DEBUG_FS +static void *dmaengine_seq_start(struct seq_file *s, loff_t *pos) +{ + struct dma_device *dma_dev = NULL; + loff_t index = *pos; + + s->private = ""; + + mutex_lock(&dma_list_mutex); + list_for_each_entry(dma_dev, &dma_device_list, global_node) + if (index-- == 0) { + mutex_unlock(&dma_list_mutex); + return dma_dev; + } + mutex_unlock(&dma_list_mutex); + + return NULL; +} + +static void *dmaengine_seq_next(struct seq_file *s, void *v, loff_t *pos) +{ + struct dma_device *dma_dev = v; + void *ret = NULL; + + mutex_lock(&dma_list_mutex); + if (list_is_last(&dma_dev->global_node, &dma_device_list)) + ret = NULL; + else + ret = list_entry(dma_dev->global_node.next, + struct dma_device, global_node); + mutex_unlock(&dma_list_mutex); + + s->private = "\n"; + ++*pos; + + return ret; +} + +static void dmaengine_seq_stop(struct seq_file *s, void *v) +{ +} + +static void dmaengine_dbg_show(struct seq_file *s, struct dma_device *dma_dev) +{ + struct dma_chan *chan; + + list_for_each_entry(chan, &dma_dev->channels, device_node) { + if (chan->client_count) { + seq_printf(s, " dma%dchan%d:", dma_dev->dev_id, + chan->chan_id); + if (chan->slave_name) + seq_printf(s, "\t\t%s\n", chan->slave_name); + else + seq_printf(s, "\t\t%s\n", "in-use"); + } + } +} + +static int dmaengine_seq_show(struct seq_file *s, void *v) +{ + struct dma_device *dma_dev = v; + + seq_printf(s, "%sdma%d (%s): number of channels: %u\n", + (char *)s->private, dma_dev->dev_id, dev_name(dma_dev->dev), + dma_dev->chancnt); + + if (dma_dev->dbg_show) + dma_dev->dbg_show(s, dma_dev); + else + dmaengine_dbg_show(s, dma_dev); + + return 0; +} + +static const struct seq_operations dmaengine_seq_ops = { + .start = dmaengine_seq_start, + .next = dmaengine_seq_next, + .stop = dmaengine_seq_stop, + .show = dmaengine_seq_show, +}; + +static int dmaengine_open(struct inode *inode, struct file *file) +{ + return seq_open(file, &dmaengine_seq_ops); +} + +static const struct file_operations dmaengine_operations = { + .owner = THIS_MODULE, + .open = dmaengine_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release, +}; + +static int __init dmaengine_debugfs_init(void) +{ + /* /sys/kernel/debug/dmaengine */ + debugfs_create_file("dmaengine", S_IFREG | 0444, NULL, NULL, + &dmaengine_operations); + return 0; +} +subsys_initcall(dmaengine_debugfs_init); + +#endif /* DEBUG_FS */ diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 64461fc64e1b..5b9d6b1aa6e9 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -300,6 +300,8 @@ struct dma_router { * @chan_id: channel ID for sysfs * @dev: class device for sysfs * @name: backlink name for sysfs + * @slave_name: slave name for debugfs in format: + * dev_name(requester's dev):channel name, for example: "2b00000.mcasp:tx" * @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 @@ -318,6 +320,9 @@ struct dma_chan { int chan_id; struct dma_chan_dev *dev; const char *name; +#ifdef CONFIG_DEBUG_FS + char *slave_name; +#endif struct list_head device_node; struct dma_chan_percpu __percpu *local; @@ -805,7 +810,9 @@ struct dma_filter { * called and there are no further references to this structure. This * must be implemented to free resources however many existing drivers * do not and are therefore not safe to unbind while in use. - * + * @dbg_show: optional routine to show contents in debugfs; default code + * will be used when this is omitted, but custom code can show extra, + * controller specific information. */ struct dma_device { struct kref ref; @@ -891,6 +898,9 @@ struct dma_device { struct dma_tx_state *txstate); void (*device_issue_pending)(struct dma_chan *chan); void (*device_release)(struct dma_device *dev); +#ifdef CONFIG_DEBUG_FS + void (*dbg_show)(struct seq_file *s, struct dma_device *dev); +#endif }; static inline int dmaengine_slave_config(struct dma_chan *chan, -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: Add basic debugfs support 2020-01-30 11:42 ` [PATCH 2/2] dmaengine: Add basic debugfs support Peter Ujfalusi @ 2020-01-30 15:42 ` Geert Uytterhoeven 2020-01-31 8:29 ` Peter Ujfalusi 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2020-01-30 15:42 UTC (permalink / raw) To: Peter Ujfalusi; +Cc: Vinod, dmaengine, Linux Kernel Mailing List, Dan Williams Hi Peter, On Thu, Jan 30, 2020 at 12:41 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > Via the /sys/kernel/debug/dmaengine users can get information about the > DMA devices and the used channels. > > Example output on am654-evm with audio using two channels and after running > dmatest on 6 channels: > > # cat /sys/kernel/debug/dmaengine > dma0 (285c0000.dma-controller): number of channels: 96 > > dma1 (31150000.dma-controller): number of channels: 267 > dma1chan0: 2b00000.mcasp:tx > dma1chan1: 2b00000.mcasp:rx > dma1chan2: in-use > dma1chan3: in-use > dma1chan4: in-use > dma1chan5: in-use > dma1chan6: in-use > dma1chan7: in-use > > For slave channels we can show the device and the channel name a given > channel is requested. > For non slave devices the only information we know is that the channel is > in use. > > DMA drivers can implement the optional dbg_show callback to provide > controller specific information instead of the generic one. > > It is easy to extend the generic dmaengine_dbg_show() to print additional > information about the used channels. > > I have taken the idea from gpiolib. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Thanks for your patch! On Salvator-XS with R-Car H3 ES2.0: dma0 (ec700000.dma-controller): number of channels: 15 dma1 (ec720000.dma-controller): number of channels: 15 dma2 (e65a0000.dma-controller): number of channels: 2 dma2chan0: e6590000.usb:ch0 dma2chan1: e6590000.usb:ch1 dma3 (e65b0000.dma-controller): number of channels: 2 dma3chan0: e6590000.usb:ch2 dma3chan1: e6590000.usb:ch3 dma4 (e6460000.dma-controller): number of channels: 2 dma4chan0: e659c000.usb:ch0 dma4chan1: e659c000.usb:ch1 dma5 (e6470000.dma-controller): number of channels: 2 dma5chan0: e659c000.usb:ch2 dma5chan1: e659c000.usb:ch3 dma6 (e6700000.dma-controller): number of channels: 15 dma7 (e7300000.dma-controller): number of channels: 15 dma7chan0: e6510000.i2c:tx dma8 (e7310000.dma-controller): number of channels: 15 dma8chan0: e6550000.serial:tx dma8chan1: e6550000.serial:rx > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c > @@ -760,6 +761,13 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) > return chan ? chan : ERR_PTR(-EPROBE_DEFER); > > found: > +#ifdef CONFIG_DEBUG_FS > + chan->slave_name = kasprintf(GFP_KERNEL, "%s:%s", dev_name(dev), name); > + if (!chan->slave_name) > + dev_warn(dev, > + "Cannot allocate memory for slave name (debugfs)\n"); No need to print a message, as the memory allocation core already takes care of that. But, do you really need chan->slave_name? You already have chan->slave and chan->name. > +#endif > + > chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); > if (!chan->name) { > dev_warn(dev, > @@ -1562,3 +1577,108 @@ static int __init dma_bus_init(void) > return class_register(&dma_devclass); > } > arch_initcall(dma_bus_init); > + > +#ifdef CONFIG_DEBUG_FS > +static void *dmaengine_seq_start(struct seq_file *s, loff_t *pos) > +{ > + struct dma_device *dma_dev = NULL; > + loff_t index = *pos; > + > + s->private = ""; > + > + mutex_lock(&dma_list_mutex); > + list_for_each_entry(dma_dev, &dma_device_list, global_node) > + if (index-- == 0) { > + mutex_unlock(&dma_list_mutex); > + return dma_dev; Can the dma_device go away after unlocking the list? Unlike dma_request_chan(), this doesn't increase a refcnt. > + } > + mutex_unlock(&dma_list_mutex); > + > + return NULL; > +} > + > +static void *dmaengine_seq_next(struct seq_file *s, void *v, loff_t *pos) > +{ > + struct dma_device *dma_dev = v; > + void *ret = NULL; > + > + mutex_lock(&dma_list_mutex); > + if (list_is_last(&dma_dev->global_node, &dma_device_list)) > + ret = NULL; > + else > + ret = list_entry(dma_dev->global_node.next, > + struct dma_device, global_node); > + mutex_unlock(&dma_list_mutex); Likewise. > + > + s->private = "\n"; > + ++*pos; > + > + return ret; > +} > + > +static void dmaengine_seq_stop(struct seq_file *s, void *v) > +{ > +} > + > +static void dmaengine_dbg_show(struct seq_file *s, struct dma_device *dma_dev) > +{ > + struct dma_chan *chan; > + > + list_for_each_entry(chan, &dma_dev->channels, device_node) { > + if (chan->client_count) { > + seq_printf(s, " dma%dchan%d:", dma_dev->dev_id, > + chan->chan_id); > + if (chan->slave_name) > + seq_printf(s, "\t\t%s\n", chan->slave_name); > + else > + seq_printf(s, "\t\t%s\n", "in-use"); The truncated ternary operator might help here: seq_printf(s, "\t\t%s\n", chan->slave_name ?: "in-use"); However, you might as well just use dev_name(chan->slave) and chan->name instead of chan->slave_name. > + } > + } > +} > + > +static int dmaengine_seq_show(struct seq_file *s, void *v) > +{ > + struct dma_device *dma_dev = v; > + > + seq_printf(s, "%sdma%d (%s): number of channels: %u\n", > + (char *)s->private, dma_dev->dev_id, dev_name(dma_dev->dev), > + dma_dev->chancnt); > + > + if (dma_dev->dbg_show) > + dma_dev->dbg_show(s, dma_dev); So providing a custom .dbg_show() means replacing the standard info, not augmenting it? > + else > + dmaengine_dbg_show(s, dma_dev); > + > + return 0; > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] dmaengine: Add basic debugfs support 2020-01-30 15:42 ` Geert Uytterhoeven @ 2020-01-31 8:29 ` Peter Ujfalusi 0 siblings, 0 replies; 7+ messages in thread From: Peter Ujfalusi @ 2020-01-31 8:29 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Vinod, dmaengine, Linux Kernel Mailing List, Dan Williams Hi Geert, On 30/01/2020 17.42, Geert Uytterhoeven wrote: > Hi Peter, > > On Thu, Jan 30, 2020 at 12:41 PM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >> Via the /sys/kernel/debug/dmaengine users can get information about the >> DMA devices and the used channels. >> >> Example output on am654-evm with audio using two channels and after running >> dmatest on 6 channels: >> >> # cat /sys/kernel/debug/dmaengine >> dma0 (285c0000.dma-controller): number of channels: 96 >> >> dma1 (31150000.dma-controller): number of channels: 267 >> dma1chan0: 2b00000.mcasp:tx >> dma1chan1: 2b00000.mcasp:rx >> dma1chan2: in-use >> dma1chan3: in-use >> dma1chan4: in-use >> dma1chan5: in-use >> dma1chan6: in-use >> dma1chan7: in-use >> >> For slave channels we can show the device and the channel name a given >> channel is requested. >> For non slave devices the only information we know is that the channel is >> in use. >> >> DMA drivers can implement the optional dbg_show callback to provide >> controller specific information instead of the generic one. >> >> It is easy to extend the generic dmaengine_dbg_show() to print additional >> information about the used channels. >> >> I have taken the idea from gpiolib. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > Thanks for your patch! > > On Salvator-XS with R-Car H3 ES2.0: > > dma0 (ec700000.dma-controller): number of channels: 15 > > dma1 (ec720000.dma-controller): number of channels: 15 > > dma2 (e65a0000.dma-controller): number of channels: 2 > dma2chan0: e6590000.usb:ch0 > dma2chan1: e6590000.usb:ch1 > > dma3 (e65b0000.dma-controller): number of channels: 2 > dma3chan0: e6590000.usb:ch2 > dma3chan1: e6590000.usb:ch3 > > dma4 (e6460000.dma-controller): number of channels: 2 > dma4chan0: e659c000.usb:ch0 > dma4chan1: e659c000.usb:ch1 > > dma5 (e6470000.dma-controller): number of channels: 2 > dma5chan0: e659c000.usb:ch2 > dma5chan1: e659c000.usb:ch3 > > dma6 (e6700000.dma-controller): number of channels: 15 > > dma7 (e7300000.dma-controller): number of channels: 15 > dma7chan0: e6510000.i2c:tx > > dma8 (e7310000.dma-controller): number of channels: 15 > dma8chan0: e6550000.serial:tx > dma8chan1: e6550000.serial:rx You have lots of DMAs over there ;) >> --- a/drivers/dma/dmaengine.c >> +++ b/drivers/dma/dmaengine.c >> @@ -760,6 +761,13 @@ struct dma_chan *dma_request_chan(struct device *dev, const char *name) >> return chan ? chan : ERR_PTR(-EPROBE_DEFER); >> >> found: >> +#ifdef CONFIG_DEBUG_FS >> + chan->slave_name = kasprintf(GFP_KERNEL, "%s:%s", dev_name(dev), name); >> + if (!chan->slave_name) >> + dev_warn(dev, >> + "Cannot allocate memory for slave name (debugfs)\n"); > > No need to print a message, as the memory allocation core already takes > care of that. Right. > But, do you really need chan->slave_name? > You already have chan->slave and chan->name. The chan->name is prefixed with "dma:" it would not look right. In production this all go away as debugfs most likely disabled. But I will change the name to dbg_client_name. > >> +#endif >> + >> chan->name = kasprintf(GFP_KERNEL, "dma:%s", name); >> if (!chan->name) { >> dev_warn(dev, > >> @@ -1562,3 +1577,108 @@ static int __init dma_bus_init(void) >> return class_register(&dma_devclass); >> } >> arch_initcall(dma_bus_init); >> + >> +#ifdef CONFIG_DEBUG_FS >> +static void *dmaengine_seq_start(struct seq_file *s, loff_t *pos) >> +{ >> + struct dma_device *dma_dev = NULL; >> + loff_t index = *pos; >> + >> + s->private = ""; >> + >> + mutex_lock(&dma_list_mutex); >> + list_for_each_entry(dma_dev, &dma_device_list, global_node) >> + if (index-- == 0) { >> + mutex_unlock(&dma_list_mutex); >> + return dma_dev; > > Can the dma_device go away after unlocking the list? > Unlike dma_request_chan(), this doesn't increase a refcnt. It could, let me see what I can do. Probably locking the dma_device_list for the duration of the show. >> + } >> + mutex_unlock(&dma_list_mutex); >> + >> + return NULL; >> +} >> + >> +static void *dmaengine_seq_next(struct seq_file *s, void *v, loff_t *pos) >> +{ >> + struct dma_device *dma_dev = v; >> + void *ret = NULL; >> + >> + mutex_lock(&dma_list_mutex); >> + if (list_is_last(&dma_dev->global_node, &dma_device_list)) >> + ret = NULL; >> + else >> + ret = list_entry(dma_dev->global_node.next, >> + struct dma_device, global_node); >> + mutex_unlock(&dma_list_mutex); > > Likewise. > >> + >> + s->private = "\n"; >> + ++*pos; >> + >> + return ret; >> +} >> + >> +static void dmaengine_seq_stop(struct seq_file *s, void *v) >> +{ >> +} >> + >> +static void dmaengine_dbg_show(struct seq_file *s, struct dma_device *dma_dev) >> +{ >> + struct dma_chan *chan; >> + >> + list_for_each_entry(chan, &dma_dev->channels, device_node) { >> + if (chan->client_count) { >> + seq_printf(s, " dma%dchan%d:", dma_dev->dev_id, >> + chan->chan_id); >> + if (chan->slave_name) >> + seq_printf(s, "\t\t%s\n", chan->slave_name); >> + else >> + seq_printf(s, "\t\t%s\n", "in-use"); > > The truncated ternary operator might help here: > > seq_printf(s, "\t\t%s\n", chan->slave_name ?: "in-use"); > > However, you might as well just use dev_name(chan->slave) and chan->name > instead of chan->slave_name. "2b00000.mcasp" + "dma:tx" would be an awkward combination ;) > >> + } >> + } >> +} >> + >> +static int dmaengine_seq_show(struct seq_file *s, void *v) >> +{ >> + struct dma_device *dma_dev = v; >> + >> + seq_printf(s, "%sdma%d (%s): number of channels: %u\n", >> + (char *)s->private, dma_dev->dev_id, dev_name(dma_dev->dev), >> + dma_dev->chancnt); >> + >> + if (dma_dev->dbg_show) >> + dma_dev->dbg_show(s, dma_dev); > > So providing a custom .dbg_show() means replacing the standard info, not > augmenting it? Correct, if a DMA driver decides to implement it, then it is it's responsibility to show things after the "dma%d (%s): number of channels: %u\n" line. The standard infor is pretty minimal and not sure if it can be more verbose. Oh, I can add the router information if it is used. > >> + else >> + dmaengine_dbg_show(s, dma_dev); >> + >> + return 0; >> +} > > Gr{oetje,eeting}s, > > Geert > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-01-31 8:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-30 11:42 [PATCH 0/2] dmaengine: Cleanups for symlink handling and debugfs support Peter Ujfalusi 2020-01-30 11:42 ` [PATCH 1/2] dmaengine: Cleanups for the slave <-> channel symlink support Peter Ujfalusi 2020-01-30 15:20 ` Geert Uytterhoeven 2020-01-31 7:14 ` Peter Ujfalusi 2020-01-30 11:42 ` [PATCH 2/2] dmaengine: Add basic debugfs support Peter Ujfalusi 2020-01-30 15:42 ` Geert Uytterhoeven 2020-01-31 8:29 ` Peter Ujfalusi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).