On 16/05/2024 15:23, James Clark wrote:
On 16/05/2024 04:56, Mao Jinlong wrote:
Dynamic trace id was introduced in coresight subsystem so trace id is allocated dynamically. However, some hardware ATB source has static trace id and it cannot be changed via software programming. Reserve trace id for this kind of hardware source.
Signed-off-by: Mao Jinlong quic_jinlmao@quicinc.com
.../hwtracing/coresight/coresight-platform.c | 26 +++++++++++++++++++ .../hwtracing/coresight/coresight-trace-id.c | 24 +++++++++++++++++ .../hwtracing/coresight/coresight-trace-id.h | 11 ++++++++ include/linux/coresight.h | 1 + 4 files changed, 62 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index 9d550f5697fa..d3e22a2608df 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -183,6 +183,17 @@ static int of_coresight_get_cpu(struct device *dev) return cpu; } +/*
- of_coresight_get_trace_id: Get the atid of a source device.
- Returns 0 on success.
- */
+static int of_coresight_get_trace_id(struct device *dev, u32 *id) +{
- return of_property_read_u32(dev->of_node, "trace-id", id);
+}
/*
- of_coresight_parse_endpoint : Parse the given output endpoint @ep
- and fill the connection information in @pdata->out_conns
@@ -315,6 +326,12 @@ static inline int of_coresight_get_cpu(struct device *dev) { return -ENODEV; }
+static int of_coresight_get_trace_id(struct device *dev, u32 *id) +{
- return -ENODEV;
+}
#endif #ifdef CONFIG_ACPI @@ -794,6 +811,15 @@ int coresight_get_cpu(struct device *dev) } EXPORT_SYMBOL_GPL(coresight_get_cpu); +int coresight_get_trace_id(struct device *dev, u32 *id) +{
- if (!is_of_node(dev->fwnode))
return -EINVAL;- return of_coresight_get_trace_id(dev, id);
Can we somehow make this function name distinct from the trace ID functions. It's a bit hard to read that it's called coresight_get_trace_id() but it doesn't actually get an ID from the existing trace ID stuff.
+} +EXPORT_SYMBOL_GPL(coresight_get_trace_id);
struct coresight_platform_data * coresight_get_platform_data(struct device *dev) { diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c index af5b4ef59cea..536a34e9de6f 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.c +++ b/drivers/hwtracing/coresight/coresight-trace-id.c @@ -110,6 +110,24 @@ static int coresight_trace_id_alloc_new_id(struct coresight_trace_id_map *id_map return id; } +static int coresight_trace_id_set(int id, struct coresight_trace_id_map *id_map) +{
- unsigned long flags;
- spin_lock_irqsave(&id_map_lock, flags);
- if (WARN(!IS_VALID_CS_TRACE_ID(id), "Invalid Trace ID %d\n", id))
return -EINVAL;- if (WARN(test_bit(id, id_map->used_ids), "ID is already used: %d\n", id))
return -EINVAL;Do these returns not skip unlocking the spinlock?
It might be slightly fewer changes if we update the existing coresight_trace_id_alloc_new_id() to add a new "only_preferred" option.
Then use the existing system id allocator which already handles the lock and unlock properly:
static int coresight_trace_id_map_get_system_id(struct coresight_trace_id_map *id_map, int id,
bool only_preferred){ ... spin_lock_irqsave(&id_map_lock, flags); /* prefer odd IDs for system components to avoid legacy CPU IDS id = coresight_trace_id_alloc_new_id(id_map, id, true, only_preferred); spin_unlock_irqrestore(&id_map_lock, flags); ...
I suppose the end result is the same as your implementation, but it trades making one existing function slightly more complicated instead of adding some new ones.
It's also not that obvious that there is the new reserve function, but you still free the ID with the same coresight_trace_id_put_system_id().
Another benefit of adding arguments to the existing functions is that we keep just ...get...() and ...put...(). 'Reserve' implies some other new mechanism, but it's really a normal get. I think we should do one of these two options for the top level API:
#1 (when id != 0, then it's an "only preferred" preferred ID: coresight_trace_id_get_system_id(int id) coresight_trace_id_put_system_id(int id)
#2 coresight_trace_id_get_system_id() coresight_trace_id_get_system_id_resrv(int id) coresight_trace_id_put_system_id(int id)