Hi Felipe,
On 11/5/2018 11:35 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <thinh.nguyen@xxxxxxxxxxxx> writes:
>>>> +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.
> what happens if this gets preempted when you release the lock and
> a different thread writes to internal_states and reads the result before
> $this thread?
I see.
>
>>>> +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).
> right, turn each of those into a directory of its own. If you don't want
> to or can't disclose proper names for those directories, just call them
> lsp0, lsp1, lsp2, and so on. Then a read of the file under lsp1
> directory would write and read the correct registers.
>
> Everything remains read-only.
This means that there will be 16384 + 256 files create for host. It also
means that we need to kmalloc at least (16384 + 256) * (size of each
selection) so that each file knows what to print. On top of that, when
we change mode of operation (e.g. device->host), then we need to
create/destroy all these files. Is this the way to do it?
Thanks,
Thinh
