coresight: core: Add sysfs node to reset all sources and sinks

Message ID 20230208111630.20132-1-quic_jinlmao@quicinc.com
State New
Headers
Series coresight: core: Add sysfs node to reset all sources and sinks |

Commit Message

Mao Jinlong Feb. 8, 2023, 11:16 a.m. UTC
  Add sysfs node to reset all the sources and sinks. When mltiple
sources are enabled, write 1 to reset_source_sink node to disable
all sources and sinks.

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 106 +++++++++++++++++--
 1 file changed, 99 insertions(+), 7 deletions(-)
  

Comments

Mike Leach Feb. 8, 2023, 4:16 p.m. UTC | #1
Hi

As this is a sysfs only update - would it not be easier to simply use
a shell script to iterate through coresight/devices/ looking for
disable_source / disable_sink files and setting those accordingly?

See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
script that does similar iteration to test coresight in perf

Regards

Mike

On Wed, 8 Feb 2023 at 11:16, Mao Jinlong <quic_jinlmao@quicinc.com> wrote:
>
> Add sysfs node to reset all the sources and sinks. When mltiple
> sources are enabled, write 1 to reset_source_sink node to disable
> all sources and sinks.
>
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>  drivers/hwtracing/coresight/coresight-core.c | 106 +++++++++++++++++--
>  1 file changed, 99 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index d3bf82c0de1d..06741ed2dee7 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -458,6 +458,28 @@ static bool coresight_disable_source(struct coresight_device *csdev)
>         return !csdev->enable;
>  }
>
> +/**
> + * coresight_get_source - Get the source from the path
> + *
> + * @path: The list of devices.
> + *
> + * Returns the soruce csdev.
> + *
> + */
> +static struct coresight_device *coresight_get_source(struct list_head *path)
> +{
> +       struct coresight_device *csdev;
> +
> +       if (!path)
> +               return NULL;
> +
> +       csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> +       if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> +               return NULL;
> +
> +       return csdev;
> +}
> +
>  /*
>   * coresight_disable_path_from : Disable components in the given path beyond
>   * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
> @@ -1159,20 +1181,18 @@ int coresight_enable(struct coresight_device *csdev)
>  }
>  EXPORT_SYMBOL_GPL(coresight_enable);
>
> -void coresight_disable(struct coresight_device *csdev)
> +static void _coresight_disable(struct coresight_device *csdev)
>  {
>         int cpu, ret;
>         struct list_head *path = NULL;
>         u32 hash;
>
> -       mutex_lock(&coresight_mutex);
> -
>         ret = coresight_validate_source(csdev, __func__);
>         if (ret)
> -               goto out;
> +               return;
>
>         if (!csdev->enable || !coresight_disable_source(csdev))
> -               goto out;
> +               return;
>
>         switch (csdev->subtype.source_subtype) {
>         case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
> @@ -1187,7 +1207,7 @@ void coresight_disable(struct coresight_device *csdev)
>                 path = idr_find(&path_idr, hash);
>                 if (path == NULL) {
>                         pr_err("Path is not found for %s\n", dev_name(&csdev->dev));
> -                       goto out;
> +                       return;
>                 }
>                 idr_remove(&path_idr, hash);
>                 break;
> @@ -1198,8 +1218,12 @@ void coresight_disable(struct coresight_device *csdev)
>
>         coresight_disable_path(path);
>         coresight_release_path(path);
> +}
>
> -out:
> +void coresight_disable(struct coresight_device *csdev)
> +{
> +       mutex_lock(&coresight_mutex);
> +       _coresight_disable(csdev);
>         mutex_unlock(&coresight_mutex);
>  }
>  EXPORT_SYMBOL_GPL(coresight_disable);
> @@ -1761,8 +1785,76 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
>  }
>  EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
>
> +/*
> + * Set the sink active status to false.
> + */
> +static int coresight_reset_sink(struct device *dev, void *data)
> +{
> +       struct coresight_device *csdev = to_coresight_device(dev);
> +
> +       if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
> +               csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
> +               csdev->activated)
> +               csdev->activated = false;
> +
> +       return 0;
> +}
> +
> +static void coresight_reset_all_sink(void)
> +{
> +       bus_for_each_dev(&coresight_bustype, NULL, NULL, coresight_reset_sink);
> +}
> +
> +static ssize_t reset_source_sink_store(struct bus_type *bus,
> +                                      const char *buf, size_t size)
> +{
> +       int id, cpu, ret = 0;
> +       unsigned long val;
> +       struct coresight_device *csdev;
> +       struct list_head *path = NULL;
> +
> +       ret = kstrtoul(buf, 10, &val);
> +       if (ret)
> +               return ret;
> +
> +       mutex_lock(&coresight_mutex);
> +
> +       /* Disable all per cpu sources */
> +       for_each_present_cpu(cpu) {
> +               path = per_cpu(tracer_path, cpu);
> +               if (path) {
> +                       csdev = coresight_get_source(path);
> +                       if (!csdev)
> +                               continue;
> +               }
> +               _coresight_disable(csdev);
> +       }
> +
> +       /* Disable all sources which aren't associated with CPU */
> +       idr_for_each_entry (&path_idr, path, id) {
> +               csdev = coresight_get_source(path);
> +               if (!csdev)
> +                       continue;
> +
> +               _coresight_disable(csdev);
> +       }
> +       /* Reset all activated sinks */
> +       coresight_reset_all_sink();
> +
> +       mutex_unlock(&coresight_mutex);
> +       return size;
> +}
> +static BUS_ATTR_WO(reset_source_sink);
> +
> +static struct attribute *coresight_reset_source_sink_attrs[] = {
> +       &bus_attr_reset_source_sink.attr,
> +       NULL,
> +};
> +ATTRIBUTE_GROUPS(coresight_reset_source_sink);
> +
>  struct bus_type coresight_bustype = {
>         .name   = "coresight",
> +       .bus_groups     = coresight_reset_source_sink_groups,
>  };
>
>  static int __init coresight_init(void)
> --
> 2.39.0
>
  
