Re: [PATCH v4 17/24] PM / devfreq: tegra30: Use tracepoints for debugging

[Dmitry Osipenko]

Ð Fri, 19 Jul 2019 10:01:55 +0900
Chanwoo Choi <cw00.choi@xxxxxxxxxxx> ÐÐÑÐÑ:

> On 19. 7. 19. ìì 9:49, Dmitry Osipenko wrote:
> > Ð Thu, 18 Jul 2019 18:47:09 +0900
> > Chanwoo Choi <cw00.choi@xxxxxxxxxxx> ÐÐÑÐÑ:
> >   
> >> On 19. 7. 8. ìì 7:32, Dmitry Osipenko wrote:  
> >>> Debug messages create too much CPU and memory activity by
> >>> themselves, so it's difficult to debug lower rates and catch
> >>> unwanted interrupts that happen rarely. Tracepoints are ideal in
> >>> that regards because they do not contribute to the sampled date at
> >>> all. This allowed me to catch few problems which are fixed by the
> >>> followup patches, without tracepoints it would be much harder to
> >>> do.
> >>>
> >>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> >>> ---
> >>>  drivers/devfreq/tegra30-devfreq.c      |  43 +++-------
> >>>  include/trace/events/tegra30_devfreq.h | 105
> >>> +++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 31
> >>> deletions(-) create mode 100644
> >>> include/trace/events/tegra30_devfreq.h    
> >>
> >> As I knew, 'include/trace/events' don't include the header file
> >> for only one device driver. Usually, the trace event is provided
> >> by framework instead of each devic driver.  
> > 
> > There are at least trace headers there for the tegra-apbdma,
> > tegra-host1x, intel-sst and intel-ish devices. I don't think that
> > there is a strict rule for the trace headers placement.  
> 
> OK.
> 
> But, As I already replied on patch4, if you want to show the register
> dump, you better to add the debugfs feature to devfreq framework for
> showing the register dump instead of printing the register dump with
> debug level and this trace event. 
> 
> As I said, just register dump is not useful for all developers.
> Almost developer cannot understand the meaning of debug log for
> register dump.

I think there is some disconnect here. I'm finding that the raw
register values are essential for debugging of this driver. The
registers tracing is very trivial and self-explanatory, just can't see
any better variant.

The registers documentation is available for everyone, you can go to
NVIDIA website and download it (after registration).

We have registers tracing in other Tegra drivers, please see for the
quick example:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/media/tegra-vde/trace.h
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/tegra/trace.h

It's the first time I'm seeing complains about debug tracing and
currently having hard time trying to understand yours point.

> Also, it is not proper way that front patch adds the some code
> and then later patch removes the additional code in the same series.
> Before sending the patches, you can renew them.

Okay.