From: Linu Cherian <linuc.decode@gmail.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: mathieu.poirier@linaro.org, suzuki.poulose@arm.com,
coresight@lists.linaro.org, Linu Cherian <lcherian@marvell.com>,
linux-arm-kernel@lists.infradead.org, mike.leach@linaro.org
Subject: Re: [PATCH V2] coresight: Make sysFS functional on topologies with per core sink
Date: Mon, 3 Aug 2020 16:56:15 +0530 [thread overview]
Message-ID: <CAAHhmWjJoEq77ROKGjfXFbT3Yi=H5Soa5_J6nt-1A=7-kWawGA@mail.gmail.com> (raw)
In-Reply-To: <b07e9924-0438-299e-e7a0-c3377884408b@arm.com>
Hi Anshuman,
On Mon, Aug 3, 2020 at 3:07 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Hello Linu,
>
> Please do mention the tree on which this patch applies on. The coresight
> API functions being changed here are not even present on v5.8 but instead
> are part of linux-next (atleast next-20200731).
Will be more explicit next time, though i have mentioned the relevant
tree below.
>
> On 08/01/2020 02:39 PM, Linu Cherian wrote:
> > Coresight driver assumes sink is common across all the ETMs,
>
> Does 'common' just implies reachable here. Should not all the sinks on
> the system be reachable from all the sources.
Common implies, a topology where a sink is common to all ETMs,
a global ETR for example. Please see below for details.
>
> > and tries to build a path between ETM and the first enabled
> > sink found using bus based search. This breaks sysFS usage
> > on implementations that has multiple per core sinks in
> > enabled state.
>
> Could you please explain how it breaks the sysfs usage ? Why those per
> core sinks were not found through existing bus based search.
>
On an implementation with per core ETR, and when a user tries to enable
multiple sinks using the syfs interface, only the first sink on the bus would
get enabled at hardware level.
This is because coresight_get_enabled_sink would always select the
first user enabled
sink on the coresight bus, which works fine on implementations with
global ETR but
not for per core ETR/ETF.
So for example, when an user enables ETM 0 and ETR0 , followed by ETM
1 and ETR 1,
then ETR1 hardware block will never get enabled.
> >
> > For this,
> > - coresight_find_sink API is updated with an additional flag
> > so that it is able to return an enabled sink
> > - coresight_get_enabled_sink API is updated to do a
> > connection based search, when a source reference is given.
> >
> > Change-Id: I6cc91ddb3ef8936a8f41a5f7c7c455b0ece9d85d
> > Signed-off-by: Linu Cherian <lcherian@marvell.com>
> > ---
> > Applies on https://git.linaro.org/kernel/coresight.git/log/?h=next
> >
> > Changes in V2:
> > - Fixed few typos in commit message
> > - Rephrased commit message
> >
> > .../hwtracing/coresight/coresight-etm-perf.c | 2 +-
> > drivers/hwtracing/coresight/coresight-priv.h | 5 +-
> > drivers/hwtracing/coresight/coresight.c | 51 +++++++++++++++++--
> > 3 files changed, 51 insertions(+), 7 deletions(-)
> >
> > +/**
> > + * coresight_find_enabled_sink: Find the suitable enabled sink
> > + *
> > + * @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.
> > + *
> > + * Used in cases where the CoreSight user (sysfs) has selected a sink.
> > + */
> > +struct coresight_device *
> > +coresight_find_enabled_sink(struct coresight_device *csdev)
> > +{
> > + int depth = 0;
> > +
> > + /* look for the enabled sink */
> > + return coresight_find_sink(csdev, &depth, true);
> > +}
> > +
>
> Why this wrapper is required ? Could not coresight_find_sink() be used
> directly in the only caller i.e coresight_get_enabled_sink(). Besides,
> their names are really very similar and might lead to confusion.
Thought that a wrapper avoids exposing the internals like "depth" in
the search API
to its users.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-08-03 11:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-01 9:09 [PATCH V2] coresight: Make sysFS functional on topologies with per core sink Linu Cherian
2020-08-03 9:37 ` Anshuman Khandual
2020-08-03 11:26 ` Linu Cherian [this message]
2020-08-07 17:07 ` Linu Cherian
2020-08-10 22:25 ` Mathieu Poirier
2020-08-11 2:46 ` Linu Cherian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAHhmWjJoEq77ROKGjfXFbT3Yi=H5Soa5_J6nt-1A=7-kWawGA@mail.gmail.com' \
--to=linuc.decode@gmail.com \
--cc=anshuman.khandual@arm.com \
--cc=coresight@lists.linaro.org \
--cc=lcherian@marvell.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).