Mike Leach Feb. 8, 2023, 4:20 p.m. UTC | #2
Quick correction - you need to look for enable_source  / enable_sink
files and disable those that are currently '1'

Mike

On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> wrote:
>
> Hi
>
> As this is a sysfs only update - would it not be easier to simply use
> a shell script to iterate through coresight/devices/ looking for
> disable_source / disable_sink files and setting those accordingly?
>
> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
> script that does similar iteration to test coresight in perf
>
> Regards
>
> Mike
>
> On Wed, 8 Feb 2023 at 11:16, Mao Jinlong <quic_jinlmao@quicinc.com> wrote:
> >
> > Add sysfs node to reset all the sources and sinks. When mltiple
> > sources are enabled, write 1 to reset_source_sink node to disable
> > all sources and sinks.
> >
> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-core.c | 106 +++++++++++++++++--
> >  1 file changed, 99 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> > index d3bf82c0de1d..06741ed2dee7 100644
> > --- a/drivers/hwtracing/coresight/coresight-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-core.c
> > @@ -458,6 +458,28 @@ static bool coresight_disable_source(struct coresight_device *csdev)
> >         return !csdev->enable;
> >  }
> >
> > +/**
> > + * coresight_get_source - Get the source from the path
> > + *
> > + * @path: The list of devices.
> > + *
> > + * Returns the soruce csdev.
> > + *
> > + */
> > +static struct coresight_device *coresight_get_source(struct list_head *path)
> > +{
> > +       struct coresight_device *csdev;
> > +
> > +       if (!path)
> > +               return NULL;
> > +
> > +       csdev = list_first_entry(path, struct coresight_node, link)->csdev;
> > +       if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
> > +               return NULL;
> > +
> > +       return csdev;
> > +}
> > +
> >  /*
> >   * coresight_disable_path_from : Disable components in the given path beyond
> >   * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
> > @@ -1159,20 +1181,18 @@ int coresight_enable(struct coresight_device *csdev)
> >  }
> >  EXPORT_SYMBOL_GPL(coresight_enable);
> >
> > -void coresight_disable(struct coresight_device *csdev)
> > +static void _coresight_disable(struct coresight_device *csdev)
> >  {
> >         int cpu, ret;
> >         struct list_head *path = NULL;
> >         u32 hash;
> >
> > -       mutex_lock(&coresight_mutex);
> > -
> >         ret = coresight_validate_source(csdev, __func__);
> >         if (ret)
> > -               goto out;
> > +               return;
> >
> >         if (!csdev->enable || !coresight_disable_source(csdev))
> > -               goto out;
> > +               return;
> >
> >         switch (csdev->subtype.source_subtype) {
> >         case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
> > @@ -1187,7 +1207,7 @@ void coresight_disable(struct coresight_device *csdev)
> >                 path = idr_find(&path_idr, hash);
> >                 if (path == NULL) {
> >                         pr_err("Path is not found for %s\n", dev_name(&csdev->dev));
> > -                       goto out;
> > +                       return;
> >                 }
> >                 idr_remove(&path_idr, hash);
> >                 break;
> > @@ -1198,8 +1218,12 @@ void coresight_disable(struct coresight_device *csdev)
> >
> >         coresight_disable_path(path);
> >         coresight_release_path(path);
> > +}
> >
> > -out:
> > +void coresight_disable(struct coresight_device *csdev)
> > +{
> > +       mutex_lock(&coresight_mutex);
> > +       _coresight_disable(csdev);
> >         mutex_unlock(&coresight_mutex);
> >  }
> >  EXPORT_SYMBOL_GPL(coresight_disable);
> > @@ -1761,8 +1785,76 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict,
> >  }
> >  EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
> >
> > +/*
> > + * Set the sink active status to false.
> > + */
> > +static int coresight_reset_sink(struct device *dev, void *data)
> > +{
> > +       struct coresight_device *csdev = to_coresight_device(dev);
> > +
> > +       if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
> > +               csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
> > +               csdev->activated)
> > +               csdev->activated = false;
> > +
> > +       return 0;
> > +}
> > +
> > +static void coresight_reset_all_sink(void)
> > +{
> > +       bus_for_each_dev(&coresight_bustype, NULL, NULL, coresight_reset_sink);
> > +}
> > +
> > +static ssize_t reset_source_sink_store(struct bus_type *bus,
> > +                                      const char *buf, size_t size)
> > +{
> > +       int id, cpu, ret = 0;
> > +       unsigned long val;
> > +       struct coresight_device *csdev;
> > +       struct list_head *path = NULL;
> > +
> > +       ret = kstrtoul(buf, 10, &val);
> > +       if (ret)
> > +               return ret;
> > +
> > +       mutex_lock(&coresight_mutex);
> > +
> > +       /* Disable all per cpu sources */
> > +       for_each_present_cpu(cpu) {
> > +               path = per_cpu(tracer_path, cpu);
> > +               if (path) {
> > +                       csdev = coresight_get_source(path);
> > +                       if (!csdev)
> > +                               continue;
> > +               }
> > +               _coresight_disable(csdev);
> > +       }
> > +
> > +       /* Disable all sources which aren't associated with CPU */
> > +       idr_for_each_entry (&path_idr, path, id) {
> > +               csdev = coresight_get_source(path);
> > +               if (!csdev)
> > +                       continue;
> > +
> > +               _coresight_disable(csdev);
> > +       }
> > +       /* Reset all activated sinks */
> > +       coresight_reset_all_sink();
> > +
> > +       mutex_unlock(&coresight_mutex);
> > +       return size;
> > +}
> > +static BUS_ATTR_WO(reset_source_sink);
> > +
> > +static struct attribute *coresight_reset_source_sink_attrs[] = {
> > +       &bus_attr_reset_source_sink.attr,
> > +       NULL,
> > +};
> > +ATTRIBUTE_GROUPS(coresight_reset_source_sink);
> > +
> >  struct bus_type coresight_bustype = {
> >         .name   = "coresight",
> > +       .bus_groups     = coresight_reset_source_sink_groups,
> >  };
> >
> >  static int __init coresight_init(void)
> > --
> > 2.39.0
> >
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK
  
Suzuki K Poulose Feb. 8, 2023, 4:36 p.m. UTC | #3
On 08/02/2023 16:20, Mike Leach wrote:
> Quick correction - you need to look for enable_source  / enable_sink
> files and disable those that are currently '1'
> 
> Mike
> 
> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> wrote:
>>
>> Hi
>>
>> As this is a sysfs only update - would it not be easier to simply use
>> a shell script to iterate through coresight/devices/ looking for
>> disable_source / disable_sink files and setting those accordingly?
>>
>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
>> script that does similar iteration to test coresight in perf
>>

+1

Suzuki
  
Mao Jinlong Feb. 9, 2023, 3:02 a.m. UTC | #4
On 2/9/2023 12:36 AM, Suzuki K Poulose wrote:
> On 08/02/2023 16:20, Mike Leach wrote:
>> Quick correction - you need to look for enable_source  / enable_sink
>> files and disable those that are currently '1'
>>
>> Mike
>>
>> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> wrote:
>>>
>>> Hi
>>>
>>> As this is a sysfs only update - would it not be easier to simply use
>>> a shell script to iterate through coresight/devices/ looking for
>>> disable_source / disable_sink files and setting those accordingly?
>>>
>>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
>>> script that does similar iteration to test coresight in perf
>>>
>
> +1
>
> Suzuki

Hi Mike & Suzuki,

Sometimes user just want to have some quick test from PC with adb commands.
It is very easy to reset all sources and sinks' status by command below.
echo 1 > /sys/bus/coresight/reset_source_sink

Preparing the script for test is not easy for users who are not familiar 
with the coresight framework.

Thanks
Jinlong Mao

>
>
  
Mike Leach Feb. 9, 2023, 2:56 p.m. UTC | #5
Hi,

On Thu, 9 Feb 2023 at 03:02, Jinlong Mao <quic_jinlmao@quicinc.com> wrote:
>
>
> On 2/9/2023 12:36 AM, Suzuki K Poulose wrote:
> > On 08/02/2023 16:20, Mike Leach wrote:
> >> Quick correction - you need to look for enable_source  / enable_sink
> >> files and disable those that are currently '1'
> >>
> >> Mike
> >>
> >> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> wrote:
> >>>
> >>> Hi
> >>>
> >>> As this is a sysfs only update - would it not be easier to simply use
> >>> a shell script to iterate through coresight/devices/ looking for
> >>> disable_source / disable_sink files and setting those accordingly?
> >>>
> >>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
> >>> script that does similar iteration to test coresight in perf
> >>>
> >
> > +1
> >
> > Suzuki
>
> Hi Mike & Suzuki,
>
> Sometimes user just want to have some quick test from PC with adb commands.
> It is very easy to reset all sources and sinks' status by command below.
> echo 1 > /sys/bus/coresight/reset_source_sink
>

Users of coresight via sysfs will have to know how to use the
coresight infrastructure in order to enable the sources and sinks in
the first place -
e.g
echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
echo 1 > /sys/bus/coresight/devices/etm0/enable_source

Given that they are aware of which sources and sinks they enabled -
disabling them should be simple.


> Preparing the script for test is not easy for users who are not familiar
> with the coresight framework.
>
If there is a genuine use case were a user has opened so many sources
on the command line that they need a simpler way of closing them than
repeating the enabled commands with an
echo 0 > ...
then any script could be shipped as part of kernel/tools/coresight or
kernel/samples/coresight - they would not have to write it themselves,
and just run it from the command line - for example :-
./kernel/tools/coresight/scripts/sysfs_disable_sources_and_sinks.sh

Realistically users will only try out a couple of devices as the
usefulness of the sysfs interface is really limited to testing or
board bring up.
Any complex use with sysfs - as in the coresight tests I mentioned
earlier is really going to be done by scripting.


Regards

Mike

> Thanks
> Jinlong Mao
>
> >
> >
  
Mao Jinlong March 1, 2023, 3:04 p.m. UTC | #6
Hi Mike & Suzuki,

On 2/9/2023 10:56 PM, Mike Leach wrote:
> Hi,
>
> On Thu, 9 Feb 2023 at 03:02, Jinlong Mao <quic_jinlmao@quicinc.com> wrote:
>>
>> On 2/9/2023 12:36 AM, Suzuki K Poulose wrote:
>>> On 08/02/2023 16:20, Mike Leach wrote:
>>>> Quick correction - you need to look for enable_source  / enable_sink
>>>> files and disable those that are currently '1'
>>>>
>>>> Mike
>>>>
>>>> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> wrote:
>>>>> Hi
>>>>>
>>>>> As this is a sysfs only update - would it not be easier to simply use
>>>>> a shell script to iterate through coresight/devices/ looking for
>>>>> disable_source / disable_sink files and setting those accordingly?
>>>>>
>>>>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
>>>>> script that does similar iteration to test coresight in perf
>>>>>
>>> +1
>>>
>>> Suzuki
>> Hi Mike & Suzuki,
>>
>> Sometimes user just want to have some quick test from PC with adb commands.
>> It is very easy to reset all sources and sinks' status by command below.
>> echo 1 > /sys/bus/coresight/reset_source_sink
>>
> Users of coresight via sysfs will have to know how to use the
> coresight infrastructure in order to enable the sources and sinks in
> the first place -
> e.g
> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>
> Given that they are aware of which sources and sinks they enabled -
> disabling them should be simple.
>
>
>> Preparing the script for test is not easy for users who are not familiar
>> with the coresight framework.
>>
> If there is a genuine use case were a user has opened so many sources
> on the command line that they need a simpler way of closing them than
> repeating the enabled commands with an
> echo 0 > ...
> then any script could be shipped as part of kernel/tools/coresight or
> kernel/samples/coresight - they would not have to write it themselves,
> and just run it from the command line - for example :-
> ./kernel/tools/coresight/scripts/sysfs_disable_sources_and_sinks.sh
>
> Realistically users will only try out a couple of devices as the
> usefulness of the sysfs interface is really limited to testing or
> board bring up.
> Any complex use with sysfs - as in the coresight tests I mentioned
> earlier is really going to be done by scripting.
>
>
> Regards
>
> Mike

There is also requirement that reset all the sources and sinks in an 
user space daemon.
For such requirement, I think it is better to use only once sysfs node 
instead of iterating through coresight/device folder in an user space 
daemon.

Thanks
Jinlong Mao

>> Thanks
>> Jinlong Mao
>>
>>>
>
>
  
Mao Jinlong March 17, 2023, 6:18 a.m. UTC | #7
On 3/1/2023 11:04 PM, Jinlong Mao wrote:
> Hi Mike & Suzuki,
>
> On 2/9/2023 10:56 PM, Mike Leach wrote:
>> Hi,
>>
>> On Thu, 9 Feb 2023 at 03:02, Jinlong Mao <quic_jinlmao@quicinc.com> 
>> wrote:
>>>
>>> On 2/9/2023 12:36 AM, Suzuki K Poulose wrote:
>>>> On 08/02/2023 16:20, Mike Leach wrote:
>>>>> Quick correction - you need to look for enable_source  / enable_sink
>>>>> files and disable those that are currently '1'
>>>>>
>>>>> Mike
>>>>>
>>>>> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> 
>>>>> wrote:
>>>>>> Hi
>>>>>>
>>>>>> As this is a sysfs only update - would it not be easier to simply 
>>>>>> use
>>>>>> a shell script to iterate through coresight/devices/ looking for
>>>>>> disable_source / disable_sink files and setting those accordingly?
>>>>>>
>>>>>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
>>>>>> script that does similar iteration to test coresight in perf
>>>>>>
>>>> +1
>>>>
>>>> Suzuki
>>> Hi Mike & Suzuki,
>>>
>>> Sometimes user just want to have some quick test from PC with adb 
>>> commands.
>>> It is very easy to reset all sources and sinks' status by command 
>>> below.
>>> echo 1 > /sys/bus/coresight/reset_source_sink
>>>
>> Users of coresight via sysfs will have to know how to use the
>> coresight infrastructure in order to enable the sources and sinks in
>> the first place -
>> e.g
>> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>>
>> Given that they are aware of which sources and sinks they enabled -
>> disabling them should be simple.
>>
>>
>>> Preparing the script for test is not easy for users who are not 
>>> familiar
>>> with the coresight framework.
>>>
>> If there is a genuine use case were a user has opened so many sources
>> on the command line that they need a simpler way of closing them than
>> repeating the enabled commands with an
>> echo 0 > ...
>> then any script could be shipped as part of kernel/tools/coresight or
>> kernel/samples/coresight - they would not have to write it themselves,
>> and just run it from the command line - for example :-
>> ./kernel/tools/coresight/scripts/sysfs_disable_sources_and_sinks.sh
>>
>> Realistically users will only try out a couple of devices as the
>> usefulness of the sysfs interface is really limited to testing or
>> board bring up.
>> Any complex use with sysfs - as in the coresight tests I mentioned
>> earlier is really going to be done by scripting.
>>
>>
>> Regards
>>
>> Mike
>
> There is also requirement that reset all the sources and sinks in an 
> user space daemon.
> For such requirement, I think it is better to use only once sysfs node 
> instead of iterating through coresight/device folder in an user space 
> daemon.

Hi Mike & Suzuki,

In our internal build, there is binary executable which can configure 
coresight source/sink.
Before running the case, it will disable all the sources and sinks by 
writing reset_source_sink node to
avoid any other source packet's impact.


Thanks
Jinlong Mao

>
> Thanks
> Jinlong Mao
>
>>> Thanks
>>> Jinlong Mao
>>>
>>>>
>>
>>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
  
Suzuki K Poulose March 17, 2023, 9:10 a.m. UTC | #8
On 17/03/2023 06:18, Jinlong Mao wrote:
> 
> On 3/1/2023 11:04 PM, Jinlong Mao wrote:
>> Hi Mike & Suzuki,
>>
>> On 2/9/2023 10:56 PM, Mike Leach wrote:
>>> Hi,
>>>
>>> On Thu, 9 Feb 2023 at 03:02, Jinlong Mao <quic_jinlmao@quicinc.com> 
>>> wrote:
>>>>
>>>> On 2/9/2023 12:36 AM, Suzuki K Poulose wrote:
>>>>> On 08/02/2023 16:20, Mike Leach wrote:
>>>>>> Quick correction - you need to look for enable_source  / enable_sink
>>>>>> files and disable those that are currently '1'
>>>>>>
>>>>>> Mike
>>>>>>
>>>>>> On Wed, 8 Feb 2023 at 16:16, Mike Leach <mike.leach@linaro.org> 
>>>>>> wrote:
>>>>>>> Hi
>>>>>>>
>>>>>>> As this is a sysfs only update - would it not be easier to simply 
>>>>>>> use
>>>>>>> a shell script to iterate through coresight/devices/ looking for
>>>>>>> disable_source / disable_sink files and setting those accordingly?
>>>>>>>
>>>>>>> See tools/perf/tests/shell/test_arm_coresight.sh for an example of a
>>>>>>> script that does similar iteration to test coresight in perf
>>>>>>>
>>>>> +1
>>>>>
>>>>> Suzuki
>>>> Hi Mike & Suzuki,
>>>>
>>>> Sometimes user just want to have some quick test from PC with adb 
>>>> commands.
>>>> It is very easy to reset all sources and sinks' status by command 
>>>> below.
>>>> echo 1 > /sys/bus/coresight/reset_source_sink
>>>>
>>> Users of coresight via sysfs will have to know how to use the
>>> coresight infrastructure in order to enable the sources and sinks in
>>> the first place -
>>> e.g
>>> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>>> echo 1 > /sys/bus/coresight/devices/etm0/enable_source
>>>
>>> Given that they are aware of which sources and sinks they enabled -
>>> disabling them should be simple.
>>>
>>>
>>>> Preparing the script for test is not easy for users who are not 
>>>> familiar
>>>> with the coresight framework.
>>>>
>>> If there is a genuine use case were a user has opened so many sources
>>> on the command line that they need a simpler way of closing them than
>>> repeating the enabled commands with an
>>> echo 0 > ...
>>> then any script could be shipped as part of kernel/tools/coresight or
>>> kernel/samples/coresight - they would not have to write it themselves,
>>> and just run it from the command line - for example :-
>>> ./kernel/tools/coresight/scripts/sysfs_disable_sources_and_sinks.sh
>>>
>>> Realistically users will only try out a couple of devices as the
>>> usefulness of the sysfs interface is really limited to testing or
>>> board bring up.
>>> Any complex use with sysfs - as in the coresight tests I mentioned
>>> earlier is really going to be done by scripting.
>>>
>>>
>>> Regards
>>>
>>> Mike
>>
>> There is also requirement that reset all the sources and sinks in an 
>> user space daemon.
>> For such requirement, I think it is better to use only once sysfs node 
>> instead of iterating through coresight/device folder in an user space 
>> daemon.
> 
> Hi Mike & Suzuki,
> 
> In our internal build, there is binary executable which can configure 
> coresight source/sink.
> Before running the case, it will disable all the sources and sinks by 
> writing reset_source_sink node to
> avoid any other source packet's impact.

How does that justify this patch ? Your internal build depending on
something is your code. It looks like there is more to these patches
than what you are disclosing. e.g., with the dummy device series.

Please could you paint a complete picture and call out the dependencies
/ requirements for what you are trying to achieve ?

Kind regards
Suzuki

> 
> 
> Thanks
> Jinlong Mao
> 
>>
>> Thanks
>> Jinlong Mao
>>
>>>> Thanks
>>>> Jinlong Mao
>>>>
>>>>>
>>>
>>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org
  

Patch

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index d3bf82c0de1d..06741ed2dee7 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -458,6 +458,28 @@  static bool coresight_disable_source(struct coresight_device *csdev)
 	return !csdev->enable;
 }
 
