Re: [PATCH v3 03/13] coresight: stm: Update STM driver to use Trace ID API

From: Suzuki K Poulose
Date: Fri Oct 07 2022 - 13:53:12 EST


On 06/10/2022 14:54, Mike Leach wrote:
Hi Suzuki,

On Mon, 3 Oct 2022 at 10:04, Suzuki K Poulose <suzuki.poulose@xxxxxxx> wrote:

On 09/08/2022 23:33, Mike Leach wrote:
Updates the STM driver to use the trace ID allocation API.
This uses the _system_id calls to allocate an ID on device poll,
and release on device remove.

The sysfs access to the STMTRACEIDR register has been changed from RW
to RO. Having this value as writable is not appropriate for the new
Trace ID scheme - and had potential to cause errors in the previous
scheme if values clashed with other sources.

Signed-off-by: Mike Leach <mike.leach@xxxxxxxxxx>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>

@@ -854,7 +830,7 @@ static void stm_init_generic_data(struct stm_drvdata *drvdata,

static int stm_probe(struct amba_device *adev, const struct amba_id *id)
{
- int ret;
+ int ret, trace_id;
void __iomem *base;
struct device *dev = &adev->dev;
struct coresight_platform_data *pdata = NULL;
@@ -938,12 +914,22 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
goto stm_unregister;
}

+ trace_id = coresight_trace_id_get_system_id();
+ if (trace_id < 0) {

The above API returns "INVALID_ID" and not a negative error status.
I think it is better to fix the API to return:

ret < 0 - If there is any error
- Otherwise a positive integer
And the users should be kept unaware of which ID is valid or invalid.


coresight_trace_id_get_system_id() returns the ID if one can be
allocated or -EINVAL if not.

Not sure what you are looking at here.

Sorry, indeed I was mistaken there. It is the get_cpu_id() which
returns the INVALID_ID on failure. Please could we make that
consistent with this scheme ? i.e, < 0 on error.

Also, please could we add a comment above the exported functions
on their entry/exit criteria ? It is not clearly evident, unless
we follow the code and figure out.

Cheers
Suzuki