Hi,
On Tue, Feb 19, 2013 at 06:50:05PM -0800, Paul Zimmerman wrote:
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/spinlock.h>
> +#include <linux/interrupt.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/ch11.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/usb/ch9.h>
the usual set of unused headers ;-)
> +#include "core.h"
> +#include "hcd.h"
> +
> +#ifdef DWC2_TRACK_MISSED_SOFS
> +#warning Compiling code to track missed SOFs
> +#define FRAME_NUM_ARRAY_SIZE 1000
> +
> +/* This function is for debug only */
> +static void dwc2_track_missed_sofs(struct dwc2_hsotg *hsotg)
> +{
> + static u16 frame_num_array[FRAME_NUM_ARRAY_SIZE];
> + static u16 last_frame_num_array[FRAME_NUM_ARRAY_SIZE];
you could turn this into a list_head and everytime you find a missed
sof, you allocate a e.g. struct dwc2_missec_isoc_data and add that to a
list of missed isocs. The only "problem" is that you would have to
allocate with GFP_ATOMIC.
> + static int frame_num_idx;
> + static u16 last_frame_num = HFNUM_MAX_FRNUM;
> + static int dumped_frame_num_array;
> + u16 curr_frame_number = hsotg->frame_number;
> +
> + if (frame_num_idx < FRAME_NUM_ARRAY_SIZE) {
> + if ((last_frame_num + 1 & HFNUM_MAX_FRNUM) !=
> + curr_frame_number) {
> + frame_num_array[frame_num_idx] = curr_frame_number;
> + last_frame_num_array[frame_num_idx++] = last_frame_num;
> + }
> + } else if (!dumped_frame_num_array) {
> + int i;
> +
> + dev_info(hsotg->dev, "Frame Last Frame\n");
> + dev_info(hsotg->dev, "----- ----------\n");
> + for (i = 0; i < FRAME_NUM_ARRAY_SIZE; i++) {
> + dev_info(hsotg->dev, "0x%04x 0x%04x\n",
> + frame_num_array[i], last_frame_num_array[i]);
> + }
> + dumped_frame_num_array = 1;
> + }
> + last_frame_num = curr_frame_number;
> +}
> +#endif
do you really need to dump this information at the time it happens ? I
ask this because this looks like a very nice feature to have in
production. You could just pack it under a debugfs interface and provide
a Kconfig choice to enable (or not) debugfs support. Then driver can
constantly track this information and let user see by "cat
/sys/kernel/debug/dwc2/missed_sofs" or something similar.
> +static void dwc2_sof_intr(struct dwc2_hsotg *hsotg)
> +{
> + struct list_head *qh_entry;
> + struct dwc2_qh *qh;
> + u32 hfnum;
> + enum dwc2_transaction_type tr_type;
> +
> +#ifdef DEBUG_SOF
> + dev_vdbg(hsotg->dev, "--Start of Frame Interrupt--\n");
> +#endif
> +
> + hfnum = readl(hsotg->regs + HFNUM);
> + hsotg->frame_number = hfnum >> HFNUM_FRNUM_SHIFT &
> + HFNUM_FRNUM_MASK >> HFNUM_FRNUM_SHIFT;
> +
> +#ifdef DWC2_TRACK_MISSED_SOFS
> + dwc2_track_missed_sofs(hsotg);
> +#endif
another suggestion, would be to move the ifdefs inside the function
body, so that it's a no-op when DWC2_TRACK_MISSED_SOFS isn't set, which
means you don't need to ifdef the call here.
> +static void dwc2_hc_chhltd_intr_dma(struct dwc2_hsotg *hsotg,
> + struct dwc2_host_chan *chan, int chnum,
> + struct dwc2_qtd *qtd)
> +{
> + u32 hcintmsk;
> + int out_nak_enh = 0;
> +
> + dev_vdbg(hsotg->dev,
> + "--Host Channel %d Interrupt: DMA Channel Halted--\n", chnum);
> +
> + /*
> + * For core with OUT NAK enhancement, the flow for high-speed
> + * CONTROL/BULK OUT is handled a little differently
> + */
> + if (hsotg->snpsid >= DWC2_CORE_REV_2_71a) {
> + if (chan->speed == USB_SPEED_HIGH && !chan->ep_is_in &&
> + (chan->ep_type == USB_ENDPOINT_XFER_CONTROL ||
> + chan->ep_type == USB_ENDPOINT_XFER_BULK)) {
> + out_nak_enh = 1;
> + }
> + }
> +
> + if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE ||
> + (chan->halt_status == DWC2_HC_XFER_AHB_ERR &&
> + hsotg->core_params->dma_desc_enable <= 0)) {
> + if (hsotg->core_params->dma_desc_enable > 0)
> + dwc2_hcd_complete_xfer_ddma(hsotg, chan, chnum,
> + chan->halt_status);
> + else
> + /*
> + * Just release the channel. A dequeue can happen on a
> + * transfer timeout. In the case of an AHB Error, the
> + * channel was forced to halt because there's no way to
> + * gracefully recover.
> + */
> + dwc2_release_channel(hsotg, chan, qtd,
> + chan->halt_status);
> + return;
> + }
> +
> + hcintmsk = readl(hsotg->regs + HCINTMSK(chnum));
> +
> + if (chan->hcint & HCINTMSK_XFERCOMPL) {
> + /*
> + * Todo: This is here because of a possible hardware bug. Spec
> + * says that on SPLIT-ISOC OUT transfers in DMA mode that a HALT
> + * interrupt w/ACK bit set should occur, but I only see the
> + * XFERCOMP bit, even with it masked out. This is a workaround
> + * for that behavior. Should fix this when hardware is fixed.
> + */
> + if (chan->ep_type == USB_ENDPOINT_XFER_ISOC && !chan->ep_is_in)
> + dwc2_hc_ack_intr(hsotg, chan, chnum, qtd);
> + dwc2_hc_xfercomp_intr(hsotg, chan, chnum, qtd);
> + } else if (chan->hcint & HCINTMSK_STALL) {
> + dwc2_hc_stall_intr(hsotg, chan, chnum, qtd);
> + } else if ((chan->hcint & HCINTMSK_XACTERR) &&
> + hsotg->core_params->dma_desc_enable <= 0) {
> + if (out_nak_enh) {
> + if (chan->hcint &
> + (HCINTMSK_NYET | HCINTMSK_NAK | HCINTMSK_ACK)) {
> + dev_vdbg(hsotg->dev,
> + "XactErr with NYET/NAK/ACK\n");
> + qtd->error_count = 0;
> + } else {
> + dev_vdbg(hsotg->dev,
> + "XactErr without NYET/NAK/ACK\n");
> + }
> + }
> +
> + /*
> + * Must handle xacterr before nak or ack. Could get a xacterr
> + * at the same time as either of these on a BULK/CONTROL OUT
> + * that started with a PING. The xacterr takes precedence.
> + */
> + dwc2_hc_xacterr_intr(hsotg, chan, chnum, qtd);
> + } else if ((chan->hcint & HCINTMSK_XCS_XACT) &&
> + hsotg->core_params->dma_desc_enable > 0) {
are all these really mutually exclusive ? I mean, can you get XACTERR
and AHBERR simultaneously ?
--
balbi
Attachment:
signature.asc
Description: Digital signature
