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.