>Hi,
>
>On 03/11/18 19:51, Pawel Laszczak wrote:
>> Patch implements some function used for debugging driver.
>>
>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>> ---
>> drivers/usb/cdns3/Makefile | 2 +-
>> drivers/usb/cdns3/debug.c | 128 +++++++++++++++++++++++++++++++++++++
>> drivers/usb/cdns3/ep0.c | 3 +
>> drivers/usb/cdns3/gadget.c | 12 ++++
>> drivers/usb/cdns3/gadget.h | 13 +++-
>> 5 files changed, 156 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/usb/cdns3/debug.c
>>
>> diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>> index f4cfc978626f..34e60d03c4ec 100644
>> --- a/drivers/usb/cdns3/Makefile
>> +++ b/drivers/usb/cdns3/Makefile
>> @@ -2,6 +2,6 @@ obj-$(CONFIG_USB_CDNS3) += cdns3.o
>> obj-$(CONFIG_USB_CDNS3_PCI_WRAP) += cdns3-pci.o
>>
>> cdns3-y := core.o drd.o
>> -cdns3-$(CONFIG_USB_CDNS3_GADGET) += gadget.o ep0.o
>> +cdns3-$(CONFIG_USB_CDNS3_GADGET) += gadget.o ep0.o debug.o
>> cdns3-$(CONFIG_USB_CDNS3_HOST) += host.o
>> cdns3-pci-y := cdns3-pci-wrap.o
>> diff --git a/drivers/usb/cdns3/debug.c b/drivers/usb/cdns3/debug.c
>> new file mode 100644
>> index 000000000000..bebf22c4d18e
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/debug.c
>> @@ -0,0 +1,128 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Cadence USBSS DRD Driver.
>> + *
>> + * Copyright (C) 2018 Cadence.
>> + *
>> + * Author: Pawel Laszczak <pawell@xxxxxxxxxxx>
>> + */
>> +
>> +#include "gadget.h"
>> +
>> +static inline char *cdns3_decode_ep_irq(u32 ep_sts, const char *ep_name)
>> +{
>> + static char str[256];
>> + int ret;
>> +
>> + ret = sprintf(str, "IRQ for %s: %08x ", ep_name, ep_sts);
>> +
>> + if (ep_sts & EP_STS_SETUP)
>> + ret += sprintf(str + ret, "SETUP ");
>> + if (ep_sts & EP_STS_IOC)
>> + ret += sprintf(str + ret, "IOC ");
>> + if (ep_sts & EP_STS_ISP)
>> + ret += sprintf(str + ret, "ISP ");
>> + if (ep_sts & EP_STS_DESCMIS)
>> + ret += sprintf(str + ret, "DESCMIS ");
>> + if (ep_sts & EP_STS_STREAMR)
>> + ret += sprintf(str + ret, "STREAMR ");
>> + if (ep_sts & EP_STS_MD_EXIT)
>> + ret += sprintf(str + ret, "MD_EXIT ");
>> + if (ep_sts & EP_STS_TRBERR)
>> + ret += sprintf(str + ret, "TRBERR ");
>> + if (ep_sts & EP_STS_NRDY)
>> + ret += sprintf(str + ret, "NRDY ");
>> + if (ep_sts & EP_STS_PRIME)
>> + ret += sprintf(str + ret, "PRIME ");
>> + if (ep_sts & EP_STS_SIDERR)
>> + ret += sprintf(str + ret, "SIDERRT ");
>> + if (ep_sts & EP_STS_OUTSMM)
>> + ret += sprintf(str + ret, "OUTSMM ");
>> + if (ep_sts & EP_STS_ISOERR)
>> + ret += sprintf(str + ret, "ISOERR ");
>> + if (ep_sts & EP_STS_IOT)
>> + ret += sprintf(str + ret, "IOT ");
>> +
>> + return str;
>> +}
>> +
>> +char *cdns3_decode_epx_irq(struct cdns3_endpoint *priv_ep)
>> +{
>> + struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
>> +
>> + return cdns3_decode_ep_irq(readl(&priv_dev->regs->ep_sts),
>> + priv_ep->name);
>> +}
>> +
>> +char *cdns3_decode_ep0_irq(struct cdns3_device *priv_dev, int dir)
>> +{
>> + if (dir)
>> + return cdns3_decode_ep_irq(readl(&priv_dev->regs->ep_sts),
>> + "ep0IN");
>> + else
>> + return cdns3_decode_ep_irq(readl(&priv_dev->regs->ep_sts),
>> + "ep0OUT");
>> +}
>> +
>> +void cdns3_dbg_setup(struct cdns3_device *priv_dev)
>> +{
>> + struct usb_ctrlrequest *setup = priv_dev->setup;
>> +
>> + dev_dbg(&priv_dev->dev,
>> + "SETUP BRT: %02x BR: %02x V: %04x I: %04x L: %04x\n",
>> + setup->bRequestType,
>> + setup->bRequest,
>> + le16_to_cpu(setup->wValue),
>> + le16_to_cpu(setup->wIndex),
>> + le16_to_cpu(setup->wLength));
>> +}
>> +
>> +/**
>> + * Debug a transfer ring.
>> + *
>> + * Prints out all TRBs in the endpoint ring, even those after the Link TRB.
>> + *.
>> + */
>> +void cdns3_dbg_ring(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep)
>> +{
>> + u64 addr = priv_ep->trb_pool_dma;
>> + struct cdns3_trb *trb;
>> + int i;
>> +
>> + for (i = 0; i < TRBS_PER_SEGMENT; ++i) {
>> + trb = &priv_ep->trb_pool[i];
>> + dev_dbg(&priv_dev->dev, "@%016llx %08x %08x %08x\n", addr,
>> + le32_to_cpu(trb->buffer),
>> + le32_to_cpu(trb->length),
>> + le32_to_cpu(trb->control));
>> + addr += sizeof(*trb);
>> + }
>> +}
>> +
>> +void cdns3_dbg_ring_ptrs(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep)
>> +{
>> + struct cdns3_trb *trb;
>> +
>> + trb = &priv_ep->trb_pool[priv_ep->dequeue];
>> + dev_dbg(&priv_dev->dev,
>> + "Ring deq index: %d, trb: %p (virt), 0x%llx (dma)\n",
>> + priv_ep->dequeue, trb,
>> + cdns3_trb_virt_to_dma(priv_ep, trb));
>> +
>> + trb = &priv_ep->trb_pool[priv_ep->enqueue];
>> + dev_dbg(&priv_dev->dev,
>> + "Ring enq index: %d, trb: %p (virt), 0x%llx (dma)\n",
>> + priv_ep->enqueue, trb,
>> + cdns3_trb_virt_to_dma(priv_ep, trb));
>> +}
>> +
>> +void cdns3_dbg_ep_rings(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep)
>> +{
>> + dev_dbg(&priv_dev->dev, "Endpoint ring %s:\n", priv_ep->name);
>> +
>> + cdns3_dbg_ring_ptrs(priv_dev, priv_ep);
>> + cdns3_dbg_ring(priv_dev, priv_ep);
>> +}
>> diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
>> index 555ac7f6842e..46c943a74f77 100644
>> --- a/drivers/usb/cdns3/ep0.c
>> +++ b/drivers/usb/cdns3/ep0.c
>> @@ -604,12 +604,15 @@ void cdns3_check_ep0_interrupt_proceed(struct cdns3_device *priv_dev, int dir)
>> cdns3_select_ep(priv_dev, 0 | (dir ? USB_DIR_IN : USB_DIR_OUT));
>> ep_sts_reg = readl(®s->ep_sts);
>>
>> + dev_dbg(&priv_dev->dev, "%s\n", cdns3_decode_ep0_irq(priv_dev, dir));
>> +
>> __pending_setup_status_handler(priv_dev);
>>
>> if ((ep_sts_reg & EP_STS_SETUP) && dir == 0) {
>> struct usb_ctrlrequest *setup = priv_dev->setup;
>>
>> writel(EP_STS_SETUP | EP_STS_IOC | EP_STS_ISP, ®s->ep_sts);
>> + cdns3_dbg_setup(priv_dev);
>>
>> priv_dev->ep0_data_dir = setup->bRequestType & USB_DIR_IN;
>> cdns3_ep0_setup_phase(priv_dev);
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 905cad1a8229..ba4a36ce6e9a 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -124,6 +124,14 @@ void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep)
>> wmb();
>> }
>>
>> +dma_addr_t cdns3_trb_virt_to_dma(struct cdns3_endpoint *priv_ep,
>> + struct cdns3_trb *trb)
>> +{
>> + u32 offset = (char *)trb - (char *)priv_ep->trb_pool;
>> +
>> + return priv_ep->trb_pool_dma + offset;
>> +}
>> +
>> /**
>> * cdns3_allocate_trb_pool - Allocates TRB's pool for selected endpoint
>> * @priv_ep: endpoint object
>> @@ -428,6 +436,8 @@ int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
>> trb->control |= first_pcs;
>>
>> priv_req->on_ring = 1;
>> +
>> + cdns3_dbg_ep_rings(priv_dev, priv_ep);
>> arm:
>> /* arm transfer on selected endpoint */
>> cdns3_select_ep(priv_ep->cdns3_dev, address);
>> @@ -537,6 +547,8 @@ static int cdns3_check_ep_interrupt_proceed(struct cdns3_endpoint *priv_ep)
>> cdns3_select_ep(priv_dev, priv_ep->endpoint.address);
>> ep_sts_reg = readl(®s->ep_sts);
>>
>> + dev_dbg(&priv_dev->dev, "%s\n", cdns3_decode_epx_irq(priv_ep));
>> +
>> if (ep_sts_reg & EP_STS_TRBERR)
>> writel(EP_STS_TRBERR, ®s->ep_sts);
>>
>> diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h
>> index 3f67ef1e5cda..3b2c01ff655b 100644
>> --- a/drivers/usb/cdns3/gadget.h
>> +++ b/drivers/usb/cdns3/gadget.h
>> @@ -1085,5 +1085,16 @@ struct usb_request *cdns3_gadget_ep_alloc_request(struct usb_ep *ep,
>> void cdns3_gadget_ep_free_request(struct usb_ep *ep,
>> struct usb_request *request);
>> int cdns3_gadget_ep_dequeue(struct usb_ep *ep, struct usb_request *request);
>> -
>> +dma_addr_t cdns3_trb_virt_to_dma(struct cdns3_endpoint *priv_ep,
>> + struct cdns3_trb *trb);
>> +
>> +char *cdns3_decode_epx_irq(struct cdns3_endpoint *priv_ep);
>> +char *cdns3_decode_ep0_irq(struct cdns3_device *priv_dev, int dir);
>> +void cdns3_dbg_ring(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep);
>> +void cdns3_dbg_ring_ptrs(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep);
>> +void cdns3_dbg_ep_rings(struct cdns3_device *priv_dev,
>> + struct cdns3_endpoint *priv_ep);
>> +void cdns3_dbg_setup(struct cdns3_device *priv_dev);
>> #endif /* __LINUX_CDNS3_GADGET */
>>
>
>How often are these invoked?
>
>For I/O intensive cases dev_dbg() will not be useful as it will affect timing adversely
>because of which it might prevent the issue from happening when debug is enabled.
>
>How about using tracepoints instead?
Yes, I know and I agree with you.
I have plan to replace this with tracepoints.
But I need some time for this. Currently I'm focusing on testing.
Probably I will change It by the end of this month, so it should be in
RFC PATCH v3.
>
>cheers,
>-roger
>--
>Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Thanks,
Cheers,
Pawell
