linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Mike Leach <mike.leach@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
	 linux-kernel@vger.kernel.org, mathieu.poirier@linaro.org,
	 peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	 linux-perf-users@vger.kernel.org, leo.yan@linaro.org,
	 quic_jinlmao@quicinc.com
Subject: Re: [PATCH v3 03/13] coresight: stm: Update STM driver to use Trace ID API
Date: Thu, 6 Oct 2022 14:54:50 +0100	[thread overview]
Message-ID: <CAJ9a7Vgz+0xEQO-MvGUzbsr_LBh4pDep7JJtFoA+cAeiAERJFw@mail.gmail.com> (raw)
In-Reply-To: <65e70db9-9f85-7285-0602-f2d29887550a@arm.com>

Hi Suzuki,

On Mon, 3 Oct 2022 at 10:04, Suzuki K Poulose <suzuki.poulose@arm.com> 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@linaro.org>
> > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > ---
> >   drivers/hwtracing/coresight/coresight-stm.c | 41 +++++++--------------
> >   1 file changed, 14 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> > index bb14a3a8a921..9ef3e923a930 100644
> > --- a/drivers/hwtracing/coresight/coresight-stm.c
> > +++ b/drivers/hwtracing/coresight/coresight-stm.c
> > @@ -31,6 +31,7 @@
> >   #include <linux/stm.h>
> >
> >   #include "coresight-priv.h"
> > +#include "coresight-trace-id.h"
> >
> >   #define STMDMASTARTR                        0xc04
> >   #define STMDMASTOPR                 0xc08
> > @@ -615,24 +616,7 @@ static ssize_t traceid_show(struct device *dev,
> >       val = drvdata->traceid;
> >       return sprintf(buf, "%#lx\n", val);
> >   }
> > -
> > -static ssize_t traceid_store(struct device *dev,
> > -                          struct device_attribute *attr,
> > -                          const char *buf, size_t size)
> > -{
> > -     int ret;
> > -     unsigned long val;
> > -     struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > -
> > -     ret = kstrtoul(buf, 16, &val);
> > -     if (ret)
> > -             return ret;
> > -
> > -     /* traceid field is 7bit wide on STM32 */
> > -     drvdata->traceid = val & 0x7f;
> > -     return size;
> > -}
> > -static DEVICE_ATTR_RW(traceid);
> > +static DEVICE_ATTR_RO(traceid);
> >
> >   #define coresight_stm_reg(name, offset)     \
> >       coresight_simple_reg32(struct stm_drvdata, name, offset)
> > @@ -819,14 +803,6 @@ static void stm_init_default_data(struct stm_drvdata *drvdata)
> >        */
> >       drvdata->stmsper = ~0x0;
> >
> > -     /*
> > -      * The trace ID value for *ETM* tracers start at CPU_ID * 2 + 0x10 and
> > -      * anything equal to or higher than 0x70 is reserved.  Since 0x00 is
> > -      * also reserved the STM trace ID needs to be higher than 0x00 and
> > -      * lowner than 0x10.
> > -      */
> > -     drvdata->traceid = 0x1;
> > -
> >       /* Set invariant transaction timing on all channels */
> >       bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
> >   }
> > @@ -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.

Mike


> 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

  reply	other threads:[~2022-10-06 13:56 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-09 22:33 [PATCH v3 00/13] coresight: Add new API to allocate trace source ID values Mike Leach
2022-08-09 22:33 ` [PATCH v3 01/13] coresight: trace-id: Add API to dynamically assign Trace " Mike Leach
2022-10-03  8:55   ` Suzuki K Poulose
2022-10-11 10:22     ` Mike Leach
2022-08-09 22:33 ` [PATCH v3 02/13] coresight: Remove obsolete Trace ID unniqueness checks Mike Leach
2022-10-03  8:56   ` Suzuki K Poulose
2022-08-09 22:33 ` [PATCH v3 03/13] coresight: stm: Update STM driver to use Trace ID API Mike Leach
2022-10-03  9:04   ` Suzuki K Poulose
2022-10-06 13:54     ` Mike Leach [this message]
2022-10-07 17:53       ` Suzuki K Poulose
2022-10-11 11:10         ` Mike Leach
2022-10-11 15:10           ` Suzuki K Poulose
2022-08-09 22:33 ` [PATCH v3 04/13] coresight: etm4x: Update ETM4 " Mike Leach
2022-10-03  9:31   ` Suzuki K Poulose
2022-10-03  9:37     ` Suzuki K Poulose
2022-10-06 13:47       ` Mike Leach
2022-08-09 22:33 ` [PATCH v3 05/13] coresight: etm3x: Update ETM3 " Mike Leach
2022-08-13  9:53   ` kernel test robot
2022-08-13 13:48   ` kernel test robot
2022-08-09 22:33 ` [PATCH v3 06/13] coresight: etmX.X: stm: Remove trace_id() callback Mike Leach
2022-08-09 22:33 ` [PATCH v3 07/13] coresight: perf: traceid: Add perf notifiers for Trace ID Mike Leach
2022-08-09 22:33 ` [PATCH v3 08/13] perf: cs-etm: Move mapping of Trace ID and cpu into helper function Mike Leach
2022-08-09 22:33 ` [PATCH v3 09/13] perf: cs-etm: Update record event to use new Trace ID protocol Mike Leach
2022-08-09 22:33 ` [PATCH v3 10/13] kernel: events: Export perf_report_aux_output_id() Mike Leach
2022-08-09 22:33 ` [PATCH v3 11/13] perf: cs-etm: Handle PERF_RECORD_AUX_OUTPUT_HW_ID packet Mike Leach
2022-08-09 22:34 ` [PATCH v3 12/13] coresight: events: PERF_RECORD_AUX_OUTPUT_HW_ID used for Trace ID Mike Leach
2022-08-09 22:34 ` [PATCH v3 13/13] coresight: trace-id: Add debug & test macros to Trace ID allocation Mike Leach
2022-10-03 11:06   ` Suzuki K Poulose
2022-10-06 13:22     ` Mike Leach
2022-08-12 19:50 ` [PATCH v3 00/13] coresight: Add new API to allocate trace source ID values Arnaldo Carvalho de Melo
2022-08-15 19:04   ` Mike Leach

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=CAJ9a7Vgz+0xEQO-MvGUzbsr_LBh4pDep7JJtFoA+cAeiAERJFw@mail.gmail.com \
    --to=mike.leach@linaro.org \
    --cc=acme@kernel.org \
    --cc=coresight@lists.linaro.org \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=quic_jinlmao@quicinc.com \
    --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).