Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1372288 > unrolled thread

[PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space

Back to article view | Back to linux.kernel

Contents

  [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-06 10:40 +0200
    Re: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available  space Greg KH <gregkh@linuxfoundation.org> - 2016-04-06 11:30 +0200
      RE: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue  available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-06 13:40 +0200
        RE: [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-06 14:30 +0200
      [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs changbin.du@intel.com - 2016-04-06 18:00 +0200
        [PATCH v2 3/3] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-06 18:00 +0200
        Re: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs Felipe Balbi <balbi@kernel.org> - 2016-04-07 07:10 +0200
          RE: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs "Du, Changbin" <changbin.du@intel.com> - 2016-04-07 07:30 +0200
            RE: [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs Felipe Balbi <balbi@kernel.org> - 2016-04-07 07:30 +0200
          [PATCH v3 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-08 11:50 +0200
            [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du@intel.com - 2016-04-08 11:50 +0200
              Re: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void Felipe Balbi <balbi@kernel.org> - 2016-04-11 10:20 +0200
                RE: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value  be void "Du, Changbin" <changbin.du@intel.com> - 2016-04-11 13:20 +0200
                  RE: [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void Felipe Balbi <balbi@kernel.org> - 2016-04-11 13:30 +0200
                    [PATCH v4 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-12 13:30 +0200
                      [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-12 13:30 +0200
                        Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue  available space Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-04-12 15:00 +0200
                          RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue  available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 05:30 +0200
                        Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 10:10 +0200
                          RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue  available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 13:20 +0200
                            RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 13:30 +0200
                              RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue  available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 13:40 +0200
                                RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space Felipe Balbi <balbi@kernel.org> - 2016-04-14 13:50 +0200
                                  RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue  available space "Du, Changbin" <changbin.du@intel.com> - 2016-04-14 14:00 +0200
                      [PATCH v4 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du@intel.com - 2016-04-12 13:30 +0200
            [PATCH v3 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du@intel.com - 2016-04-08 11:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1378700 — RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space

[Multipart message — attachments visible in raw view] — view raw

Hi,

"Du, Changbin" <changbin.du@intel.com> writes:
> Hi, Balbi.
>
> Feel free to change it, I may not have enough time on this currently.
> "per-endpoint directory" is great idea, then we do not need find out
> wanted info from one big file, but just go to specific dir. 

that was the idea, glad you liked it ;-)

> Btw, I'd mention that not all out ep has a rx fifo. So in my original patch,

yeah, rx fifos are dynamically allocated by the HW itself, AFAICT.

> not all FIFO/Queue info are valid. We need pick out the real info we need.
> And I didn't find any method to read the FIFO map.
>
> At last, comparing with the FIFO/Queue info, I think software transfer
> Requests list, TRBs info, EVENTs history are much more useful for debugging
> the driver. If you can also add these info to each EP folder, that is awesome!
> :)

I'll think about adding these but for the lifetime of requests and trbs
and events, etc, we have tracepoints for that. I usually do the
following when debugging:

# mount -t debugfs none /sys/kernel/debug
# cd /sys/kernel/debug/tracing
# echo 2048 > buffer_size_kb
# echo 1 > events/dwc3/enable

(do something to break it)

# cp trace /mnt/sdcard # or something like that

then read the file. You can make it as large or as small as you like
(given some constraints, of course ;-) but I've had no issues allocating
128MiB in the past.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1378707 — RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space

> > At last, comparing with the FIFO/Queue info, I think software transfer
> > Requests list, TRBs info, EVENTs history are much more useful for
> debugging
> > the driver. If you can also add these info to each EP folder, that is awesome!
> > :)
> 
> I'll think about adding these but for the lifetime of requests and trbs
> and events, etc, we have tracepoints for that. I usually do the
> following when debugging:
> 
> # mount -t debugfs none /sys/kernel/debug
> # cd /sys/kernel/debug/tracing
> # echo 2048 > buffer_size_kb
> # echo 1 > events/dwc3/enable
> 
> (do something to break it)
> 
> # cp trace /mnt/sdcard # or something like that
> 
> then read the file. You can make it as large or as small as you like
> (given some constraints, of course ;-) but I've had no issues allocating
> 128MiB in the past.
> 
> --
> Balbi

Thanks for the sharing, this is a good approach to capture dynamic
behaviors. But a dump of current state has below advantages:
1. a quick view for the pending transfers. Then we can quickly 
     checking the transfer status.
2. no side-effect. This is important in some case. We usually
    encounter some transfer issues but very hard to reproduce
    it. But we cannot enable trace all the time since performance
    concern. Then I thought it was so great if I can have a look for
    the trb status. :)

Best Regards,
Du, Changbin

[toc] | [prev] | [next] | [standalone]

#1378720 — RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space

[Multipart message — attachments visible in raw view] — view raw

Hi,

"Du, Changbin" <changbin.du@intel.com> writes:
>> > At last, comparing with the FIFO/Queue info, I think software transfer
>> > Requests list, TRBs info, EVENTs history are much more useful for
>> debugging
>> > the driver. If you can also add these info to each EP folder, that is awesome!
>> > :)
>> 
>> I'll think about adding these but for the lifetime of requests and trbs
>> and events, etc, we have tracepoints for that. I usually do the
>> following when debugging:
>> 
>> # mount -t debugfs none /sys/kernel/debug
>> # cd /sys/kernel/debug/tracing
>> # echo 2048 > buffer_size_kb
>> # echo 1 > events/dwc3/enable
>> 
>> (do something to break it)
>> 
>> # cp trace /mnt/sdcard # or something like that
>> 
>> then read the file. You can make it as large or as small as you like
>> (given some constraints, of course ;-) but I've had no issues allocating
>> 128MiB in the past.
>> 
>> --
>> Balbi
>
> Thanks for the sharing, this is a good approach to capture dynamic
> behaviors. But a dump of current state has below advantages:
> 1. a quick view for the pending transfers. Then we can quickly 
>      checking the transfer status.
> 2. no side-effect. This is important in some case. We usually
>     encounter some transfer issues but very hard to reproduce
>     it. But we cannot enable trace all the time since performance
>     concern. Then I thought it was so great if I can have a look for
>     the trb status. :)

yeah, okay. We can definitely add "current state" of almost anything,
but if you need history, then debugfs is not the best interface and I'd
point you to tracepoints ;-)

I'll think about how I can add TRB state, seems like we'd need to dump
the entire endpoint ring, and that's 256 TRBs per endpoint :-p Then we
also need to know endpoint's dequeue and enqueue pointer. Oh well, let
me get this first setup of files out of the way, then we can add more
later much more easily.

-- 
balbi

[toc] | [prev] | [next] | [standalone]

#1378723 — RE: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space

> Hi,
> 
> "Du, Changbin" <changbin.du@intel.com> writes:
> >> > At last, comparing with the FIFO/Queue info, I think software transfer
> >> > Requests list, TRBs info, EVENTs history are much more useful for
> >> debugging
> >> > the driver. If you can also add these info to each EP folder, that is
> awesome!
> >> > :)
> >>
> >> I'll think about adding these but for the lifetime of requests and trbs
> >> and events, etc, we have tracepoints for that. I usually do the
> >> following when debugging:
> >>
> >> # mount -t debugfs none /sys/kernel/debug
> >> # cd /sys/kernel/debug/tracing
> >> # echo 2048 > buffer_size_kb
> >> # echo 1 > events/dwc3/enable
> >>
> >> (do something to break it)
> >>
> >> # cp trace /mnt/sdcard # or something like that
> >>
> >> then read the file. You can make it as large or as small as you like
> >> (given some constraints, of course ;-) but I've had no issues allocating
> >> 128MiB in the past.
> >>
> >> --
> >> Balbi
> >
> > Thanks for the sharing, this is a good approach to capture dynamic
> > behaviors. But a dump of current state has below advantages:
> > 1. a quick view for the pending transfers. Then we can quickly
> >      checking the transfer status.
> > 2. no side-effect. This is important in some case. We usually
> >     encounter some transfer issues but very hard to reproduce
> >     it. But we cannot enable trace all the time since performance
> >     concern. Then I thought it was so great if I can have a look for
> >     the trb status. :)
> 
> yeah, okay. We can definitely add "current state" of almost anything,
> but if you need history, then debugfs is not the best interface and I'd
> point you to tracepoints ;-)
> 
> I'll think about how I can add TRB state, seems like we'd need to dump
> the entire endpoint ring, and that's 256 TRBs per endpoint :-p Then we
> also need to know endpoint's dequeue and enqueue pointer. Oh well, let
> me get this first setup of files out of the way, then we can add more
> later much more easily.
> 
> --
> Balbi

Okay, things need finish step by step. Thank you, Balbi. ( ゜- ゜)つロ

Best Regards,
Du, Changbin

[toc] | [prev] | [next] | [standalone]

#1376718 — [PATCH v4 1/2] usb: dwc3: make dwc3_debugfs_init return value be void

From: "Du, Changbin" <changbin.du@intel.com>

Debugfs init failure is not so important. We can continue our job on
this failure. Also no break need for debugfs_create_file call failure.

Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
v4:
  Do not fail silently, but print error.

---
 drivers/usb/dwc3/core.c    | 10 +--------
 drivers/usb/dwc3/debug.h   |  6 ++---
 drivers/usb/dwc3/debugfs.c | 56 +++++++++++++++++-----------------------------
 3 files changed, 24 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 17fd814..30f825c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1062,19 +1062,11 @@ static int dwc3_probe(struct platform_device *pdev)
 	if (ret)
 		goto err5;
 
-	ret = dwc3_debugfs_init(dwc);
-	if (ret) {
-		dev_err(dev, "failed to initialize debugfs\n");
-		goto err6;
-	}
-
+	dwc3_debugfs_init(dwc);
 	pm_runtime_allow(dev);
 
 	return 0;
 
-err6:
-	dwc3_core_exit_mode(dwc);
-
 err5:
 	dwc3_event_buffers_cleanup(dwc);
 
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 07fbc2d..71e3180 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -217,11 +217,11 @@ static inline const char *dwc3_gadget_event_type_string(u8 event)
 void dwc3_trace(void (*trace)(struct va_format *), const char *fmt, ...);
 
 #ifdef CONFIG_DEBUG_FS
-extern int dwc3_debugfs_init(struct dwc3 *);
+extern void dwc3_debugfs_init(struct dwc3 *);
 extern void dwc3_debugfs_exit(struct dwc3 *);
 #else
-static inline int dwc3_debugfs_init(struct dwc3 *d)
-{  return 0;  }
+static inline void dwc3_debugfs_init(struct dwc3 *d)
+{  }
 static inline void dwc3_debugfs_exit(struct dwc3 *d)
 {  }
 #endif
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe..615d4dc 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -618,24 +618,24 @@ static const struct file_operations dwc3_link_state_fops = {
 	.release		= single_release,
 };
 
-int dwc3_debugfs_init(struct dwc3 *dwc)
+void dwc3_debugfs_init(struct dwc3 *dwc)
 {
 	struct dentry		*root;
-	struct dentry		*file;
-	int			ret;
+	struct dentry           *file;
 
 	root = debugfs_create_dir(dev_name(dwc->dev), NULL);
-	if (!root) {
-		ret = -ENOMEM;
-		goto err0;
+	if (IS_ERR_OR_NULL(root)) {
+		if (!root)
+			dev_err(dwc->dev, "Can't create debugfs root\n");
+		return;
 	}
-
 	dwc->root = root;
 
 	dwc->regset = kzalloc(sizeof(*dwc->regset), GFP_KERNEL);
 	if (!dwc->regset) {
-		ret = -ENOMEM;
-		goto err1;
+		dev_err(dwc->dev, "Failed to alloc regset\n");
+		debugfs_remove_recursive(root);
+		return;
 	}
 
 	dwc->regset->regs = dwc3_regs;
@@ -643,44 +643,28 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs;
 
 	file = debugfs_create_regset32("regdump", S_IRUGO, root, dwc->regset);
-	if (!file) {
-		ret = -ENOMEM;
-		goto err1;
-	}
+	if (!file)
+		dev_err(dwc->dev, "Can't create debugfs regdump\n");
 
 	if (IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)) {
 		file = debugfs_create_file("mode", S_IRUGO | S_IWUSR, root,
 				dwc, &dwc3_mode_fops);
-		if (!file) {
-			ret = -ENOMEM;
-			goto err1;
-		}
+		if (!file)
+			dev_err(dwc->dev, "Can't create debugfs mode\n");
 	}
 
 	if (IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE) ||
 			IS_ENABLED(CONFIG_USB_DWC3_GADGET)) {
 		file = debugfs_create_file("testmode", S_IRUGO | S_IWUSR, root,
 				dwc, &dwc3_testmode_fops);
-		if (!file) {
-			ret = -ENOMEM;
-			goto err1;
-		}
-
-		file = debugfs_create_file("link_state", S_IRUGO | S_IWUSR, root,
-				dwc, &dwc3_link_state_fops);
-		if (!file) {
-			ret = -ENOMEM;
-			goto err1;
-		}
-	}
+		if (!file)
+			dev_err(dwc->dev, "Can't create debugfs testmode\n");
 
-	return 0;
-
-err1:
-	debugfs_remove_recursive(root);
-
-err0:
-	return ret;
+		file = debugfs_create_file("link_state", S_IRUGO | S_IWUSR,
+				root, dwc, &dwc3_link_state_fops);
+		if (!file)
+			dev_err(dwc->dev, "Can't create debugfs link_state\n");
+	}
 }
 
 void dwc3_debugfs_exit(struct dwc3 *dwc)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1374113 — [PATCH v3 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space

From: "Du, Changbin" <changbin.du@intel.com>

For DWC3 USB controller, the Global Debug Queue/FIFO Space Available
Register(GDBGFIFOSPACE) can be used to dump FIFO/Queue available space.
This can be used to check some special issues, like whether data is
successfully copied from memory to fifo when a trb is blocked.

Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
changes from v2:
  no changes

---
 drivers/usb/dwc3/core.h    |  5 +++++
 drivers/usb/dwc3/debugfs.c | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6254b2f..899cf76 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -348,6 +348,11 @@
 #define DWC3_DSTS_LOWSPEED		(2 << 0)
 #define DWC3_DSTS_FULLSPEED1		(3 << 0)
 
+/* Global Debug Queue/FIFO Space Available Register */
+#define DWC3_GDBGFIFOSPACE_NUM(x)	(((x) << 0) & 0x1F)
+#define DWC3_GDBGFIFOSPACE_TYPE(x)	(((x) << 5) & 0xE0)
+#define DWC3_GDBGFIFOSPACE_GET_SPACE(x)	(((x) >> 16) & 0xFFFF)
+
 /* Device Generic Command Register */
 #define DWC3_DGCMD_SET_LMP		0x01
 #define DWC3_DGCMD_SET_PERIODIC_PAR	0x02
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 071b286..02eaf54 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -426,6 +426,45 @@ static const struct file_operations dwc3_mode_fops = {
 	.release		= single_release,
 };
 
+static int dwc3_fifo_show(struct seq_file *s, void *unused)
+{
+	struct dwc3		*dwc = s->private;
+	unsigned long		flags;
+	unsigned int		type, index;
+	const char		*name;
+	u32			reg;
+
+	static const char * const fifo_names[] = {
+		"TxFIFO", "RxFIFO", "TxReqQ", "RxReqQ", "RxInfoQ",
+		"DescFetchQ", "EventQ", "ProtocolStatusQ"};
+	spin_lock_irqsave(&dwc->lock, flags);
+	for (type = 0; type < 8; type++) {
+		name = fifo_names[type];
+		for (index = 0; index < 32; index++) {
+			dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE,
+				DWC3_GDBGFIFOSPACE_NUM(index) |
+				DWC3_GDBGFIFOSPACE_TYPE(type));
+			reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE);
+			seq_printf(s, "%s%02d = %d\n", name, index,
+				DWC3_GDBGFIFOSPACE_GET_SPACE(reg));
+		}
+	}
+	spin_unlock_irqrestore(&dwc->lock, flags);
+	return 0;
+}
+
+static int dwc3_fifo_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, dwc3_fifo_show, inode->i_private);
+}
+
+static const struct file_operations dwc3_fifo_fops = {
+	.open			= dwc3_fifo_open,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
 static int dwc3_testmode_show(struct seq_file *s, void *unused)
 {
 	struct dwc3		*dwc = s->private;
@@ -639,6 +678,7 @@ void dwc3_debugfs_init(struct dwc3 *dwc)
 	dwc->regset->base = dwc->regs;
 
 	debugfs_create_regset32("regdump", S_IRUGO, root, dwc->regset);
+	debugfs_create_file("fifo", S_IRUGO, root, dwc, &dwc3_fifo_fops);
 
 	if (IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE))
 		debugfs_create_file("mode", S_IRUGO | S_IWUSR, root,
-- 
2.5.0

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web