Hi Felipe,
On 11/5/2018 4:16 AM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx> 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?
Yes, I can dump it 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
Sure. It can be done.
>> +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?
I think the lock for the prints should be held at a higher level.
Otherwise, I don't think it will make a difference using dwc->lock.
>> +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.
Can you clarify more about the directory structure? Actually, I was
wondering what's the best way to do this also. The reason I want to
write to it is because the LSP selection for host is quite large (2^14).
It doesn't make sense to dump everything if the controller is in host
mode. So I force the user to write a specific LSP selection to dump at a
time.
Thanks for the review!
Thinh
