Re: [PATCH v4 5/5] coresight: sysfs: Allow select default sink on source enable.

[Mike Leach <mike.leach@linaro.org>]

HI Suzuki,

On Tue, 2 Jun 2020 at 12:46, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Hi Mike,
>
> On 05/26/2020 11:46 AM, Mike Leach wrote:
> > When enabling a trace source using sysfs, allow the CoreSight system to
> > auto-select a default sink if none has been enabled by the user.
> >
> > Uses the sink select algorithm that uses the default select priorities
> > set when sinks are registered with the system. At present this will
> > prefer ETR over ETB / ETF.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> >   drivers/hwtracing/coresight/coresight.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> > index 7632d060e25d..bd1a52a65d00 100644
> > --- a/drivers/hwtracing/coresight/coresight.c
> > +++ b/drivers/hwtracing/coresight/coresight.c
> > @@ -965,8 +965,15 @@ int coresight_enable(struct coresight_device *csdev)
> >        */
> >       sink = coresight_get_enabled_sink(false);
> >       if (!sink) {
> > -             ret = -EINVAL;
> > -             goto out;
> > +             /* look for a default sink if nothing enabled */
> > +             sink = coresight_find_default_sink(csdev);
> > +             if (!sink) {
> > +                     ret = -EINVAL;
> > +                     goto out;
> > +             }
> > +             /* mark the default as enabled */
> > +             sink->activated = true;
> > +             dev_info(&sink->dev, "Enabled default sink.");
> >       }
>
> To be honest, I would drop this change and mandate that the
> user enables the sink(s).

This is here to make it easy for users to explore CoreSight using
sysfs - which is what tends to happen when they first start using it.
Also useful for generic test scripts if no sink is needed to check if
the ETM is working.


> There is no way to reliably match
> which ETM is tracing to the sink above in case multiple ETMs
> are being enabled, even with the above message.

I can't imagine users setting multiple sinks, either through the
default route or the explicit set route. Either way the problem is the
same - which sink does the etm hit?
I think this means that we need to improve this rather than drop it.
The default sink could easily be read by from the ETM via sysfs. as
could an addition to provide a last used sink for sysfs.
This would be really useful in developing test scripts that exercised
ETMs on a system without having to figure out the sink used. Such
scripts would then be more portable.

Regards

Mike


>It is important
> for sysfs mode, as the user must collect the trace data manually,
> unlike the perf mode where the race data is collected and presented to
> the user via memory buffers.
>
> Suzuki



--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel