[PATCH 07/10 v3] coresight-etm: add CoreSight ETM/PTM driver

[Al Grant]

> On 8 August 2014 10:58, Al Grant <Al.Grant at arm.com> wrote:
> > Hi,
> >
> > Comments inline...
> >
> >
> >> -----Original Message-----
> >> From: mathieu.poirier at linaro.org
> [mailto:mathieu.poirier at linaro.org]
>
> SNIP...
>
> >> ETMCNTVRn(i));
> >> +     }
> >> +     etm_writel(drvdata, drvdata->seq_12_event, ETMSQ12EVR);
> >> +     etm_writel(drvdata, drvdata->seq_21_event, ETMSQ21EVR);
> >> +     etm_writel(drvdata, drvdata->seq_23_event, ETMSQ23EVR);
> >> +     etm_writel(drvdata, drvdata->seq_31_event, ETMSQ31EVR);
> >> +     etm_writel(drvdata, drvdata->seq_32_event, ETMSQ32EVR);
> >> +     etm_writel(drvdata, drvdata->seq_13_event, ETMSQ13EVR);
> >> +     etm_writel(drvdata, drvdata->seq_curr_state, ETMSQR);
> >> +     for (i = 0; i < drvdata->nr_ext_out; i++)
> >> +             etm_writel(drvdata, ETM_DEFAULT_EVENT_VAL,
> >> ETMEXTOUTEVRn(i));
> >> +     for (i = 0; i < drvdata->nr_ctxid_cmp; i++)
> >> +             etm_writel(drvdata, drvdata->ctxid_val[i],
> >> ETMCIDCVRn(i));
> >> +     etm_writel(drvdata, drvdata->ctxid_mask, ETMCIDCMR);
> >> +     etm_writel(drvdata, drvdata->sync_freq, ETMSYNCFR);
> >> +     /* no external input selected */
> >> +     etm_writel(drvdata, 0x0, ETMEXTINSELR);
> >> +     etm_writel(drvdata, drvdata->timestamp_event, ETMTSEVR);
> >> +     /* no auxiliary control selected */
> >> +     etm_writel(drvdata, 0x0, ETMAUXCR);
> >> +     etm_writel(drvdata, drvdata->cpu + 1, ETMTRACEIDR);
> >
> > This seems slightly hardwired.  When you have additional non-CPU
> trace
> > sources like STM (perhaps several STM) there will need to be a
> scheme
> > for allocating the trace ids uniquely across the sources.
> Numbering
> > the ETMs statically from 1 upwards and the non-CPU sources
> dynamically
> > from say 0x70 downwards is fine, but something ought to check they
> > don't collide.
>
> I think you are right - source IDs should be configurable from
> userspace.  On the flip side I don't think the kernel should enforce
> having unique IDs among sources - what if someone want to have
> source
> of the same type have the same ID?

Then it would be impossible to demultiplex if those trace streams ever
came together in the same funnel.  The CoreSight architecture just says
"The ID for each trace stream must be unique".  You can only get away
with reusing ids if you are 100% sure they will never be funnelled together.
Having unique ids avoids this fragility.


> I think we can display a warning
> message when the ID of a source that is about become active matches
> an
> already active one but nothing more.  Get back to me if you feel
> differently about that.

I'd be happy with that.  The main thing is that if the framework itself
is auto allocating trace source ids, make sure they don't bump into any
that the user might have set.  You could do that by reserving some
ids for the CPUs (i.e. ETMs) or some other way.


> > Also, if the intention is the user can find the configuration of a
> > component from the drvdata without a device read, this needs to be
> true
> > of TRACEID too.
>
> Sorry, I don't quite get your comment.  Could you please be more
> specific of provide an example?  That way I can be sure that your
> comment is addressed.

I meant, if you do let the user set the trace id (e.g. by adding a
drvdata->traceid, ways to set it via debugfs etc.), please provide a
way to read it back out again.  Then something that's producing a
post-mortem trace package (say) can produce all the required metadata
to associate trace ids with sources.


> > Does that mean ETM can't be enabled over cpu power-down?
> > In the case where an ETM is powered up and the cpu is not,
> > you might want to program and enable the ETM, to trace the
> > cpu's power-up sequence.
>
> This a valid usecase that I had not considered - ok, I'll look into
> that.
>
> >
> > Or, if the ETM is powered down but can be powered up independently
> > of the CPU, etm_enable could do so.
>
> Once again, I'm not sure that I fully grasp your train of thoughts -
> please rephrase or be more specific.

I meant that if the ETM was in a separate power domain from the CPU
and could be powered up before the CPU was, the driver could take some
action to power up the ETM's power domain.


> You're touching on something I've been thinking about for a while...
> Since configuration is pushed to the HW when a source is "enabled",
> there is a window where what the SW report is different from how the
> HW is configured.  One can't really live without the other - when
> configuring the system it is always good to have the "current" value
> held by a register.  I'll have to think about how to represent
> ongoing
> configuration (SW) and what the HW really reports.
>
> > It would be good to have a way of directly reading
> > and changing the sequencer state via debugfs, since that could
> enable
> > some quite interesting kinds of trace control.  The same applies
> to the
> > counter values.
>
> You mean while an ETM/PTM is configured and tracing?

Yes, you can read out the sequencer and counters at any time, and you'll
see a value that it held at some recent point.  Use of the sequencer
and counters is quite advanced, but if you're using them at all, to set
up complex trace enable/disable conditions, then being able to monitor
their current state without stopping the ETM could be useful.

You can only change them once the ETM is stopped and in programming mode.

Al

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782