Hi,

Thinh Nguyen <thinh.ngu...@synopsys.com> writes:
> +static void dwc3_dump_gadget_internal_states(struct seq_file *s)
> +{
> +     struct dwc3             *dwc = s->private;
> +     int                     num_selects = 16;
> +     int                     i;
> +     u32                     reg;
> +     u64                     ep_info;
> +
> +     for (i = 0; i < num_selects; i++) {
> +             reg = dwc3_gadget_lsp_register(dwc, i);
> +             seq_printf(s, "GDBGLSP[%d] = 0x%08x\n", i, reg);
> +     }
> +
> +     for (i = 0; i < dwc->num_eps; i++) {
> +             ep_info = dwc3_ep_info_register(dwc, i);
> +             seq_printf(s, "GDBGEPINFO[%d] = 0x%016llx\n", i, ep_info);
> +     }
> +}

we have per-endpoint directories already. Why don't you dump endpoint
debug info there? Also, while at that, could you write a patch that
properly decodes the queue sizes? It looks to me as the queue sizes are
in same units as GTXFIFOSIZ registers

> +static int dwc3_internal_states_show(struct seq_file *s, void *unused)
> +{
> +     struct dwc3             *dwc = s->private;
> +     unsigned int            current_mode;
> +     unsigned long           flags;
> +     u32                     reg;
> +
> +     spin_lock_irqsave(&dwc->lock, flags);
> +     reg = dwc3_readl(dwc->regs, DWC3_GSTS);
> +     current_mode = DWC3_GSTS_CURMOD(reg);
> +
> +     reg = dwc3_readl(dwc->regs, DWC3_GDBGBMU);
> +     spin_unlock_irqrestore(&dwc->lock, flags);
> +
> +     seq_printf(s, "GDBGBMU = 0x%08x\n", reg);

shouldn't the print be done with locks held?

> +static ssize_t dwc3_internal_states_write(struct file *file,
> +             const char __user *ubuf, size_t count, loff_t *ppos)

why is this necessary? Seems like it would be nicer to create a
directory structure if the current operating mode is host so that we
don't need to write anything.

-- 
balbi

Attachment: signature.asc
Description: PGP signature

Reply via email to