Re: [PATCH v4 2/5] coresight: Add default sink selection to CoreSight base

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

Hi Suzuki,

On Tue, 2 Jun 2020 at 11:20, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> On 05/26/2020 11:46 AM, Mike Leach wrote:
> > Adds a method to select a suitable sink connected to a given source.
> >
> > In cases where no sink is defined, the coresight_find_default_sink
> > routine can search from a given source, through the child connections
> > until a suitable sink is found.
> >
> > The suitability is defined in by the sink coresight_dev_subtype on the
> > CoreSight device, and the distance from the source by counting
> > connections.
> >
> > Higher value subtype is preferred - where these are equal, shorter
> > distance from source is used as a tie-break.
> >
> > This allows for default sink to be discovered were none is specified
> > (e.g. perf command line)
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
>
> Feel free to add:
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>

Will do.

> > +/**
> > + * coresight_find_default_sink: Find a sink suitable for use as a
> > + * default sink.Yes - the newer topologies will need some changes - beyond what we are handling here.
However - especially for 1:1 -  the best way may be to always use the
default sink - as specifying multiple sinks on the perf command line
may be problematical.


> > + *
> > + * @csdev: starting source to find a connected sink.
> > + *
> > + * Walks connections graph looking for a suitable sink to enable for the
> > + * supplied source. Uses CoreSight device subtypes and distance from source
> > + * to select the best sink.
> > + *
> > + * If a sink is found, then the default sink for this device is set and
> > + * will be automatically used in future.
> > + *
> > + * Used in cases where the CoreSight user (perf / sysfs) has not selected a
> > + * sink.
> > + */
> > +struct coresight_device *
> > +coresight_find_default_sink(struct coresight_device *csdev)
> > +{
> > +     int depth = 0;
> > +
> > +     /* look for a default sink if we have not found for this device */
> > +     if (!csdev->def_sink)
> > +             csdev->def_sink = coresight_find_sink(csdev, &depth);
>
> Could we add a helper to clear the cached "def-sink" from all the
> "sources", when the sink is going away ? You may be able to do this via
> coresight_remove_match()
>

Is this needed in the current state of the CoreSight driver
infrastructure? The topology is fixed for the lifetime of the device
so a sink cannot disappear without the rest of the CoreSight going
away - i.e. reboot / shutdown.
If there is concern about a race condition then something similar to
coresight_remove_match can be developed, but the actual function only
works on immediate connections / output ports.

Thanks

Mike


> Rest looks good to me.
> 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