+/**
+ * coresight_get_source - Get the source from the path
+ *
+ * @path: The list of devices.
+ *
+ * Returns the soruce csdev.
+ *
+ */
+static struct coresight_device *coresight_get_source(struct list_head *path)
+{
+	struct coresight_device *csdev;
+
+	if (!path)
+		return NULL;
+
+	csdev = list_first_entry(path, struct coresight_node, link)->csdev;
+	if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
+		return NULL;
+
+	return csdev;
+}
+
 /*
  * coresight_disable_path_from : Disable components in the given path beyond
  * @nd in the list. If @nd is NULL, all the components, except the SOURCE are
@@ -1159,20 +1181,18 @@  int coresight_enable(struct coresight_device *csdev)
 }
 EXPORT_SYMBOL_GPL(coresight_enable);
 
-void coresight_disable(struct coresight_device *csdev)
+static void _coresight_disable(struct coresight_device *csdev)
 {
 	int cpu, ret;
 	struct list_head *path = NULL;
 	u32 hash;
 
-	mutex_lock(&coresight_mutex);
-
 	ret = coresight_validate_source(csdev, __func__);
 	if (ret)
-		goto out;
+		return;
 
 	if (!csdev->enable || !coresight_disable_source(csdev))
-		goto out;
+		return;
 
 	switch (csdev->subtype.source_subtype) {
 	case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC:
@@ -1187,7 +1207,7 @@  void coresight_disable(struct coresight_device *csdev)
 		path = idr_find(&path_idr, hash);
 		if (path == NULL) {
 			pr_err("Path is not found for %s\n", dev_name(&csdev->dev));
-			goto out;
+			return;
 		}
 		idr_remove(&path_idr, hash);
 		break;
@@ -1198,8 +1218,12 @@  void coresight_disable(struct coresight_device *csdev)
 
 	coresight_disable_path(path);
 	coresight_release_path(path);
+}
 
-out:
+void coresight_disable(struct coresight_device *csdev)
+{
+	mutex_lock(&coresight_mutex);
+	_coresight_disable(csdev);
 	mutex_unlock(&coresight_mutex);
 }
 EXPORT_SYMBOL_GPL(coresight_disable);
@@ -1761,8 +1785,76 @@  char *coresight_alloc_device_name(struct coresight_dev_list *dict,
 }
 EXPORT_SYMBOL_GPL(coresight_alloc_device_name);
 
+/*
+ * Set the sink active status to false.
+ */
+static int coresight_reset_sink(struct device *dev, void *data)
+{
+	struct coresight_device *csdev = to_coresight_device(dev);
+
+	if ((csdev->type == CORESIGHT_DEV_TYPE_SINK ||
+		csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) &&
+		csdev->activated)
+		csdev->activated = false;
+
+	return 0;
+}
+
+static void coresight_reset_all_sink(void)
+{
+	bus_for_each_dev(&coresight_bustype, NULL, NULL, coresight_reset_sink);
+}
+
+static ssize_t reset_source_sink_store(struct bus_type *bus,
+				       const char *buf, size_t size)
+{
+	int id, cpu, ret = 0;
+	unsigned long val;
+	struct coresight_device *csdev;
+	struct list_head *path = NULL;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	mutex_lock(&coresight_mutex);
+
+	/* Disable all per cpu sources */
+	for_each_present_cpu(cpu) {
+		path = per_cpu(tracer_path, cpu);
+		if (path) {
+			csdev = coresight_get_source(path);
+			if (!csdev)
+				continue;
+		}
+		_coresight_disable(csdev);
+	}
+
+	/* Disable all sources which aren't associated with CPU */
+	idr_for_each_entry (&path_idr, path, id) {
+		csdev = coresight_get_source(path);
+		if (!csdev)
+			continue;
+
+		_coresight_disable(csdev);
+	}
+	/* Reset all activated sinks */
+	coresight_reset_all_sink();
+
+	mutex_unlock(&coresight_mutex);
+	return size;
+}
+static BUS_ATTR_WO(reset_source_sink);
+
+static struct attribute *coresight_reset_source_sink_attrs[] = {
+	&bus_attr_reset_source_sink.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(coresight_reset_source_sink);
+
 struct bus_type coresight_bustype = {
 	.name	= "coresight",
+	.bus_groups	= coresight_reset_source_sink_groups,
 };
 
 static int __init coresight_init(void)