Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752698AbcDNIFK (ORCPT ); Thu, 14 Apr 2016 04:05:10 -0400 Received: from mga09.intel.com ([134.134.136.24]:9368 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752079AbcDNIFF (ORCPT ); Thu, 14 Apr 2016 04:05:05 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,484,1455004800"; d="asc'?scan'208";a="954739373" From: Felipe Balbi To: changbin.du@intel.com Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Du\, Changbin" Subject: Re: [PATCH v4 2/2] usb: dwc3: add debugfs node to dump FIFO/Queue available space In-Reply-To: <1460459419-9487-3-git-send-email-changbin.du@intel.com> References: <1460459419-9487-1-git-send-email-changbin.du@intel.com> <1460459419-9487-3-git-send-email-changbin.du@intel.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Thu, 14 Apr 2016 11:02:40 +0300 Message-ID: <87shyok47z.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11323 Lines: 470 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, changbin.du@intel.com writes: > From: "Du, Changbin" > > 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 > --- > 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) >=20=20 > +/* 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 = =3D { > .release =3D single_release, > }; >=20=20 > +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 =3D s->private; > + unsigned long flags; > + unsigned int type, index; > + const char *name; > + u32 reg; > + > + static const char * const fifo_names[] =3D { > + "TxFIFO", "RxFIFO", "TxReqQ", "RxReqQ", "RxInfoQ", > + "DescFetchQ", "EventQ", "ProtocolStatusQ"}; > + spin_lock_irqsave(&dwc->lock, flags); > + for (type =3D 0; type < 8; type++) { type < ARRAY_SIZE(fifo_names) ?? > + name =3D fifo_names[type]; > + for (index =3D 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 =3D 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 =3D 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 =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXD06gAAoJEIaOsuA1yqRE9MEQAIShHXynJboE5qbNaSj3AMxm YiEKSzz98++dZO3N923HjNBEiAHi5eTZOUKiuXFGcSckr6SRAAtrjw1eauONfq8h eW+mE+TMrqwBTcdmDAjvAH7cTs/YWc5LHQBX9M5EconjIncmtkA6WdxurObeuxdR 8hE/zuQy9K1dpNS2MrsEdQsvGmygqtRQPNu/iAPmIPa8OK/uAcrTpNlLwAYG+Kvw y3EwLypcTzk4SzgnGmSRsVsxP0r2G5TzqPJg3OPDWW3G8H14NV1eIkGy1xUzNkk6 QgftmSNBROtNe2rJLR9EsP1TTouAfDbH4TNz0qwVdFumhRymSrfYaHEL68fsFhyk 1qdCJh6lWDP7MwHTyuWot2FoLPjvIYsXfNp5qta8GeIKdPgt+icKHv23vKivudtz QWnffzYGhPI4g2utH/U89OqCXvkUp3RNUFQ5JMZVccIApRE7E7DBcsmdJPhwx5Cd WXTIVY4skMXVjJlmyI3fcQTVL9jOWyZJfOUrn7zQ4QKwBnTObbrLFAiacst7vUPL tMbCf+J66mbglCFZFDOQocf0b13IhFPvDGaofyZ1DzcKl71ueWeqzOgCewljVvDx FR2RiJJ49/UvBLimX7chIRKNQbO8Vnj7BARjQhuXKgKbCY/KQvznRqJgHaOf+AhS LsQxDIiyeZWqx2VvcXVe =wKBM -----END PGP SIGNATURE----- --=-=-=--