From: Felipe Balbi <balbi@kernel.org>
To: changbin.du@intel.com
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, "Du\,
Changbin" <changbin.du@intel.com>
Subject: Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space
Date: Thu, 14 Apr 2016 11:02:40 +0300 [thread overview]
Message-ID: <87shyok47z.fsf@intel.com> (raw)
In-Reply-To: <1460459419-9487-3-git-send-email-changbin.du@intel.com>
[-- Attachment #1: Type: text/plain, Size: 10752 bytes --]
Hi,
changbin.du@intel.com writes:
> 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>
> ---
> v4:
> Do not fail silently, but print error.
>
> ---
> drivers/usb/dwc3/core.h | 5 +++++
> drivers/usb/dwc3/debugfs.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 49 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)
we always use lower case hex numbers. Also, databook refers to top 16
bits as "Space Avaiable" so I'd prefer that you called this macro:
DWC3_GDBGFIFOSPACE_SPACE_AVAILABLE(x)
as that will aid with grepping
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 615d4dc..83e5bc1 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)
you call this file 'fifo' however you print more than just the
fifos. You printk the request queues, info queue, descriptor fetch
queue, event queue and protocol status queue.
It seems, to me, you're trying to do way too much in a single file.
> +{
> + 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++) {
type < ARRAY_SIZE(fifo_names) ??
> + name = fifo_names[type];
> + for (index = 0; index < 32; index++) {
not *all* dwc3 implementations enable all 32 endpoints, that's why we
have dwc->num_endpoints
> + dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE,
> + DWC3_GDBGFIFOSPACE_NUM(index) |
> + DWC3_GDBGFIFOSPACE_TYPE(type));
> + reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE);
this writel() followed by a readl() could be a nice little helper
function in core.c. Remember that we need the FIFO Space and link state
to make sure we're allowed to start a transfer ;-)
I'd suggesting adding the following to core.c:
int dwc3_core_fifo_space(struct dwc3_ep *dep, unsigned int type)
{
u32 reg;
dwc3_writel(dwc->regs, DWC3_GDBGFIFOSPACE,
DWC3_GDBGFIFOSPACE_NUM(dep->number) |
DWC3_GDBGFIFOSPACE_TYPE(type));
reg = dwc3_readl(dwc->regs, DWC3_GDBGFIFOSPACE);
return DWC3_GDBGFIFOSPACE_SPACE_AVAILABLE(reg);
}
then, add the prototype to core.h. (both of these in a separate patch).
Then $subject just uses that helper to print out the data you want to
print. I still think we might be better off with one file per type (iow
TX fifo file, RX fifo file, TX request queue file, and so on).
In fact, we could actually have a per-endpoint directory where all of
these details are printed out in a much more structured form. If you
want, I can work on that, no problem, but you're also welcome to
implement it yourself and I'd be around to help as needed.
In summary, we should have our debugfs looking like so:
./ep0
./ep0/descriptor_fetch_queue
./ep0/event_queue
./ep0/info_queue
./ep0/protocol_status_queue
./ep0/request_queue
./ep0/rx_fifo
./ep0/rx_request_queue
./ep0/tx_fifo
./ep0/tx_request_queue
./ep1
./ep10
./ep10/descriptor_fetch_queue
./ep10/event_queue
./ep10/info_queue
./ep10/protocol_status_queue
./ep10/request_queue
./ep10/rx_fifo
./ep10/rx_request_queue
./ep10/tx_fifo
./ep10/tx_request_queue
./ep11
./ep11/descriptor_fetch_queue
./ep11/event_queue
./ep11/info_queue
./ep11/protocol_status_queue
./ep11/request_queue
./ep11/rx_fifo
./ep11/rx_request_queue
./ep11/tx_fifo
./ep11/tx_request_queue
./ep12
./ep12/descriptor_fetch_queue
./ep12/event_queue
./ep12/info_queue
./ep12/protocol_status_queue
./ep12/request_queue
./ep12/rx_fifo
./ep12/rx_request_queue
./ep12/tx_fifo
./ep12/tx_request_queue
./ep13
./ep13/descriptor_fetch_queue
./ep13/event_queue
./ep13/info_queue
./ep13/protocol_status_queue
./ep13/request_queue
./ep13/rx_fifo
./ep13/rx_request_queue
./ep13/tx_fifo
./ep13/tx_request_queue
./ep14
./ep14/descriptor_fetch_queue
./ep14/event_queue
./ep14/info_queue
./ep14/protocol_status_queue
./ep14/request_queue
./ep14/rx_fifo
./ep14/rx_request_queue
./ep14/tx_fifo
./ep14/tx_request_queue
./ep15
./ep15/descriptor_fetch_queue
./ep15/event_queue
./ep15/info_queue
./ep15/protocol_status_queue
./ep15/request_queue
./ep15/rx_fifo
./ep15/rx_request_queue
./ep15/tx_fifo
./ep15/tx_request_queue
./ep16
./ep16/descriptor_fetch_queue
./ep16/event_queue
./ep16/info_queue
./ep16/protocol_status_queue
./ep16/request_queue
./ep16/rx_fifo
./ep16/rx_request_queue
./ep16/tx_fifo
./ep16/tx_request_queue
./ep17
./ep17/descriptor_fetch_queue
./ep17/event_queue
./ep17/info_queue
./ep17/protocol_status_queue
./ep17/request_queue
./ep17/rx_fifo
./ep17/rx_request_queue
./ep17/tx_fifo
./ep17/tx_request_queue
./ep18
./ep18/descriptor_fetch_queue
./ep18/event_queue
./ep18/info_queue
./ep18/protocol_status_queue
./ep18/request_queue
./ep18/rx_fifo
./ep18/rx_request_queue
./ep18/tx_fifo
./ep18/tx_request_queue
./ep19
./ep19/descriptor_fetch_queue
./ep19/event_queue
./ep19/info_queue
./ep19/protocol_status_queue
./ep19/request_queue
./ep19/rx_fifo
./ep19/rx_request_queue
./ep19/tx_fifo
./ep19/tx_request_queue
./ep1/descriptor_fetch_queue
./ep1/event_queue
./ep1/info_queue
./ep1/protocol_status_queue
./ep1/request_queue
./ep1/rx_fifo
./ep1/rx_request_queue
./ep1/tx_fifo
./ep1/tx_request_queue
./ep2
./ep20
./ep20/descriptor_fetch_queue
./ep20/event_queue
./ep20/info_queue
./ep20/protocol_status_queue
./ep20/request_queue
./ep20/rx_fifo
./ep20/rx_request_queue
./ep20/tx_fifo
./ep20/tx_request_queue
./ep21
./ep21/descriptor_fetch_queue
./ep21/event_queue
./ep21/info_queue
./ep21/protocol_status_queue
./ep21/request_queue
./ep21/rx_fifo
./ep21/rx_request_queue
./ep21/tx_fifo
./ep21/tx_request_queue
./ep22
./ep22/descriptor_fetch_queue
./ep22/event_queue
./ep22/info_queue
./ep22/protocol_status_queue
./ep22/request_queue
./ep22/rx_fifo
./ep22/rx_request_queue
./ep22/tx_fifo
./ep22/tx_request_queue
./ep23
./ep23/descriptor_fetch_queue
./ep23/event_queue
./ep23/info_queue
./ep23/protocol_status_queue
./ep23/request_queue
./ep23/rx_fifo
./ep23/rx_request_queue
./ep23/tx_fifo
./ep23/tx_request_queue
./ep24
./ep24/descriptor_fetch_queue
./ep24/event_queue
./ep24/info_queue
./ep24/protocol_status_queue
./ep24/request_queue
./ep24/rx_fifo
./ep24/rx_request_queue
./ep24/tx_fifo
./ep24/tx_request_queue
./ep25
./ep25/descriptor_fetch_queue
./ep25/event_queue
./ep25/info_queue
./ep25/protocol_status_queue
./ep25/request_queue
./ep25/rx_fifo
./ep25/rx_request_queue
./ep25/tx_fifo
./ep25/tx_request_queue
./ep26
./ep26/descriptor_fetch_queue
./ep26/event_queue
./ep26/info_queue
./ep26/protocol_status_queue
./ep26/request_queue
./ep26/rx_fifo
./ep26/rx_request_queue
./ep26/tx_fifo
./ep26/tx_request_queue
./ep27
./ep27/descriptor_fetch_queue
./ep27/event_queue
./ep27/info_queue
./ep27/protocol_status_queue
./ep27/request_queue
./ep27/rx_fifo
./ep27/rx_request_queue
./ep27/tx_fifo
./ep27/tx_request_queue
./ep28
./ep28/descriptor_fetch_queue
./ep28/event_queue
./ep28/info_queue
./ep28/protocol_status_queue
./ep28/request_queue
./ep28/rx_fifo
./ep28/rx_request_queue
./ep28/tx_fifo
./ep28/tx_request_queue
./ep29
./ep29/descriptor_fetch_queue
./ep29/event_queue
./ep29/info_queue
./ep29/protocol_status_queue
./ep29/request_queue
./ep29/rx_fifo
./ep29/rx_request_queue
./ep29/tx_fifo
./ep29/tx_request_queue
./ep2/descriptor_fetch_queue
./ep2/event_queue
./ep2/info_queue
./ep2/protocol_status_queue
./ep2/request_queue
./ep2/rx_fifo
./ep2/rx_request_queue
./ep2/tx_fifo
./ep2/tx_request_queue
./ep3
./ep30
./ep30/descriptor_fetch_queue
./ep30/event_queue
./ep30/info_queue
./ep30/protocol_status_queue
./ep30/request_queue
./ep30/rx_fifo
./ep30/rx_request_queue
./ep30/tx_fifo
./ep30/tx_request_queue
./ep31
./ep31/descriptor_fetch_queue
./ep31/event_queue
./ep31/info_queue
./ep31/protocol_status_queue
./ep31/request_queue
./ep31/rx_fifo
./ep31/rx_request_queue
./ep31/tx_fifo
./ep31/tx_request_queue
./ep3/descriptor_fetch_queue
./ep3/event_queue
./ep3/info_queue
./ep3/protocol_status_queue
./ep3/request_queue
./ep3/rx_fifo
./ep3/rx_request_queue
./ep3/tx_fifo
./ep3/tx_request_queue
./ep4
./ep4/descriptor_fetch_queue
./ep4/event_queue
./ep4/info_queue
./ep4/protocol_status_queue
./ep4/request_queue
./ep4/rx_fifo
./ep4/rx_request_queue
./ep4/tx_fifo
./ep4/tx_request_queue
./ep5
./ep5/descriptor_fetch_queue
./ep5/event_queue
./ep5/info_queue
./ep5/protocol_status_queue
./ep5/request_queue
./ep5/rx_fifo
./ep5/rx_request_queue
./ep5/tx_fifo
./ep5/tx_request_queue
./ep6
./ep6/descriptor_fetch_queue
./ep6/event_queue
./ep6/info_queue
./ep6/protocol_status_queue
./ep6/request_queue
./ep6/rx_fifo
./ep6/rx_request_queue
./ep6/tx_fifo
./ep6/tx_request_queue
./ep7
./ep7/descriptor_fetch_queue
./ep7/event_queue
./ep7/info_queue
./ep7/protocol_status_queue
./ep7/request_queue
./ep7/rx_fifo
./ep7/rx_request_queue
./ep7/tx_fifo
./ep7/tx_request_queue
./ep8
./ep8/descriptor_fetch_queue
./ep8/event_queue
./ep8/info_queue
./ep8/protocol_status_queue
./ep8/request_queue
./ep8/rx_fifo
./ep8/rx_request_queue
./ep8/tx_fifo
./ep8/tx_request_queue
./ep9
./ep9/descriptor_fetch_queue
./ep9/event_queue
./ep9/info_queue
./ep9/protocol_status_queue
./ep9/request_queue
./ep9/rx_fifo
./ep9/rx_request_queue
./ep9/tx_fifo
./ep9/tx_request_queue
./link_state
./mode
./regdump
./testmode
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
next prev parent reply other threads:[~2016-04-14 8:05 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-06 8:27 [PATCH] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du
2016-04-06 9:25 ` Greg KH
2016-04-06 11:38 ` Du, Changbin
2016-04-06 12:24 ` Felipe Balbi
2016-04-06 15:44 ` [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs changbin.du
2016-04-07 5:05 ` Felipe Balbi
2016-04-07 5:21 ` Du, Changbin
2016-04-07 5:22 ` Felipe Balbi
2016-04-08 9:34 ` [PATCH v3 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du
2016-04-08 9:34 ` [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du
2016-04-11 8:14 ` Felipe Balbi
2016-04-11 11:19 ` Du, Changbin
2016-04-11 11:23 ` Felipe Balbi
2016-04-12 11:10 ` [PATCH v4 0/2] Add a new debugfs entry to dump FIFO/Queue available space changbin.du
2016-04-12 11:10 ` [PATCH v4 1/2] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du
2016-04-12 11:10 ` [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du
2016-04-12 12:58 ` Sergei Shtylyov
2016-04-14 3:27 ` Du, Changbin
2016-04-14 8:02 ` Felipe Balbi [this message]
2016-04-14 11:15 ` Du, Changbin
2016-04-14 11:18 ` Felipe Balbi
2016-04-14 11:37 ` Du, Changbin
2016-04-14 11:41 ` Felipe Balbi
2016-04-14 11:58 ` Du, Changbin
2016-04-08 9:34 ` [PATCH v3 " changbin.du
2016-04-06 15:44 ` [PATCH v2 1/3] usb: dwc3: make dwc3_debugfs_init return value be void changbin.du
2016-04-06 15:44 ` [PATCH v2 2/3] usb: dwc3: free dwc->regset on dwc3_debugfs_exit changbin.du
2016-04-06 21:08 ` Greg KH
2016-04-07 5:05 ` Felipe Balbi
2016-04-06 15:44 ` [PATCH v2 3/3] usb: dwc3: add debugfs node to dump FIFO/Queue available space changbin.du
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=87shyok47z.fsf@intel.com \
--to=balbi@kernel.org \
--cc=changbin.du@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.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 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).