* [PATCH V2 0/6] Introduce CoreSight STM support @ 2016-02-03 8:15 Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 1/6] stm class: Add ioctl get_options interface Chunyan Zhang ` (5 more replies) 0 siblings, 6 replies; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc This patchset adds support for CoreSight STM IP block. It also makes a little modification to the generic STM framework to cover the CoreSight STM requirements. Full description follows the changelog. Changes from v1: - Added a definition of coresight_simple_func() in CS-STM driver to avoid the kbuild test robot error for the time being. This modification will be removed when merging the code in which the coresight_simple_func() has been moved to the header file. - Calculate the channel number according to the channel memory space size. Thanks, Chunyan Chunyan Zhang (3): stm class: Add ioctl get_options interface stm class: adds a loop to extract the first valid STM device name Documentations: Add explanations of the case for non-configurable masters Mathieu Poirier (2): stm class: provision for statically assigned masterIDs coresight-stm: Bindings for System Trace Macrocell Pratik Patel (1): coresight-stm: adding driver for CoreSight STM component .../ABI/testing/sysfs-bus-coresight-devices-stm | 53 ++ .../devicetree/bindings/arm/coresight.txt | 28 + Documentation/trace/coresight.txt | 37 +- Documentation/trace/stm.txt | 6 + drivers/hwtracing/coresight/Kconfig | 11 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-stm.c | 972 +++++++++++++++++++++ drivers/hwtracing/stm/core.c | 29 +- drivers/hwtracing/stm/policy.c | 46 +- include/linux/coresight-stm.h | 6 + include/linux/stm.h | 11 + include/uapi/linux/coresight-stm.h | 12 + include/uapi/linux/stm.h | 1 + 13 files changed, 1196 insertions(+), 17 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm create mode 100644 drivers/hwtracing/coresight/coresight-stm.c create mode 100644 include/linux/coresight-stm.h create mode 100644 include/uapi/linux/coresight-stm.h -- 1.9.1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2 1/6] stm class: Add ioctl get_options interface 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-05 12:55 ` Alexander Shishkin 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang ` (4 subsequent siblings) 5 siblings, 1 reply; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc There is already an interface of set_options, but no get_options yet. Before setting any options, one would may want to see the current status of that option by means of get_options interface. This interface has been used in CoreSight STM driver. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/hwtracing/stm/core.c | 11 +++++++++++ include/linux/stm.h | 3 +++ include/uapi/linux/stm.h | 1 + 3 files changed, 15 insertions(+) diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index b6445d9..86bb4e3 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -571,6 +571,17 @@ stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg) options); break; + + case STP_GET_OPTIONS: + if (stm_data->get_options) + err = stm_data->get_options(stm_data, + stmf->output.master, + stmf->output.channel, + stmf->output.nr_chans, + &options); + + return copy_to_user((void __user *)arg, &options, sizeof(u64)); + default: break; } diff --git a/include/linux/stm.h b/include/linux/stm.h index 9d0083d..f351d62 100644 --- a/include/linux/stm.h +++ b/include/linux/stm.h @@ -88,6 +88,9 @@ struct stm_data { long (*set_options)(struct stm_data *, unsigned int, unsigned int, unsigned int, unsigned long); + long (*get_options)(struct stm_data *, unsigned int, + unsigned int, unsigned int, + u64 *); }; int stm_register_device(struct device *parent, struct stm_data *stm_data, diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h index 626a8d3..0dab16e 100644 --- a/include/uapi/linux/stm.h +++ b/include/uapi/linux/stm.h @@ -46,5 +46,6 @@ struct stp_policy_id { #define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id) #define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id) #define STP_SET_OPTIONS _IOW('%', 2, __u64) +#define STP_GET_OPTIONS _IOR('%', 3, __u64) #endif /* _UAPI_LINUX_STM_H */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V2 1/6] stm class: Add ioctl get_options interface 2016-02-03 8:15 ` [PATCH V2 1/6] stm class: Add ioctl get_options interface Chunyan Zhang @ 2016-02-05 12:55 ` Alexander Shishkin 0 siblings, 0 replies; 27+ messages in thread From: Alexander Shishkin @ 2016-02-05 12:55 UTC (permalink / raw) To: Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Chunyan Zhang <zhang.chunyan@linaro.org> writes: > There is already an interface of set_options, but no get_options yet. > Before setting any options, one would may want to see the current > status of that option by means of get_options interface. This > interface has been used in CoreSight STM driver. > > Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> > --- > drivers/hwtracing/stm/core.c | 11 +++++++++++ > include/linux/stm.h | 3 +++ > include/uapi/linux/stm.h | 1 + > 3 files changed, 15 insertions(+) > > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index b6445d9..86bb4e3 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -571,6 +571,17 @@ stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > options); > > break; > + > + case STP_GET_OPTIONS: > + if (stm_data->get_options) > + err = stm_data->get_options(stm_data, > + stmf->output.master, > + stmf->output.channel, > + stmf->output.nr_chans, > + &options); > + > + return copy_to_user((void __user *)arg, &options, sizeof(u64)); The return value of copy_to_user() is not what you assume it is. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 1/6] stm class: Add ioctl get_options interface Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-03 10:05 ` [PATCH] stm class: fix semicolon.cocci warnings kbuild test robot ` (2 more replies) 2016-02-03 8:15 ` [PATCH V2 3/6] stm class: provision for statically assigned masterIDs Chunyan Zhang ` (3 subsequent siblings) 5 siblings, 3 replies; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc The node name of STM master management policy is a concatenation of an STM device name to which this policy applies and following an arbitrary string, these two strings are concatenated with a dot. This patch adds a loop for extracting the STM device name when an arbitrary number of dot(s) are found in this STM device name. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/hwtracing/stm/policy.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c index 11ab6d0..691686e 100644 --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -321,21 +321,26 @@ stp_policies_make(struct config_group *group, const char *name) /* * node must look like <device_name>.<policy_name>, where * <device_name> is the name of an existing stm device and - * <policy_name> is an arbitrary string + * <policy_name> is an arbitrary string, when an arbitrary + * number of dot(s) are found in the <device_name>, the + * first matched STM device name would be extracted. */ - p = strchr(devname, '.'); - if (!p) { - kfree(devname); - return ERR_PTR(-EINVAL); - } + for (p = devname; ; p++) { + p = strchr(p, '.'); + if (!p) { + kfree(devname); + return ERR_PTR(-EINVAL); + } - *p++ = '\0'; + *p = '\0'; - stm = stm_find_device(devname); - kfree(devname); + stm = stm_find_device(devname); + if (stm) + break; + *p = '.'; + }; - if (!stm) - return ERR_PTR(-ENODEV); + kfree(devname); mutex_lock(&stm->policy_mutex); if (stm->policy) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH] stm class: fix semicolon.cocci warnings 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang @ 2016-02-03 10:05 ` kbuild test robot 2016-02-03 10:05 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name kbuild test robot 2016-02-04 8:56 ` Chunyan Zhang 2 siblings, 0 replies; 27+ messages in thread From: kbuild test robot @ 2016-02-03 10:05 UTC (permalink / raw) To: Chunyan Zhang Cc: kbuild-all, mathieu.poirier, alexander.shishkin, robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc drivers/hwtracing/stm/policy.c:341:2-3: Unneeded semicolon Remove unneeded semicolon. Generated by: scripts/coccinelle/misc/semicolon.cocci CC: Chunyan Zhang <zhang.chunyan@linaro.org> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> --- policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -338,7 +338,7 @@ stp_policies_make(struct config_group *g if (stm) break; *p = '.'; - }; + } kfree(devname); ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang 2016-02-03 10:05 ` [PATCH] stm class: fix semicolon.cocci warnings kbuild test robot @ 2016-02-03 10:05 ` kbuild test robot 2016-02-04 8:56 ` Chunyan Zhang 2 siblings, 0 replies; 27+ messages in thread From: kbuild test robot @ 2016-02-03 10:05 UTC (permalink / raw) To: Chunyan Zhang Cc: kbuild-all, mathieu.poirier, alexander.shishkin, robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Hi Chunyan, [auto build test WARNING on robh/for-next] [also build test WARNING on v4.5-rc2 next-20160203] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Chunyan-Zhang/Introduce-CoreSight-STM-support/20160203-161836 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next coccinelle warnings: (new ones prefixed by >>) >> drivers/hwtracing/stm/policy.c:341:2-3: Unneeded semicolon Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang 2016-02-03 10:05 ` [PATCH] stm class: fix semicolon.cocci warnings kbuild test robot 2016-02-03 10:05 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name kbuild test robot @ 2016-02-04 8:56 ` Chunyan Zhang 2016-02-04 17:30 ` Alexander Shishkin 2 siblings, 1 reply; 27+ messages in thread From: Chunyan Zhang @ 2016-02-04 8:56 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc The node name of STM master management policy is a concatenation of an STM device name to which this policy applies and following an arbitrary string, these two strings are concatenated with a dot. This patch adds a loop for extracting the STM device name when an arbitrary number of dot(s) are found in this STM device name. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/hwtracing/stm/policy.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c index 11ab6d0..691686e 100644 --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -321,21 +321,26 @@ stp_policies_make(struct config_group *group, const char *name) /* * node must look like <device_name>.<policy_name>, where * <device_name> is the name of an existing stm device and - * <policy_name> is an arbitrary string + * <policy_name> is an arbitrary string, when an arbitrary + * number of dot(s) are found in the <device_name>, the + * first matched STM device name would be extracted. */ - p = strchr(devname, '.'); - if (!p) { - kfree(devname); - return ERR_PTR(-EINVAL); - } + for (p = devname; ; p++) { + p = strchr(p, '.'); + if (!p) { + kfree(devname); + return ERR_PTR(-EINVAL); + } - *p++ = '\0'; + *p = '\0'; - stm = stm_find_device(devname); - kfree(devname); + stm = stm_find_device(devname); + if (stm) + break; + *p = '.'; + } - if (!stm) - return ERR_PTR(-ENODEV); + kfree(devname); mutex_lock(&stm->policy_mutex); if (stm->policy) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name 2016-02-04 8:56 ` Chunyan Zhang @ 2016-02-04 17:30 ` Alexander Shishkin 2016-02-05 3:18 ` Chunyan Zhang 0 siblings, 1 reply; 27+ messages in thread From: Alexander Shishkin @ 2016-02-04 17:30 UTC (permalink / raw) To: Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Chunyan Zhang <zhang.chunyan@linaro.org> writes: I few comments below: > The node name of STM master management policy is a concatenation of an > STM device name to which this policy applies and following an arbitrary > string, these two strings are concatenated with a dot. This is true. > This patch adds a loop for extracting the STM device name when an > arbitrary number of dot(s) are found in this STM device name. It's not very easy to tell what's going on here from this description. The reader be left curious as to why an arbitrary number of dots is a reason to run a loop. When in doubt, try to imagine as if you're seeing this patch for the first time and ask yourself, does the message give a clear explanation of what's going on in it. > Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> > --- > drivers/hwtracing/stm/policy.c | 27 ++++++++++++++++----------- > 1 file changed, 16 insertions(+), 11 deletions(-) > > diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c > index 11ab6d0..691686e 100644 > --- a/drivers/hwtracing/stm/policy.c > +++ b/drivers/hwtracing/stm/policy.c > @@ -321,21 +321,26 @@ stp_policies_make(struct config_group *group, const char *name) > /* > * node must look like <device_name>.<policy_name>, where > * <device_name> is the name of an existing stm device and > - * <policy_name> is an arbitrary string > + * <policy_name> is an arbitrary string, when an arbitrary > + * number of dot(s) are found in the <device_name>, the > + * first matched STM device name would be extracted. > */ This leaves room for a number of suspicious situations. What if both "xyz" and "xyz.0" are stm devices, how would you create a policy for the latter, for example? The rules should be such that you can tell exactly what the intended stm device is from a policy directory name, otherwise it's just asking for trouble. > - p = strchr(devname, '.'); > - if (!p) { > - kfree(devname); > - return ERR_PTR(-EINVAL); > - } > + for (p = devname; ; p++) { > + p = strchr(p, '.'); > + if (!p) { > + kfree(devname); > + return ERR_PTR(-EINVAL); > + } > > - *p++ = '\0'; > + *p = '\0'; > > - stm = stm_find_device(devname); > - kfree(devname); > + stm = stm_find_device(devname); > + if (stm) > + break; > + *p = '.'; > + } > > - if (!stm) > - return ERR_PTR(-ENODEV); > + kfree(devname); In the existing code there is a clear distinction between -ENODEV, which is to say "we didn't find the device" and -EINVAL, "directory name breaks rules/is badly formatted". After the change, it's all -EINVAL, which also becomes "we tried everything, sorry". So, having said all that, does the following patch solve your problem: >From 870dc5fefa5623c39552511d31e0fa0da984d581 Mon Sep 17 00:00:00 2001 From: Alexander Shishkin <alexander.shishkin@linux.intel.com> Date: Thu, 4 Feb 2016 18:56:34 +0200 Subject: [PATCH] stm class: Support devices with multiple instances By convention, the name of the stm policy directory in configfs consists of the device name to which it applies and the actual policy name, separated by a dot. Now, some devices already have dots in their names that separate name of the actual device from its instance identifier. Such devices will result in two (or more, who can tell) dots in the policy directory name. Existing policy code, however, will treat the first dot as the one that separates device name from policy name, therefore failing the above case. This patch makes the last dot in the directory name be the separator, thus prohibiting dots from being used in policy names. Suggested-by: Chunyan Zhang <zhang.chunyan@linaro.org> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> --- drivers/hwtracing/stm/policy.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c index 94d3abfb73..1db189657b 100644 --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -332,10 +332,11 @@ stp_policies_make(struct config_group *group, const char *name) /* * node must look like <device_name>.<policy_name>, where - * <device_name> is the name of an existing stm device and - * <policy_name> is an arbitrary string + * <device_name> is the name of an existing stm device; may + * contain dots; + * <policy_name> is an arbitrary string; may not contain dots */ - p = strchr(devname, '.'); + p = strrchr(devname, '.'); if (!p) { kfree(devname); return ERR_PTR(-EINVAL); -- 2.7.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name 2016-02-04 17:30 ` Alexander Shishkin @ 2016-02-05 3:18 ` Chunyan Zhang 0 siblings, 0 replies; 27+ messages in thread From: Chunyan Zhang @ 2016-02-05 3:18 UTC (permalink / raw) To: Alexander Shishkin Cc: Mathieu Poirier, robh, Mark Brown, pratikp, nicolas.guion, Jon Corbet, Mark Rutland, Mike Leach, Tor Jeremiassen, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On Fri, Feb 5, 2016 at 1:30 AM, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > > I few comments below: > >> The node name of STM master management policy is a concatenation of an >> STM device name to which this policy applies and following an arbitrary >> string, these two strings are concatenated with a dot. > > This is true. > >> This patch adds a loop for extracting the STM device name when an >> arbitrary number of dot(s) are found in this STM device name. > > It's not very easy to tell what's going on here from this > description. The reader be left curious as to why an arbitrary number of > dots is a reason to run a loop. When in doubt, try to imagine as if > you're seeing this patch for the first time and ask yourself, does the > message give a clear explanation of what's going on in it. > >> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> >> --- >> drivers/hwtracing/stm/policy.c | 27 ++++++++++++++++----------- >> 1 file changed, 16 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c >> index 11ab6d0..691686e 100644 >> --- a/drivers/hwtracing/stm/policy.c >> +++ b/drivers/hwtracing/stm/policy.c >> @@ -321,21 +321,26 @@ stp_policies_make(struct config_group *group, const char *name) >> /* >> * node must look like <device_name>.<policy_name>, where >> * <device_name> is the name of an existing stm device and >> - * <policy_name> is an arbitrary string >> + * <policy_name> is an arbitrary string, when an arbitrary >> + * number of dot(s) are found in the <device_name>, the >> + * first matched STM device name would be extracted. >> */ > > This leaves room for a number of suspicious situations. What if both > "xyz" and "xyz.0" are stm devices, how would you create a policy for the > latter, for example? > > The rules should be such that you can tell exactly what the intended stm > device is from a policy directory name, otherwise it's just asking for > trouble. > >> - p = strchr(devname, '.'); >> - if (!p) { >> - kfree(devname); >> - return ERR_PTR(-EINVAL); >> - } >> + for (p = devname; ; p++) { >> + p = strchr(p, '.'); >> + if (!p) { >> + kfree(devname); >> + return ERR_PTR(-EINVAL); >> + } >> >> - *p++ = '\0'; >> + *p = '\0'; >> >> - stm = stm_find_device(devname); >> - kfree(devname); >> + stm = stm_find_device(devname); >> + if (stm) >> + break; >> + *p = '.'; >> + } >> >> - if (!stm) >> - return ERR_PTR(-ENODEV); >> + kfree(devname); > > In the existing code there is a clear distinction between -ENODEV, which > is to say "we didn't find the device" and -EINVAL, "directory name > breaks rules/is badly formatted". After the change, it's all -EINVAL, > which also becomes "we tried everything, sorry". > > So, having said all that, does the following patch solve your problem: Yes, I originally modified as well like your following patch, but at that moment, I didn't get your agreement that the policy name (i.e. an arbitrary string) cannot contain dots, so I had to consider the case. Whatever, there isn't a panacea. I'm very good with your patch. Many thanks for your review and providing the patch. Chunyan > > From 870dc5fefa5623c39552511d31e0fa0da984d581 Mon Sep 17 00:00:00 2001 > From: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Date: Thu, 4 Feb 2016 18:56:34 +0200 > Subject: [PATCH] stm class: Support devices with multiple instances > > By convention, the name of the stm policy directory in configfs consists of > the device name to which it applies and the actual policy name, separated > by a dot. Now, some devices already have dots in their names that separate > name of the actual device from its instance identifier. Such devices will > result in two (or more, who can tell) dots in the policy directory name. > > Existing policy code, however, will treat the first dot as the one that > separates device name from policy name, therefore failing the above case. > > This patch makes the last dot in the directory name be the separator, thus > prohibiting dots from being used in policy names. > > Suggested-by: Chunyan Zhang <zhang.chunyan@linaro.org> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> > --- > drivers/hwtracing/stm/policy.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c > index 94d3abfb73..1db189657b 100644 > --- a/drivers/hwtracing/stm/policy.c > +++ b/drivers/hwtracing/stm/policy.c > @@ -332,10 +332,11 @@ stp_policies_make(struct config_group *group, const char *name) > > /* > * node must look like <device_name>.<policy_name>, where > - * <device_name> is the name of an existing stm device and > - * <policy_name> is an arbitrary string > + * <device_name> is the name of an existing stm device; may > + * contain dots; > + * <policy_name> is an arbitrary string; may not contain dots > */ > - p = strchr(devname, '.'); > + p = strrchr(devname, '.'); > if (!p) { > kfree(devname); > return ERR_PTR(-EINVAL); > -- > 2.7.0 > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 1/6] stm class: Add ioctl get_options interface Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-05 12:52 ` Alexander Shishkin 2016-02-03 8:15 ` [PATCH V2 4/6] Documentations: Add explanations of the case for non-configurable masters Chunyan Zhang ` (2 subsequent siblings) 5 siblings, 1 reply; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc From: Mathieu Poirier <mathieu.poirier@linaro.org> Some architecture like ARM assign masterIDs statically at the HW design phase, making masterID manipulation in the generic STM core irrelevant. This patch adds a new 'mstatic' flag to struct stm_data that tells the core that this specific STM device doesn't need explicit masterID management. In the core sw_start/end of masterID are set to '1', i.e there is only one masterID to deal with. Also this patch depends on [1], so that the number of masterID is '1' too. Finally the lower and upper bound for masterIDs as presented in ($SYSFS)/class/stm/XYZ.stm/masters and ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters are set to '-1'. That way users can't confuse them with architecture where masterID management is required (where any other value would be valid). [1] https://lkml.org/lkml/2015/12/22/348 Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- drivers/hwtracing/stm/core.c | 18 ++++++++++++++++-- drivers/hwtracing/stm/policy.c | 19 +++++++++++++++++-- include/linux/stm.h | 8 ++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 86bb4e3..cd3dc19 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -44,9 +44,15 @@ static ssize_t masters_show(struct device *dev, char *buf) { struct stm_device *stm = to_stm_device(dev); - int ret; + int ret, sw_start, sw_end; + + sw_start = stm->data->sw_start; + sw_end = stm->data->sw_end; - ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end); + if (stm->data->mstatic) + sw_start = sw_end = STM_STATIC_MASTERID; + + ret = sprintf(buf, "%d %d\n", sw_start, sw_end); return ret; } @@ -629,7 +635,15 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data, if (!stm_data->packet || !stm_data->sw_nchannels) return -EINVAL; + /* + * MasterIDs are statically set in HW. As such the core is + * using a single master for interaction with this device. + */ + if (stm_data->mstatic) + stm_data->sw_start = stm_data->sw_end = 1; + nmasters = stm_data->sw_end - stm_data->sw_start; + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL); if (!stm) return -ENOMEM; diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c index 691686e..7e70ca2 100644 --- a/drivers/hwtracing/stm/policy.c +++ b/drivers/hwtracing/stm/policy.c @@ -80,10 +80,17 @@ static ssize_t stp_policy_node_masters_show(struct config_item *item, char *page) { struct stp_policy_node *policy_node = to_stp_policy_node(item); + struct stm_device *stm = policy_node->policy->stm; + int first_master, last_master; ssize_t count; - count = sprintf(page, "%u %u\n", policy_node->first_master, - policy_node->last_master); + first_master = policy_node->first_master; + last_master = policy_node->last_master; + + if (stm && stm->data->mstatic) + first_master = last_master = STM_STATIC_MASTERID; + + count = sprintf(page, "%d %d\n", first_master, last_master); return count; } @@ -106,6 +113,13 @@ stp_policy_node_masters_store(struct config_item *item, const char *page, if (!stm) goto unlock; + /* + * masterIDs are statically allocated in HW, no point in trying to + * change their values. + */ + if (stm->data->mstatic) + goto unlock; + /* must be within [sw_start..sw_end], which is an inclusive range */ if (first > INT_MAX || last > INT_MAX || first > last || first < stm->data->sw_start || @@ -325,6 +339,7 @@ stp_policies_make(struct config_group *group, const char *name) * number of dot(s) are found in the <device_name>, the * first matched STM device name would be extracted. */ + for (p = devname; ; p++) { p = strchr(p, '.'); if (!p) { diff --git a/include/linux/stm.h b/include/linux/stm.h index f351d62..c9712a7 100644 --- a/include/linux/stm.h +++ b/include/linux/stm.h @@ -18,6 +18,11 @@ #include <linux/device.h> /** + * The masterIDs are statically set in hardware and can't be queried + */ +#define STM_STATIC_MASTERID -1 + +/** * enum stp_packet_type - STP packets that an STM driver sends */ enum stp_packet_type { @@ -46,6 +51,8 @@ struct stm_device; * struct stm_data - STM device description and callbacks * @name: device name * @stm: internal structure, only used by stm class code + * @mstatic: true if masterIDs are assigned in HW. If so @sw_start + * and @sw_end are set to '1' by the core. * @sw_start: first STP master available to software * @sw_end: last STP master available to software * @sw_nchannels: number of STP channels per master @@ -71,6 +78,7 @@ struct stm_device; struct stm_data { const char *name; struct stm_device *stm; + bool mstatic; unsigned int sw_start; unsigned int sw_end; unsigned int sw_nchannels; -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-03 8:15 ` [PATCH V2 3/6] stm class: provision for statically assigned masterIDs Chunyan Zhang @ 2016-02-05 12:52 ` Alexander Shishkin 2016-02-05 16:31 ` Mike Leach 2016-02-05 18:08 ` Mathieu Poirier 0 siblings, 2 replies; 27+ messages in thread From: Alexander Shishkin @ 2016-02-05 12:52 UTC (permalink / raw) To: Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Chunyan Zhang <zhang.chunyan@linaro.org> writes: > From: Mathieu Poirier <mathieu.poirier@linaro.org> > > Some architecture like ARM assign masterIDs statically at the HW design > phase, making masterID manipulation in the generic STM core irrelevant. > > This patch adds a new 'mstatic' flag to struct stm_data that tells the > core that this specific STM device doesn't need explicit masterID > management. So why do we need this patch? If your STM only has master 42 allocated for software sources, simply set sw_start = 42, sw_end = 42 and you're good to go, software will have exactly one channel to choose from. See also the comment from <linux/stm.h>: * @sw_start: first STP master available to software * @sw_end: last STP master available to software particularly the "available to software" part. Any other kinds of masters the STM class code doesn't care about (yet). > In the core sw_start/end of masterID are set to '1', > i.e there is only one masterID to deal with. This is also a completely arbitrary and unnecessary requirement. Again, you can set both to 42 and it will still work. > Also this patch depends on [1], so that the number of masterID > is '1' too. > > Finally the lower and upper bound for masterIDs as presented > in ($SYSFS)/class/stm/XYZ.stm/masters and > ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters > are set to '-1'. That way users can't confuse them with > architecture where masterID management is required (where any > other value would be valid). Why is this a good idea? Having the actual master there will allow software to know what it is and also tell the decoding side what it is (assuming you have more than one source in the STP stream, it might not be easy to figure out, unless you know it in advance). I don't see how one master statically assigned to software sources is different from two masters or 32 masters. And I don't see any benefit of hiding the master id from userspace. Am I missing something? Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-05 12:52 ` Alexander Shishkin @ 2016-02-05 16:31 ` Mike Leach 2016-02-08 10:52 ` Alexander Shishkin 2016-02-05 18:08 ` Mathieu Poirier 1 sibling, 1 reply; 27+ messages in thread From: Mike Leach @ 2016-02-05 16:31 UTC (permalink / raw) To: Alexander Shishkin, Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, Mark Rutland, tor, Al Grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Hi, I think a quick clarification of the ARM hardware STM architecture may be of value here. The ARM hardware STM, when implemented as recommend in a hardware design, the master IDs are not under driver control, but have a hardwire association with source devices in the system. The master IDs are hardwired to individual cores and core security states, and there could be other IDs associated with hardware trace sources requiring output via the hardware STM. (an example of this is the ARM Juno development board). Therefore in multi-core systems many masters may be associated with a single software STM stream. A user-space application running on Core 1, may have a master ID of 0x11 in the STPv2 trace stream, but if this application is context switched and later continues running on Core 2, then master ID could change to 0x21. Masters identify a hardware source in this scheme, the precise values used will be implementation dependent. So the number of masters "available to the software" depends on your interpretation of the phrase. If you mean "master IDs on which software trace will appear" then that will be many - it depends on which core is running the application. However as described above, this is not predictable by any decoder, and the master set used may not be contiguous. If you mean "master IDs selectable/allocated by the driver" then that will be 0 - the driver has no control, and possibly no knowledge of which master is associated with the current execution context. (this is of course system dependent - it may well be possible to have some configuration database associating cores with hardware IDs, but this will be implementation and board support dependant). Therefore, there is a requirement to tell both the user-space STM client and decoder that not only is master ID management not needed - it is in fact not possible and the key to identify the software source for a given STM packet is the channel alone. In addition we have a nominal single Master ID "available to the software", to satisfy the requirements of the generic ST module API. So on Juno for example, while we do have 128 masters, each with 65536 channels, the master allocation is not visible to system software - each core sees a single master with 65536 channels. Therefore differentiation between software sources in the STM trace is by channel number allocations only. Best Regards Mike. ---------------------------------------------------------------- Mike Leach +44 (0)1254 893911 (Direct) Principal Engineer +44 (0)1254 893900 (Main) Arm Blackburn Design Centre +44 (0)1254 893901 (Fax) Belthorn House Walker Rd mailto:mike.leach@arm.com Guide Blackburn BB1 2QE ---------------------------------------------------------------- > -----Original Message----- > From: Alexander Shishkin [mailto:alexander.shishkin@linux.intel.com] > Sent: 05 February 2016 12:52 > To: Chunyan Zhang; mathieu.poirier@linaro.org > Cc: robh@kernel.org; broonie@kernel.org; pratikp@codeaurora.org; > nicolas.guion@st.com; corbet@lwn.net; Mark Rutland; Mike Leach; > tor@ti.com; Al Grant; zhang.lyra@gmail.com; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-api@vger.kernel.org; linux- > doc@vger.kernel.org > Subject: Re: [PATCH V2 3/6] stm class: provision for statically assigned > masterIDs > > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > > > From: Mathieu Poirier <mathieu.poirier@linaro.org> > > > > Some architecture like ARM assign masterIDs statically at the HW design > > phase, making masterID manipulation in the generic STM core irrelevant. > > > > This patch adds a new 'mstatic' flag to struct stm_data that tells the > > core that this specific STM device doesn't need explicit masterID > > management. > > So why do we need this patch? If your STM only has master 42 allocated > for software sources, simply set sw_start = 42, sw_end = 42 and you're > good to go, software will have exactly one channel to choose from. See > also the comment from <linux/stm.h>: > > * @sw_start: first STP master available to software > * @sw_end: last STP master available to software > > particularly the "available to software" part. Any other kinds of > masters the STM class code doesn't care about (yet). > > > In the core sw_start/end of masterID are set to '1', > > i.e there is only one masterID to deal with. > > This is also a completely arbitrary and unnecessary requirement. Again, > you can set both to 42 and it will still work. > > > Also this patch depends on [1], so that the number of masterID > > is '1' too. > > > > Finally the lower and upper bound for masterIDs as presented > > in ($SYSFS)/class/stm/XYZ.stm/masters and > > ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters > > are set to '-1'. That way users can't confuse them with > > architecture where masterID management is required (where any > > other value would be valid). > > Why is this a good idea? Having the actual master there will allow > software to know what it is and also tell the decoding side what it is > (assuming you have more than one source in the STP stream, it might not > be easy to figure out, unless you know it in advance). > > I don't see how one master statically assigned to software sources is > different from two masters or 32 masters. And I don't see any benefit of > hiding the master id from userspace. Am I missing something? > > Regards, > -- > Alex IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-05 16:31 ` Mike Leach @ 2016-02-08 10:52 ` Alexander Shishkin 0 siblings, 0 replies; 27+ messages in thread From: Alexander Shishkin @ 2016-02-08 10:52 UTC (permalink / raw) To: Mike Leach, Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, Mark Rutland, tor, Al Grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc Mike Leach <Mike.Leach@arm.com> writes: > Hi, > > I think a quick clarification of the ARM hardware STM architecture may be of value here. > > The ARM hardware STM, when implemented as recommend in a hardware design, the master IDs are not under driver control, but have a hardwire association with source devices in the system. The master IDs are hardwired to individual cores and core security states, and there could be other IDs associated with hardware trace sources requiring output via the hardware STM. (an example of this is the ARM Juno development board). > > Therefore in multi-core systems many masters may be associated with a single software STM stream. A user-space application running on Core 1, may have a master ID of 0x11 in the STPv2 trace stream, but if this application is context switched and later continues running on Core 2, then master ID could change to 0x21. Masters identify a hardware source in this scheme, the precise values used will be implementation dependent. > > So the number of masters "available to the software" depends on your interpretation of the phrase. > If you mean "master IDs on which software trace will appear" then that will be many - it depends on which core is running the application. However as described above, this is not predictable by any decoder, and the master set used may not be contiguous. > If you mean "master IDs selectable/allocated by the driver" then that will be 0 - the driver has no control, and possibly no knowledge of which master is associated with the current execution context. (this is of course system dependent - it may well be possible to have some configuration database associating cores with hardware IDs, but this will be implementation and board support dependant). > > Therefore, there is a requirement to tell both the user-space STM client and decoder that not only is master ID management not needed - it is in fact not possible and the key to identify the software source for a given STM packet is the channel alone. In addition we have a nominal single Master ID "available to the software", to satisfy the requirements of the generic ST module API. > > So on Juno for example, while we do have 128 masters, each with 65536 channels, the master allocation is not visible to system software - each core sees a single master with 65536 channels. Therefore differentiation between software sources in the STM trace is by channel number allocations only. Ok, thanks a lot for the detailed explanation. I was confused by the "static" bit in the patch description. It looks like something along the lines of this patch might indeed be in order. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-05 12:52 ` Alexander Shishkin 2016-02-05 16:31 ` Mike Leach @ 2016-02-05 18:08 ` Mathieu Poirier 2016-02-08 13:26 ` Alexander Shishkin 1 sibling, 1 reply; 27+ messages in thread From: Mathieu Poirier @ 2016-02-05 18:08 UTC (permalink / raw) To: Alexander Shishkin Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On 5 February 2016 at 05:52, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > >> From: Mathieu Poirier <mathieu.poirier@linaro.org> >> >> Some architecture like ARM assign masterIDs statically at the HW design >> phase, making masterID manipulation in the generic STM core irrelevant. >> >> This patch adds a new 'mstatic' flag to struct stm_data that tells the >> core that this specific STM device doesn't need explicit masterID >> management. > > So why do we need this patch? If your STM only has master 42 allocated > for software sources, simply set sw_start = 42, sw_end = 42 and you're > good to go, software will have exactly one channel to choose from. See > also the comment from <linux/stm.h>: On ARM each source, i.e entity capable of accessing STM channels, has a different master ID set in HW. We can't assume the IDs are contiguous and from a SW point of view there is no way to probe the values. > > * @sw_start: first STP master available to software > * @sw_end: last STP master available to software > > particularly the "available to software" part. Any other kinds of > masters the STM class code doesn't care about (yet). > >> In the core sw_start/end of masterID are set to '1', >> i.e there is only one masterID to deal with. > > This is also a completely arbitrary and unnecessary requirement. Again, > you can set both to 42 and it will still work. True - any value will do. The important thing to remember is that on ARM, there is only one masterID channel (from an STM core point of view). But we could also proceed differently, see below for more details. > >> Also this patch depends on [1], so that the number of masterID >> is '1' too. >> >> Finally the lower and upper bound for masterIDs as presented >> in ($SYSFS)/class/stm/XYZ.stm/masters and >> ($SYSFS)/../stp-policy/XYZ.stm.my_policy/some_device/masters >> are set to '-1'. That way users can't confuse them with >> architecture where masterID management is required (where any >> other value would be valid). > > Why is this a good idea? Having the actual master there will allow > software to know what it is and also tell the decoding side what it is > (assuming you have more than one source in the STP stream, it might not > be easy to figure out, unless you know it in advance). In the ARM world masterIDs are irrelevant so why bother with them? Writing a '-1' simply highlight this reality. Another way of approaching the problem would be to change sw_start/end to a bitfield that represent the valid masterIDs but in my opinion, it is a lot of code churn for information that isn't used outside of the decoder. Thanks, Mathieu > > I don't see how one master statically assigned to software sources is > different from two masters or 32 masters. And I don't see any benefit of > hiding the master id from userspace. Am I missing something? > > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-05 18:08 ` Mathieu Poirier @ 2016-02-08 13:26 ` Alexander Shishkin 2016-02-08 17:05 ` Mathieu Poirier 0 siblings, 1 reply; 27+ messages in thread From: Alexander Shishkin @ 2016-02-08 13:26 UTC (permalink / raw) To: Mathieu Poirier Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc Mathieu Poirier <mathieu.poirier@linaro.org> writes: > On 5 February 2016 at 05:52, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >> >>> From: Mathieu Poirier <mathieu.poirier@linaro.org> >>> >>> Some architecture like ARM assign masterIDs statically at the HW design >>> phase, making masterID manipulation in the generic STM core irrelevant. >>> >>> This patch adds a new 'mstatic' flag to struct stm_data that tells the >>> core that this specific STM device doesn't need explicit masterID >>> management. >> >> So why do we need this patch? If your STM only has master 42 allocated >> for software sources, simply set sw_start = 42, sw_end = 42 and you're >> good to go, software will have exactly one channel to choose from. See >> also the comment from <linux/stm.h>: > > On ARM each source, i.e entity capable of accessing STM channels, has > a different master ID set in HW. We can't assume the IDs are > contiguous and from a SW point of view there is no way to probe the > values. Ok, it's the 'static' word that got me confused. From Mike's explanation it seems to me that it's the antithesis of static; the master ID assignment is so dynamic that it's not controllable by the software and may or may not reflect core id, power state, phase of the moon, etc. >>> In the core sw_start/end of masterID are set to '1', >>> i.e there is only one masterID to deal with. >> >> This is also a completely arbitrary and unnecessary requirement. Again, >> you can set both to 42 and it will still work. > > True - any value will do. The important thing to remember is that on > ARM, there is only one masterID channel (from an STM core point of > view). But we could also proceed differently, see below for more > details. Well, we have the masters attribute with two numbers in it that define the range of master IDs that the software can choose from. More specifically to this situation: * the number of channel ID spaces available to the SW is $end - $start + 1, that is, in your case, just 1; * the number of master IDs for the SW to choose from is $end - $start; * if $end==$start, their actual numeric value doesn't really matter, either for the policy definition or for the actual writers. This $end==$start situation itself may be ambiguous and can be interpreted either as having just one *static* master ID fixed for all SW writers (what I assumed from your commit message) or as having a floating master ID, which changes of its own accord and is not controllable by software. These two situations are really the same thing from the perspective of the system under tracing. Also, both of these situations should already work if the driver sets both sw_start and sw_end to the same value. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-08 13:26 ` Alexander Shishkin @ 2016-02-08 17:05 ` Mathieu Poirier 2016-02-08 17:44 ` Al Grant 2016-02-12 16:27 ` Alexander Shishkin 0 siblings, 2 replies; 27+ messages in thread From: Mathieu Poirier @ 2016-02-08 17:05 UTC (permalink / raw) To: Alexander Shishkin Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On 8 February 2016 at 06:26, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> On 5 February 2016 at 05:52, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> Chunyan Zhang <zhang.chunyan@linaro.org> writes: >>> >>>> From: Mathieu Poirier <mathieu.poirier@linaro.org> >>>> >>>> Some architecture like ARM assign masterIDs statically at the HW design >>>> phase, making masterID manipulation in the generic STM core irrelevant. >>>> >>>> This patch adds a new 'mstatic' flag to struct stm_data that tells the >>>> core that this specific STM device doesn't need explicit masterID >>>> management. >>> >>> So why do we need this patch? If your STM only has master 42 allocated >>> for software sources, simply set sw_start = 42, sw_end = 42 and you're >>> good to go, software will have exactly one channel to choose from. See >>> also the comment from <linux/stm.h>: >> >> On ARM each source, i.e entity capable of accessing STM channels, has >> a different master ID set in HW. We can't assume the IDs are >> contiguous and from a SW point of view there is no way to probe the >> values. > > Ok, it's the 'static' word that got me confused. From Mike's explanation > it seems to me that it's the antithesis of static; the master ID > assignment is so dynamic that it's not controllable by the software and > may or may not reflect core id, power state, phase of the moon, etc. Mike did write "master IDs are hardwired to individual cores and core security states", which make assignment for one platform very static. On the flip side those will change from one system to another. > >>>> In the core sw_start/end of masterID are set to '1', >>>> i.e there is only one masterID to deal with. >>> >>> This is also a completely arbitrary and unnecessary requirement. Again, >>> you can set both to 42 and it will still work. >> >> True - any value will do. The important thing to remember is that on >> ARM, there is only one masterID channel (from an STM core point of >> view). But we could also proceed differently, see below for more >> details. > > Well, we have the masters attribute with two numbers in it that define > the range of master IDs that the software can choose from. More > specifically to this situation: > > * the number of channel ID spaces available to the SW is > $end - $start + 1, that is, in your case, just 1; > * the number of master IDs for the SW to choose from is $end - $start; > * if $end==$start, their actual numeric value doesn't really matter, > either for the policy definition or for the actual writers. > > This $end==$start situation itself may be ambiguous and can be > interpreted either as having just one *static* master ID fixed for all > SW writers (what I assumed from your commit message) or as having a > floating master ID, which changes of its own accord and is not > controllable by software. Some clarification here. On ARM from a SW point of view $end == $start and that doesn't change (with regards to masterIDs) . The ambiguity comes from the fact that on other platforms where masterID configuration does change and is important, the condition $end == $start could also be valid. >From configFS, how do we tell users when masterIDs are set in HW? That's what I wanted to highlight with the '-1'. Regardless of the solution we choose I think it is important that we inform, at the very least, users when masterIDs don't matter. We could also leave thing as is, kill this patch and document things thoroughly. With the former things are obvious. > > These two situations are really the same thing from the perspective of > the system under tracing. Also, both of these situations should already > work if the driver sets both sw_start and sw_end to the same > value. > > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-08 17:05 ` Mathieu Poirier @ 2016-02-08 17:44 ` Al Grant 2016-02-09 17:06 ` Mathieu Poirier 2016-02-12 15:54 ` Alexander Shishkin 2016-02-12 16:27 ` Alexander Shishkin 1 sibling, 2 replies; 27+ messages in thread From: Al Grant @ 2016-02-08 17:44 UTC (permalink / raw) To: Mathieu Poirier, Alexander Shishkin Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc > Mike did write "master IDs are hardwired to individual cores and core security > states", which make assignment for one platform very static. > On the flip side those will change from one system to another. It depends on your perspective. From the perspective of a userspace process not pinned to a core, the master id will appear to vary dynamically and unpredictably as the thread migrates from one core to another. (That's actually useful if the decoder wants to know where the thread is running at any given point, as it can find that out for free, without the need to track migration events.) On the other hand if you are pinned (e.g. you're the kernel on a particular core, or you're a per-core worker thread in some thread pooling system) then you have a fixed master id, and then you can have one instance per core all using the same range of channel numbers, with the master id indicating the core - this saves on channel space compared to having to give each core its own range of channel space. Al IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-08 17:44 ` Al Grant @ 2016-02-09 17:06 ` Mathieu Poirier 2016-02-12 15:54 ` Alexander Shishkin 1 sibling, 0 replies; 27+ messages in thread From: Mathieu Poirier @ 2016-02-09 17:06 UTC (permalink / raw) To: Al Grant Cc: Alexander Shishkin, Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On 8 February 2016 at 10:44, Al Grant <Al.Grant@arm.com> wrote: >> Mike did write "master IDs are hardwired to individual cores and core security >> states", which make assignment for one platform very static. >> On the flip side those will change from one system to another. > > It depends on your perspective. From the perspective of a userspace > process not pinned to a core, the master id will appear to vary > dynamically and unpredictably as the thread migrates from one > core to another. (That's actually useful if the decoder wants to know > where the thread is running at any given point, as it can find that out > for free, without the need to track migration events.) Right, that's the expected (and desired) behaviour. > > On the other hand if you are pinned (e.g. you're the kernel on a > particular core, or you're a per-core worker thread in some thread > pooling system) then you have a fixed master id, and then you can > have one instance per core all using the same range of channel > numbers, with the master id indicating the core - this saves on > channel space compared to having to give each core its own > range of channel space. >From an STM core and driver point of view channel mapping works the same way - pinning of tasks is a kernel artefact. The problem of representing masterIDs in the STM core for an architecture with HW assigned masterIDs is still unresolved. > > Al > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 27+ messages in thread
* RE: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-08 17:44 ` Al Grant 2016-02-09 17:06 ` Mathieu Poirier @ 2016-02-12 15:54 ` Alexander Shishkin 1 sibling, 0 replies; 27+ messages in thread From: Alexander Shishkin @ 2016-02-12 15:54 UTC (permalink / raw) To: Al Grant, Mathieu Poirier Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc Al Grant <Al.Grant@arm.com> writes: >> Mike did write "master IDs are hardwired to individual cores and core security >> states", which make assignment for one platform very static. >> On the flip side those will change from one system to another. > > It depends on your perspective. From the perspective of a userspace > process not pinned to a core, the master id will appear to vary > dynamically and unpredictably as the thread migrates from one > core to another. (That's actually useful if the decoder wants to know > where the thread is running at any given point, as it can find that out > for free, without the need to track migration events.) > > On the other hand if you are pinned (e.g. you're the kernel on a > particular core, or you're a per-core worker thread in some thread > pooling system) then you have a fixed master id, and then you can > have one instance per core all using the same range of channel > numbers, with the master id indicating the core - this saves on > channel space compared to having to give each core its own > range of channel space. What I understood from Mike's explanation is that basically nothing is fixed from the software perspective (talking about Coresight STM). One doesn't know the master id with which one's data will be tagged, regardless of whether one is cpu-affine or not, because: * even per-core master IDs will be highly implementation dependent and therefore not knowable by the kernel and subsequently userspace, * master IDs may change based on other conditions (besides security states), so even if we knew per-core master IDs for sure, it wouldn't give us the complete picture. So even though from the HW perspective master IDs may be "hardwired" to hardware states, the hardware states themselves are dynamic, so from the SW perspective these master IDs are not static or hardwired at all. If the same mmio write done by software would always result in the exact same master ID tagged to it, that would be more like "static" to me. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-08 17:05 ` Mathieu Poirier 2016-02-08 17:44 ` Al Grant @ 2016-02-12 16:27 ` Alexander Shishkin 2016-02-12 20:33 ` Mathieu Poirier 1 sibling, 1 reply; 27+ messages in thread From: Alexander Shishkin @ 2016-02-12 16:27 UTC (permalink / raw) To: Mathieu Poirier Cc: Chunyan Zhang, robh, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc Mathieu Poirier <mathieu.poirier@linaro.org> writes: > On 8 February 2016 at 06:26, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> This $end==$start situation itself may be ambiguous and can be >> interpreted either as having just one *static* master ID fixed for all >> SW writers (what I assumed from your commit message) or as having a >> floating master ID, which changes of its own accord and is not >> controllable by software. > > Some clarification here. > > On ARM from a SW point of view $end == $start and that doesn't change > (with regards to masterIDs) . The ambiguity comes from the fact that > on other platforms where masterID configuration does change and is > important, the condition $end == $start could also be valid. Yes, that's what I was saying. The thing is, on the system-under-tracing side these two situations are not very different from one another. Master IDs are really just numbers without any semantics attached to them in the sense that they are not covered by the mipi spec or any other standard (to my knowledge). The difference is in the way we map channels to masters. One way is to allocate a distinct set of channels for each master (the way Intel Trace Hub does it); another way is to share the same set of channels between multiple masters. So we can describe this as "hardware implements the same set of channels across multiple masters" or something along those lines. Actually, in the latter scheme of things you can also have multiple masters, at least theoretically. Say, you have masters [0..15], each with distinct set of channels, but depending on hardware state these masters actually end up as $state*16+$masterID in the STP stream. So we might also think about differentiating between the hardware masters (0 though 15 in the above example) and STP masters. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-12 16:27 ` Alexander Shishkin @ 2016-02-12 20:33 ` Mathieu Poirier 2016-02-22 18:01 ` Mathieu Poirier 0 siblings, 1 reply; 27+ messages in thread From: Mathieu Poirier @ 2016-02-12 20:33 UTC (permalink / raw) To: Alexander Shishkin Cc: Chunyan Zhang, Rob Herring, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On 12 February 2016 at 09:27, Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> writes: > >> On 8 February 2016 at 06:26, Alexander Shishkin >> <alexander.shishkin@linux.intel.com> wrote: >>> This $end==$start situation itself may be ambiguous and can be >>> interpreted either as having just one *static* master ID fixed for all >>> SW writers (what I assumed from your commit message) or as having a >>> floating master ID, which changes of its own accord and is not >>> controllable by software. >> >> Some clarification here. >> >> On ARM from a SW point of view $end == $start and that doesn't change >> (with regards to masterIDs) . The ambiguity comes from the fact that >> on other platforms where masterID configuration does change and is >> important, the condition $end == $start could also be valid. > > Yes, that's what I was saying. The thing is, on the system-under-tracing > side these two situations are not very different from one > another. Master IDs are really just numbers without any semantics > attached to them in the sense that they are not covered by the mipi spec > or any other standard (to my knowledge). We are definitely on the same page here, just using slightly different terms. > > The difference is in the way we map channels to masters. One way is to > allocate a distinct set of channels for each master (the way Intel Trace > Hub does it); another way is to share the same set of channels between > multiple masters. We are in total agreement. > So we can describe this as "hardware implements the > same set of channels across multiple masters" or something along those > lines. I suggest "Shared channels"? In the end, that's really what it is... The outstanding issue is still how to represent these to different way of mapping things in the STM core. I suggested a flag, called "mstatic" (but that can be changed), and a representation of '-1' in for masterIDs sysFS. Whether we stick with that or not is irrelevant, I'd be fine with another mechanism. What I am keen on is that from sysFS users can quickly tell which heuristic is enacted on that specific architecture. > > Actually, in the latter scheme of things you can also have multiple > masters, at least theoretically. Say, you have masters [0..15], each > with distinct set of channels, but depending on hardware state these > masters actually end up as $state*16+$masterID in the STP stream. > > So we might also think about differentiating between the hardware > masters (0 though 15 in the above example) and STP masters. I'm not sure I get what you mean here. On ARM the masterIDs assigned in HW, which will depend on the state, will show up in the STP stream. But again, I might be missing your point. Thanks, Mathieu > > Regards, > -- > Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 3/6] stm class: provision for statically assigned masterIDs 2016-02-12 20:33 ` Mathieu Poirier @ 2016-02-22 18:01 ` Mathieu Poirier 0 siblings, 0 replies; 27+ messages in thread From: Mathieu Poirier @ 2016-02-22 18:01 UTC (permalink / raw) To: Alexander Shishkin Cc: Chunyan Zhang, Rob Herring, Mark Brown, Pratik Patel, Nicolas GUION, Jon Corbet, Mark Rutland, Mike Leach, Jeremiassen, Tor, Al Grant, Lyra Zhang, linux-kernel, linux-arm-kernel, linux-api, linux-doc On 12 February 2016 at 13:33, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 12 February 2016 at 09:27, Alexander Shishkin > <alexander.shishkin@linux.intel.com> wrote: >> Mathieu Poirier <mathieu.poirier@linaro.org> writes: >> >>> On 8 February 2016 at 06:26, Alexander Shishkin >>> <alexander.shishkin@linux.intel.com> wrote: >>>> This $end==$start situation itself may be ambiguous and can be >>>> interpreted either as having just one *static* master ID fixed for all >>>> SW writers (what I assumed from your commit message) or as having a >>>> floating master ID, which changes of its own accord and is not >>>> controllable by software. >>> >>> Some clarification here. >>> >>> On ARM from a SW point of view $end == $start and that doesn't change >>> (with regards to masterIDs) . The ambiguity comes from the fact that >>> on other platforms where masterID configuration does change and is >>> important, the condition $end == $start could also be valid. >> >> Yes, that's what I was saying. The thing is, on the system-under-tracing >> side these two situations are not very different from one >> another. Master IDs are really just numbers without any semantics >> attached to them in the sense that they are not covered by the mipi spec >> or any other standard (to my knowledge). > > We are definitely on the same page here, just using slightly different terms. > >> >> The difference is in the way we map channels to masters. One way is to >> allocate a distinct set of channels for each master (the way Intel Trace >> Hub does it); another way is to share the same set of channels between >> multiple masters. > > We are in total agreement. > >> So we can describe this as "hardware implements the >> same set of channels across multiple masters" or something along those >> lines. > > I suggest "Shared channels"? In the end, that's really what it is... > > The outstanding issue is still how to represent these to different way > of mapping things in the STM core. I suggested a flag, called > "mstatic" (but that can be changed), and a representation of '-1' in > for masterIDs sysFS. Whether we stick with that or not is irrelevant, > I'd be fine with another mechanism. What I am keen on is that from > sysFS users can quickly tell which heuristic is enacted on that > specific architecture. Alex, How do you want to proceed with the above? Do you agree with my current proposal or can you think of a better way? Thanks, Mathieu > >> >> Actually, in the latter scheme of things you can also have multiple >> masters, at least theoretically. Say, you have masters [0..15], each >> with distinct set of channels, but depending on hardware state these >> masters actually end up as $state*16+$masterID in the STP stream. >> >> So we might also think about differentiating between the hardware >> masters (0 though 15 in the above example) and STP masters. > > I'm not sure I get what you mean here. On ARM the masterIDs assigned > in HW, which will depend on the state, will show up in the STP stream. > But again, I might be missing your point. > > Thanks, > Mathieu > >> >> Regards, >> -- >> Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH V2 4/6] Documentations: Add explanations of the case for non-configurable masters 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang ` (2 preceding siblings ...) 2016-02-03 8:15 ` [PATCH V2 3/6] stm class: provision for statically assigned masterIDs Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 5/6] coresight-stm: Bindings for System Trace Macrocell Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component Chunyan Zhang 5 siblings, 0 replies; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc For some STM hardware (e.g. ARM CoreSight STM), the masterID associated to a source is set at the hardware level and not user configurable. Since the masterID information isn't available to SW, introducing a new value of -1 to reflect this reality. Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- Documentation/trace/stm.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt index ea035f9..f03bc2b 100644 --- a/Documentation/trace/stm.txt +++ b/Documentation/trace/stm.txt @@ -47,6 +47,12 @@ through 127 in it. Now, any producer (trace source) identifying itself with "user" identification string will be allocated a master and channel from within these ranges. +$ cat /config/stp-policy/dummy_stm.my-policy/user/masters +-1 -1 + +Would indicate the masters for this rule are set in hardware and +not configurable in user space. + These rules can be nested, for example, one can define a rule "dummy" under "user" directory from the example above and this new rule will be used for trace sources with the id string of "user/dummy". -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V2 5/6] coresight-stm: Bindings for System Trace Macrocell 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang ` (3 preceding siblings ...) 2016-02-03 8:15 ` [PATCH V2 4/6] Documentations: Add explanations of the case for non-configurable masters Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component Chunyan Zhang 5 siblings, 0 replies; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc From: Mathieu Poirier <mathieu.poirier@linaro.org> The System Trace Macrocell (STM) is an IP block falling under the CoreSight umbrella. It's main purpose it so expose stimulus channels to any system component for the purpose of information logging. Bindings for this IP block adds a couple of items to the current mandatory definition for CoreSight components. Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- .../devicetree/bindings/arm/coresight.txt | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 62938eb..93147c0c 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -19,6 +19,7 @@ its hardware characteristcs. - "arm,coresight-etm3x", "arm,primecell"; - "arm,coresight-etm4x", "arm,primecell"; - "qcom,coresight-replicator1x", "arm,primecell"; + - "arm,coresight-stm", "arm,primecell"; [1] * reg: physical base address and length of the register set(s) of the component. @@ -36,6 +37,14 @@ its hardware characteristcs. layout using the generic DT graph presentation found in "bindings/graph.txt". +* Additional required properties for System Trace Macrocells (STM): + * reg: along with the physical base address and length of the register + set as described above, another entry is required to describe the + mapping of the extended stimulus port area. + + * reg-names: the only acceptable values are "stm-base" and + "stm-stimulus-base", each corresponding to the areas defined in "reg". + * Required properties for devices that don't show up on the AMBA bus, such as non-configurable replicators: @@ -202,3 +211,22 @@ Example: }; }; }; + +4. STM + stm@20100000 { + compatible = "arm,coresight-stm", "arm,primecell"; + reg = <0 0x20100000 0 0x1000>, + <0 0x28000000 0 0x180000>; + reg-names = "stm-base", "stm-stimulus-base"; + + clocks = <&soc_smc50mhz>; + clock-names = "apb_pclk"; + port { + stm_out_port: endpoint { + remote-endpoint = <&main_funnel_in_port2>; + }; + }; + }; + +[1]. There is currently two version of STM: STM32 and STM500. Both +have the same HW interface and as such don't need an explicit binding name. -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang ` (4 preceding siblings ...) 2016-02-03 8:15 ` [PATCH V2 5/6] coresight-stm: Bindings for System Trace Macrocell Chunyan Zhang @ 2016-02-03 8:15 ` Chunyan Zhang 2016-02-05 13:06 ` Alexander Shishkin 5 siblings, 1 reply; 27+ messages in thread From: Chunyan Zhang @ 2016-02-03 8:15 UTC (permalink / raw) To: mathieu.poirier, alexander.shishkin Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc From: Pratik Patel <pratikp@codeaurora.org> This driver adds support for the STM CoreSight IP block, allowing any system compoment (HW or SW) to log and aggregate messages via a single entity. The CoreSight STM exposes an application defined number of channels called stimulus port. Configuration is done using entries in sysfs and channels made available to userspace via configfs. Signed-off-by: Pratik Patel <pratikp@codeaurora.org> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org> --- .../ABI/testing/sysfs-bus-coresight-devices-stm | 53 ++ Documentation/trace/coresight.txt | 37 +- drivers/hwtracing/coresight/Kconfig | 11 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/coresight-stm.c | 972 +++++++++++++++++++++ include/linux/coresight-stm.h | 6 + include/uapi/linux/coresight-stm.h | 12 + 7 files changed, 1090 insertions(+), 2 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-stm create mode 100644 drivers/hwtracing/coresight/coresight-stm.c create mode 100644 include/linux/coresight-stm.h create mode 100644 include/uapi/linux/coresight-stm.h diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm new file mode 100644 index 0000000..a1d7022 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-stm @@ -0,0 +1,53 @@ +What: /sys/bus/coresight/devices/<memory_map>.stm/enable_source +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Enable/disable tracing on this specific trace macrocell. + Enabling the trace macrocell implies it has been configured + properly and a sink has been identidifed for it. The path + of coresight components linking the source to the sink is + configured and managed automatically by the coresight framework. + +What: /sys/bus/coresight/devices/<memory_map>.stm/hwevent_enable +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Provides access to the HW event enable register, used in + conjunction with HW event bank select register. + +What: /sys/bus/coresight/devices/<memory_map>.stm/hwevent_select +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Gives access to the HW event block select register + (STMHEBSR) in order to configure up to 256 channels. Used in + conjunction with "hwevent_enable" register as described above. + +What: /sys/bus/coresight/devices/<memory_map>.stm/port_enable +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Provides access to the stimlus port enable register + (STMSPER). Used in conjunction with "port_select" described + below. + +What: /sys/bus/coresight/devices/<memory_map>.stm/port_select +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Used to determine which bank of stimulus port bit in + register STMSPER (see above) apply to. + +What: /sys/bus/coresight/devices/<memory_map>.stm/status +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (R) List various control and status registers. The specific + layout and content is driver specific. + +What: /sys/bus/coresight/devices/<memory_map>.stm/traceid +Date: February 2016 +KernelVersion: 4.5 +Contact: Mathieu Poirier <mathieu.poirier@linaro.org> +Description: (RW) Holds the trace ID that will appear in the trace stream + coming from this trace entity. diff --git a/Documentation/trace/coresight.txt b/Documentation/trace/coresight.txt index 0a5c329..a33c88c 100644 --- a/Documentation/trace/coresight.txt +++ b/Documentation/trace/coresight.txt @@ -190,8 +190,8 @@ expected to be accessed and controlled using those entries. Last but not least, "struct module *owner" is expected to be set to reflect the information carried in "THIS_MODULE". -How to use ----------- +How to use the tracer modules +----------------------------- Before trace collection can start, a coresight sink needs to be identify. There is no limit on the amount of sinks (nor sources) that can be enabled at @@ -297,3 +297,36 @@ Info Tracing enabled Instruction 13570831 0x8026B584 E28DD00C false ADD sp,sp,#0xc Instruction 0 0x8026B588 E8BD8000 true LDM sp!,{pc} Timestamp Timestamp: 17107041535 + +How to use the STM module +------------------------- + +Using the System Trace Macrocell module is the same as the tracers - the only +difference is that clients are driving the trace capture rather +than the program flow through the code. + +As with any other CoreSight component, specifics about the STM tracer can be +found in sysfs with more information on each entry being found in [1]: + +root@genericarmv8:~# ls /sys/bus/coresight/devices/20100000.stm +enable_source hwevent_select port_enable subsystem uevent +hwevent_enable mgmt port_select traceid +root@genericarmv8:~# + +Like any other source a sink needs to be identified and the STM enabled before +being used: + +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20010000.etf/enable_sink +root@genericarmv8:~# echo 1 > /sys/bus/coresight/devices/20100000.stm/enable_source + +From there user space applications can request and use channels using the devfs +interface provided for that purpose by the generic STM API: + +root@genericarmv8:~# ls -l /dev/20100000.stm +crw------- 1 root root 10, 61 Jan 3 18:11 /dev/20100000.stm +root@genericarmv8:~# + +Details on how to use the generic STM API can be found here [2]. + +[1]. Documentation/ABI/testing/sysfs-bus-coresight-devices-stm +[2]. Documentation/trace/stm.txt diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index c85935f..f4a8bfa 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -77,4 +77,15 @@ config CORESIGHT_QCOM_REPLICATOR programmable ATB replicator sends the ATB trace stream from the ETB/ETF to the TPIUi and ETR. +config CORESIGHT_STM + bool "CoreSight System Trace Macrocell driver" + depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64 + select CORESIGHT_LINKS_AND_SINKS + select STM + help + This driver provides support for hardware assisted software + instrumentation based tracing. This is primarily used for + logging useful software events or data coming from various entities + in the system, possibly running different OSs + endif diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 99f8e5f..1f6eaec 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o +obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c new file mode 100644 index 0000000..c84f57a --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -0,0 +1,972 @@ +/* Copyright (c) 2015-2016, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Initial implementation by Pratik Patel + * (C) 2014-2015 Pratik Patel <pratikp@codeaurora.org> + * + * Serious refactoring, code cleanup and upgrading to the Coresight upstream + * framework by Mathieu Poirier + * (C) 2015-2016 Mathieu Poirier <mathieu.poirier@linaro.org> + * + * Guaranteed timing and support for various packet type coming from the + * generic STM API by Chunyan Zhang + * (C) 2015-2016 Chunyan Zhang <zhang.chunyan@linaro.org> + */ +#include <linux/amba/bus.h> +#include <linux/bitmap.h> +#include <linux/clk.h> +#include <linux/coresight.h> +#include <linux/coresight-stm.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/pm_runtime.h> +#include <linux/stm.h> + +#include "coresight-priv.h" + +#define STMDMASTARTR 0xc04 +#define STMDMASTOPR 0xc08 +#define STMDMASTATR 0xc0c +#define STMDMACTLR 0xc10 +#define STMDMAIDR 0xcfc +#define STMHEER 0xd00 +#define STMHETER 0xd20 +#define STMHEBSR 0xd60 +#define STMHEMCR 0xd64 +#define STMHEMASTR 0xdf4 +#define STMHEFEAT1R 0xdf8 +#define STMHEIDR 0xdfc +#define STMSPER 0xe00 +#define STMSPTER 0xe20 +#define STMPRIVMASKR 0xe40 +#define STMSPSCR 0xe60 +#define STMSPMSCR 0xe64 +#define STMSPOVERRIDER 0xe68 +#define STMSPMOVERRIDER 0xe6c +#define STMSPTRIGCSR 0xe70 +#define STMTCSR 0xe80 +#define STMTSSTIMR 0xe84 +#define STMTSFREQR 0xe8c +#define STMSYNCR 0xe90 +#define STMAUXCR 0xe94 +#define STMSPFEAT1R 0xea0 +#define STMSPFEAT2R 0xea4 +#define STMSPFEAT3R 0xea8 +#define STMITTRIGGER 0xee8 +#define STMITATBDATA0 0xeec +#define STMITATBCTR2 0xef0 +#define STMITATBID 0xef4 +#define STMITATBCTR0 0xef8 + +#define STM_32_CHANNEL 32 +#define BYTES_PER_CHANNEL 256 +#define STM_TRACE_BUF_SIZE 4096 +#define STM_SW_MASTER_END 127 + +/* Register bit definition */ +#define STMTCSR_BUSY_BIT 23 +/* Reserve the first 10 channels for kernel usage */ +#define STM_CHANNEL_OFFSET 0 + +enum stm_pkt_type { + STM_PKT_TYPE_DATA = 0x98, + STM_PKT_TYPE_FLAG = 0xE8, + STM_PKT_TYPE_TRIG = 0xF8, +}; + +#define stm_channel_addr(drvdata, ch) (drvdata->chs.base + \ + (ch * BYTES_PER_CHANNEL)) +#define stm_channel_off(type, opts) (type & ~opts) + +#ifndef CONFIG_64BIT +static inline void __raw_writeq(u64 val, volatile void __iomem *addr) +{ + asm volatile("strd %1, %0" + : "+Qo" (*(volatile u64 __force *)addr) + : "r" (val)); +} +#undef writeq_relaxed +#define writeq_relaxed(v, c) __raw_writeq((__force u64) cpu_to_le64(v), c) +#endif + +static int boot_nr_channel; + +module_param_named( + boot_nr_channel, boot_nr_channel, int, S_IRUGO +); + +/** + * struct channel_space - central management entity for extended ports + * @base: memory mapped base address where channels start. + * @guaraneed: is the channel delivery guaranteed. + */ +struct channel_space { + void __iomem *base; + unsigned long *guaranteed; +}; + +/** + * struct stm_drvdata - specifics associated to an STM component + * @base: memory mapped base address for this component. + * @dev: the device entity associated to this component. + * @atclk: optional clock for the core parts of the STM. + * @csdev: component vitals needed by the framework. + * @spinlock: only one at a time pls. + * @chs: the channels accociated to this STM. + * @stm: structure associated to the generic STM interface. + * @enable: this STM is being used. + * @traceid: value of the current ID for this component. + * @write_64bit: whether this STM supports 64 bit access. + * @stmsper: settings for register STMSPER. + * @stmspscr: settings for register STMSPSCR. + * @numsp: the total number of stimulus port support by this STM. + * @stmheer: settings for register STMHEER. + * @stmheter: settings for register STMHETER. + * @stmhebsr: settings for register STMHEBSR. + */ +struct stm_drvdata { + void __iomem *base; + struct device *dev; + struct clk *atclk; + struct coresight_device *csdev; + spinlock_t spinlock; + struct channel_space chs; + struct stm_data stm; + bool enable; + u8 traceid; + u32 write_64bit; + u32 stmsper; + u32 stmspscr; + u32 numsp; + u32 stmheer; + u32 stmheter; + u32 stmhebsr; +}; + +static void stm_hwevent_enable_hw(struct stm_drvdata *drvdata) +{ + CS_UNLOCK(drvdata->base); + + writel_relaxed(drvdata->stmhebsr, drvdata->base + STMHEBSR); + writel_relaxed(drvdata->stmheter, drvdata->base + STMHETER); + writel_relaxed(drvdata->stmheer, drvdata->base + STMHEER); + writel_relaxed(0x01 | /* Enable HW event tracing */ + 0x04, /* Error detection on event tracing */ + drvdata->base + STMHEMCR); + + CS_LOCK(drvdata->base); +} + +static void stm_port_enable_hw(struct stm_drvdata *drvdata) +{ + CS_UNLOCK(drvdata->base); + /* ATB trigger enable on direct writes to TRIG locations */ + writel_relaxed(0x10, + drvdata->base + STMSPTRIGCSR); + writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR); + writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER); + + CS_LOCK(drvdata->base); +} + +static void stm_enable_hw(struct stm_drvdata *drvdata) +{ + if (drvdata->stmheer) + stm_hwevent_enable_hw(drvdata); + + stm_port_enable_hw(drvdata); + + CS_UNLOCK(drvdata->base); + + /* 4096 byte between synchronisation packets */ + writel_relaxed(0xFFF, drvdata->base + STMSYNCR); + writel_relaxed((drvdata->traceid << 16 | /* trace id */ + 0x02 | /* timestamp enable */ + 0x01), /* global STM enable */ + drvdata->base + STMTCSR); + + CS_LOCK(drvdata->base); +} + +static int stm_enable(struct coresight_device *csdev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + + pm_runtime_get_sync(drvdata->dev); + + spin_lock(&drvdata->spinlock); + stm_enable_hw(drvdata); + drvdata->enable = true; + spin_unlock(&drvdata->spinlock); + + dev_info(drvdata->dev, "STM tracing enabled\n"); + return 0; +} + +static void stm_hwevent_disable_hw(struct stm_drvdata *drvdata) +{ + CS_UNLOCK(drvdata->base); + + writel_relaxed(0x0, drvdata->base + STMHEMCR); + writel_relaxed(0x0, drvdata->base + STMHEER); + writel_relaxed(0x0, drvdata->base + STMHETER); + + CS_LOCK(drvdata->base); +} + +static void stm_port_disable_hw(struct stm_drvdata *drvdata) +{ + CS_UNLOCK(drvdata->base); + + writel_relaxed(0x0, drvdata->base + STMSPER); + writel_relaxed(0x0, drvdata->base + STMSPTRIGCSR); + + CS_LOCK(drvdata->base); +} + +static void stm_disable_hw(struct stm_drvdata *drvdata) +{ + u32 val; + + CS_UNLOCK(drvdata->base); + + val = readl_relaxed(drvdata->base + STMTCSR); + val &= ~0x1; /* clear global STM enable [0] */ + writel_relaxed(val, drvdata->base + STMTCSR); + + CS_LOCK(drvdata->base); + + stm_port_disable_hw(drvdata); + if (drvdata->stmheer) + stm_hwevent_disable_hw(drvdata); +} + +static void stm_disable(struct coresight_device *csdev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + + spin_lock(&drvdata->spinlock); + stm_disable_hw(drvdata); + drvdata->enable = false; + spin_unlock(&drvdata->spinlock); + + /* Wait until the engine has completely stopped */ + coresight_timeout(drvdata, STMTCSR, STMTCSR_BUSY_BIT, 0); + + pm_runtime_put(drvdata->dev); + + dev_info(drvdata->dev, "STM tracing disabled\n"); +} + +static int stm_trace_id(struct coresight_device *csdev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + + return drvdata->traceid; +} + +static const struct coresight_ops_source stm_source_ops = { + .trace_id = stm_trace_id, + .enable = stm_enable, + .disable = stm_disable, +}; + +static const struct coresight_ops stm_cs_ops = { + .source_ops = &stm_source_ops, +}; + +static int stm_send_64bit(void *addr, const void *data, u32 size) +{ + u64 prepad = 0; + u64 postpad = 0; + char *pad; + u8 off, endoff; + u32 len = size; + + off = (unsigned long)data & 0x7; + + if (off) { + endoff = 8 - off; + pad = (char *)&prepad; + pad += off; + + while (endoff && size) { + *pad++ = *(char *)data++; + endoff--; + size--; + } + writeq_relaxed(prepad, addr); + } + + /* now we are 64bit aligned */ + while (size >= 8) { + writeq_relaxed(*(u64 *)data, addr); + data += 8; + size -= 8; + } + + endoff = 0; + + if (size) { + endoff = 8 - (u8)size; + pad = (char *)&postpad; + + while (size) { + *pad++ = *(char *)data++; + size--; + } + writeq_relaxed(postpad, addr); + } + + return len + off + endoff; +} + +static int stm_send(void *addr, const void *data, u32 size) +{ + u32 len = size; + + if (((unsigned long)data & 0x1) && (size >= 1)) { + writeb_relaxed(*(u8 *)data, addr); + data++; + size--; + } + if (((unsigned long)data & 0x2) && (size >= 2)) { + writew_relaxed(*(u16 *)data, addr); + data += 2; + size -= 2; + } + + /* now we are 32bit aligned */ + while (size >= 4) { + writel_relaxed(*(u32 *)data, addr); + data += 4; + size -= 4; + } + + if (size >= 2) { + writew_relaxed(*(u16 *)data, addr); + data += 2; + size -= 2; + } + if (size >= 1) { + writeb_relaxed(*(u8 *)data, addr); + data++; + size--; + } + + return len; +} + +static int stm_generic_link(struct stm_data *stm_data, + unsigned int master, unsigned int channel) +{ + struct stm_drvdata *drvdata = container_of(stm_data, + struct stm_drvdata, stm); + if (!drvdata || !drvdata->csdev) + return -EINVAL; + + return coresight_enable(drvdata->csdev); +} + +static void stm_generic_unlink(struct stm_data *stm_data, + unsigned int master, unsigned int channel) +{ + struct stm_drvdata *drvdata = container_of(stm_data, + struct stm_drvdata, stm); + if (!drvdata || !drvdata->csdev) + return; + + stm_disable(drvdata->csdev); +} + +static long stm_generic_set_options(struct stm_data *stm_data, + unsigned int master, + unsigned int channel, + unsigned int nr_chans, + unsigned long options) +{ + struct stm_drvdata *drvdata = container_of(stm_data, + struct stm_drvdata, stm); + if (!(drvdata && drvdata->enable)) + return -EINVAL; + + if (channel >= drvdata->numsp) + return -EINVAL; + + switch (options) { + case STM_OPTION_GUARANTEED: + set_bit(channel, drvdata->chs.guaranteed); + break; + + case STM_OPTION_INVARIANT: + clear_bit(channel, drvdata->chs.guaranteed); + break; + + default: + return -EINVAL; + } + + return 0; +} + +static long stm_generic_get_options(struct stm_data *stm_data, + unsigned int master, + unsigned int channel, + unsigned int nr_chans, + u64 *options) +{ + struct stm_drvdata *drvdata = container_of(stm_data, + struct stm_drvdata, stm); + if (!(drvdata && drvdata->enable)) + return -EINVAL; + + if (channel >= drvdata->numsp) + return -EINVAL; + + switch (*options) { + case STM_OPTION_GUARANTEED: + *options = test_bit(channel, drvdata->chs.guaranteed); + break; + + default: + return -EINVAL; + } + + return 0; +} + +static ssize_t stm_generic_packet(struct stm_data *stm_data, + unsigned int master, + unsigned int channel, + unsigned int packet, + unsigned int flags, + unsigned int size, + const unsigned char *payload) +{ + unsigned int len = size; + unsigned long ch_addr; + struct stm_drvdata *drvdata = container_of(stm_data, + struct stm_drvdata, stm); + if (!(drvdata && drvdata->enable)) + return 0; + + if (channel >= drvdata->numsp) + return 0; + + ch_addr = (unsigned long)stm_channel_addr(drvdata, channel); + + flags = (flags == STP_PACKET_TIMESTAMPED) ? STM_FLAG_TIMESTAMPED : 0; + flags |= test_bit(channel, drvdata->chs.guaranteed) ? + STM_FLAG_GUARANTEED : 0; + + switch (packet) { + case STP_PACKET_FLAG: + ch_addr |= stm_channel_off(STM_PKT_TYPE_FLAG, flags); + + /* the generic STM API set a size of zero on flag packets. */ + len = 1; + break; + + case STP_PACKET_DATA: + ch_addr |= stm_channel_off(STM_PKT_TYPE_DATA, flags); + break; + + default: + return 0; + } + + if (drvdata->write_64bit) + return stm_send_64bit((void *)ch_addr, payload, len); + + return stm_send((void *)ch_addr, payload, len); +} + +static ssize_t hwevent_enable_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val = drvdata->stmheer; + + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); +} + +static ssize_t hwevent_enable_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val; + int ret = 0; + + ret = kstrtoul(buf, 16, &val); + if (ret) + return -EINVAL; + + drvdata->stmheer = val; + /* HW event enable and trigger go hand in hand */ + drvdata->stmheter = val; + + return size; +} +static DEVICE_ATTR_RW(hwevent_enable); + +static ssize_t hwevent_select_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val = drvdata->stmhebsr; + + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); +} + +static ssize_t hwevent_select_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val; + int ret = 0; + + ret = kstrtoul(buf, 16, &val); + if (ret) + return -EINVAL; + + drvdata->stmhebsr = val; + + return size; +} +static DEVICE_ATTR_RW(hwevent_select); + +static ssize_t port_select_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val; + + if (!drvdata->enable) { + val = drvdata->stmspscr; + } else { + spin_lock(&drvdata->spinlock); + val = readl_relaxed(drvdata->base + STMSPSCR); + spin_unlock(&drvdata->spinlock); + } + + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); +} + +static ssize_t port_select_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val, stmsper; + int ret = 0; + + ret = kstrtoul(buf, 16, &val); + if (ret) + return ret; + + spin_lock(&drvdata->spinlock); + drvdata->stmspscr = val; + + if (drvdata->enable) { + CS_UNLOCK(drvdata->base); + /* Process as per ARM's TRM recommendation */ + stmsper = readl_relaxed(drvdata->base + STMSPER); + writel_relaxed(0x0, drvdata->base + STMSPER); + writel_relaxed(drvdata->stmspscr, drvdata->base + STMSPSCR); + writel_relaxed(stmsper, drvdata->base + STMSPER); + CS_LOCK(drvdata->base); + } + spin_unlock(&drvdata->spinlock); + + return size; +} +static DEVICE_ATTR_RW(port_select); + +static ssize_t port_enable_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val; + + if (!drvdata->enable) { + val = drvdata->stmsper; + } else { + spin_lock(&drvdata->spinlock); + val = readl_relaxed(drvdata->base + STMSPER); + spin_unlock(&drvdata->spinlock); + } + + return scnprintf(buf, PAGE_SIZE, "%#lx\n", val); +} + +static ssize_t port_enable_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t size) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val; + int ret = 0; + + ret = kstrtoul(buf, 16, &val); + if (ret) + return ret; + + spin_lock(&drvdata->spinlock); + drvdata->stmsper = val; + + if (drvdata->enable) { + CS_UNLOCK(drvdata->base); + writel_relaxed(drvdata->stmsper, drvdata->base + STMSPER); + CS_LOCK(drvdata->base); + } + spin_unlock(&drvdata->spinlock); + + return size; +} +static DEVICE_ATTR_RW(port_enable); + +static ssize_t traceid_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + unsigned long val; + struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent); + + 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); + +#define coresight_simple_func(type, name, offset) \ +static ssize_t name##_show(struct device *_dev, \ + struct device_attribute *attr, char *buf) \ +{ \ + type *drvdata = dev_get_drvdata(_dev->parent); \ + return scnprintf(buf, PAGE_SIZE, "0x%08x\n", \ + readl_relaxed(drvdata->base + offset)); \ +} \ +DEVICE_ATTR_RO(name) + +#define coresight_stm_simple_func(name, offset) \ + coresight_simple_func(struct stm_drvdata, name, offset) + +coresight_stm_simple_func(tcsr, STMTCSR); +coresight_stm_simple_func(tsfreqr, STMTSFREQR); +coresight_stm_simple_func(syncr, STMSYNCR); +coresight_stm_simple_func(sper, STMSPER); +coresight_stm_simple_func(spter, STMSPTER); +coresight_stm_simple_func(privmaskr, STMPRIVMASKR); +coresight_stm_simple_func(spscr, STMSPSCR); +coresight_stm_simple_func(spmscr, STMSPMSCR); +coresight_stm_simple_func(spfeat1r, STMSPFEAT1R); +coresight_stm_simple_func(spfeat2r, STMSPFEAT2R); +coresight_stm_simple_func(spfeat3r, STMSPFEAT3R); +coresight_stm_simple_func(devid, CORESIGHT_DEVID); + +static struct attribute *coresight_stm_attrs[] = { + &dev_attr_hwevent_enable.attr, + &dev_attr_hwevent_select.attr, + &dev_attr_port_enable.attr, + &dev_attr_port_select.attr, + &dev_attr_traceid.attr, + NULL, +}; + +static struct attribute *coresight_stm_mgmt_attrs[] = { + &dev_attr_tcsr.attr, + &dev_attr_tsfreqr.attr, + &dev_attr_syncr.attr, + &dev_attr_sper.attr, + &dev_attr_spter.attr, + &dev_attr_privmaskr.attr, + &dev_attr_spscr.attr, + &dev_attr_spmscr.attr, + &dev_attr_spfeat1r.attr, + &dev_attr_spfeat2r.attr, + &dev_attr_spfeat3r.attr, + &dev_attr_devid.attr, + NULL, +}; + +static const struct attribute_group coresight_stm_group = { + .attrs = coresight_stm_attrs, +}; + +static const struct attribute_group coresight_stm_mgmt_group = { + .attrs = coresight_stm_mgmt_attrs, + .name = "mgmt", +}; + +static const struct attribute_group *coresight_stm_groups[] = { + &coresight_stm_group, + &coresight_stm_mgmt_group, + NULL, +}; + +static int stm_get_resource_byname(struct device_node *np, + char *ch_base, struct resource *res) +{ + const char *name = NULL; + int index = 0, found = 0; + + while (!of_property_read_string_index(np, "reg-names", index, &name)) { + if (strcmp(ch_base, name)) { + index++; + continue; + } + + /* We have a match and @index is where it's at */ + found = 1; + break; + } + + if (!found) + return -EINVAL; + + return of_address_to_resource(np, index, res); +} + +static u32 stm_fundamental_data_size(struct stm_drvdata *drvdata) +{ + u32 stmspfeat2r; + + stmspfeat2r = readl_relaxed(drvdata->base + STMSPFEAT2R); + return BMVAL(stmspfeat2r, 12, 15); +} + +static u32 stm_num_stimulus_port(struct stm_drvdata *drvdata) +{ + u32 numsp; + + numsp = readl_relaxed(drvdata->base + CORESIGHT_DEVID); + /* + * NUMPS in STMDEVID is 17 bit long and if equal to 0x0, + * 32 stimulus ports are supported. + */ + numsp &= 0x1ffff; + if (!numsp) + numsp = STM_32_CHANNEL; + return numsp; +} + +static void stm_init_default_data(struct stm_drvdata *drvdata) +{ + /* Don't use port selection */ + drvdata->stmspscr = 0x0; + /* + * Enable all channel regardless of their number. When port + * selection isn't used (see above) STMSPER applies to all + * 32 channel group available, hence setting all 32 bits to 1 + */ + drvdata->stmsper = ~0x0; + + /* + * Select arbitrary value to start with. If there is a conflict + * with other tracers the framework will deal with it. + */ + drvdata->traceid = 0x20; + + /* Set invariant transaction timing on all channels */ + bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp); +} + +static void stm_init_generic_data(struct stm_drvdata *drvdata) +{ + drvdata->stm.name = dev_name(drvdata->dev); + drvdata->stm.mstatic = true; + drvdata->stm.sw_nchannels = drvdata->numsp; + drvdata->stm.packet = stm_generic_packet; + drvdata->stm.link = stm_generic_link; + drvdata->stm.unlink = stm_generic_unlink; + drvdata->stm.set_options = stm_generic_set_options; + drvdata->stm.get_options = stm_generic_get_options; +} + +static int stm_probe(struct amba_device *adev, const struct amba_id *id) +{ + int ret; + void __iomem *base; + unsigned long *guaranteed; + struct device *dev = &adev->dev; + struct coresight_platform_data *pdata = NULL; + struct stm_drvdata *drvdata; + struct resource *res = &adev->res; + struct resource ch_res; + size_t res_size, bitmap_size; + struct coresight_desc *desc; + struct device_node *np = adev->dev.of_node; + + if (np) { + pdata = of_get_coresight_platform_data(dev, np); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + adev->dev.platform_data = pdata; + } + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + drvdata->dev = &adev->dev; + drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */ + if (!IS_ERR(drvdata->atclk)) { + ret = clk_prepare_enable(drvdata->atclk); + if (ret) + return ret; + } + dev_set_drvdata(dev, drvdata); + + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + drvdata->base = base; + + ret = stm_get_resource_byname(np, "stm-stimulus-base", &ch_res); + if (ret) + return ret; + + base = devm_ioremap_resource(dev, &ch_res); + if (IS_ERR(base)) + return PTR_ERR(base); + drvdata->chs.base = base; + + drvdata->write_64bit = stm_fundamental_data_size(drvdata); + + if (boot_nr_channel) + drvdata->numsp = boot_nr_channel; + else + drvdata->numsp = stm_num_stimulus_port(drvdata); + res_size = min((resource_size_t)(drvdata->numsp * + BYTES_PER_CHANNEL), resource_size(&ch_res)); + drvdata->numsp = res_size / BYTES_PER_CHANNEL; + + bitmap_size = BITS_TO_LONGS(drvdata->numsp) * sizeof(long); + guaranteed = devm_kzalloc(dev, bitmap_size, GFP_KERNEL); + if (!guaranteed) + return -ENOMEM; + drvdata->chs.guaranteed = guaranteed; + + spin_lock_init(&drvdata->spinlock); + + stm_init_default_data(drvdata); + stm_init_generic_data(drvdata); + + if (stm_register_device(dev, &drvdata->stm, THIS_MODULE)) { + dev_info(dev, "stm_register_device failed, probing deffered\n"); + return -EPROBE_DEFER; + } + + desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL); + if (!desc) { + ret = -ENOMEM; + goto stm_unregister; + } + + desc->type = CORESIGHT_DEV_TYPE_SOURCE; + desc->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE; + desc->ops = &stm_cs_ops; + desc->pdata = pdata; + desc->dev = dev; + desc->groups = coresight_stm_groups; + drvdata->csdev = coresight_register(desc); + if (IS_ERR(drvdata->csdev)) { + ret = PTR_ERR(drvdata->csdev); + goto stm_unregister; + } + + pm_runtime_put(&adev->dev); + + dev_info(dev, "%s initialized\n", (char *)id->data); + return 0; + +stm_unregister: + stm_unregister_device(&drvdata->stm); + return ret; +} + +static int stm_remove(struct amba_device *adev) +{ + struct stm_drvdata *drvdata = amba_get_drvdata(adev); + + stm_unregister_device(&drvdata->stm); + coresight_unregister(drvdata->csdev); + return 0; +} + +#ifdef CONFIG_PM +static int stm_runtime_suspend(struct device *dev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata && !IS_ERR(drvdata->atclk)) + clk_disable_unprepare(drvdata->atclk); + + return 0; +} + +static int stm_runtime_resume(struct device *dev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata && !IS_ERR(drvdata->atclk)) + clk_prepare_enable(drvdata->atclk); + + return 0; +} +#endif + +static const struct dev_pm_ops stm_dev_pm_ops = { + SET_RUNTIME_PM_OPS(stm_runtime_suspend, stm_runtime_resume, NULL) +}; + +static struct amba_id stm_ids[] = { + { + .id = 0x0003b962, + .mask = 0x0003ffff, + .data = "STM32", + }, + { 0, 0}, +}; + +static struct amba_driver stm_driver = { + .drv = { + .name = "coresight-stm", + .owner = THIS_MODULE, + .pm = &stm_dev_pm_ops, + }, + .probe = stm_probe, + .remove = stm_remove, + .id_table = stm_ids, +}; + +module_amba_driver(stm_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("CoreSight System Trace Macrocell driver"); diff --git a/include/linux/coresight-stm.h b/include/linux/coresight-stm.h new file mode 100644 index 0000000..a978bb8 --- /dev/null +++ b/include/linux/coresight-stm.h @@ -0,0 +1,6 @@ +#ifndef __LINUX_CORESIGHT_STM_H_ +#define __LINUX_CORESIGHT_STM_H_ + +#include <uapi/linux/coresight-stm.h> + +#endif diff --git a/include/uapi/linux/coresight-stm.h b/include/uapi/linux/coresight-stm.h new file mode 100644 index 0000000..fa35f0b --- /dev/null +++ b/include/uapi/linux/coresight-stm.h @@ -0,0 +1,12 @@ +#ifndef __UAPI_CORESIGHT_STM_H_ +#define __UAPI_CORESIGHT_STM_H_ + +#define STM_FLAG_TIMESTAMPED BIT(3) +#define STM_FLAG_GUARANTEED BIT(7) + +enum { + STM_OPTION_GUARANTEED = 0, + STM_OPTION_INVARIANT, +}; + +#endif -- 1.9.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component 2016-02-03 8:15 ` [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component Chunyan Zhang @ 2016-02-05 13:06 ` Alexander Shishkin 2016-02-05 14:30 ` Arnd Bergmann 0 siblings, 1 reply; 27+ messages in thread From: Alexander Shishkin @ 2016-02-05 13:06 UTC (permalink / raw) To: Chunyan Zhang, mathieu.poirier Cc: robh, broonie, pratikp, nicolas.guion, corbet, mark.rutland, mike.leach, tor, al.grant, zhang.lyra, linux-kernel, linux-arm-kernel, linux-api, linux-doc, Russell King Chunyan Zhang <zhang.chunyan@linaro.org> writes: > +#ifndef CONFIG_64BIT > +static inline void __raw_writeq(u64 val, volatile void __iomem *addr) > +{ > + asm volatile("strd %1, %0" > + : "+Qo" (*(volatile u64 __force *)addr) > + : "r" (val)); > +} Is it really ok to do this for all !64bit arms, inside a driver, just like that? I'm not an expert, but I'm pretty sure there's more to it. Regards, -- Alex ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component 2016-02-05 13:06 ` Alexander Shishkin @ 2016-02-05 14:30 ` Arnd Bergmann 0 siblings, 0 replies; 27+ messages in thread From: Arnd Bergmann @ 2016-02-05 14:30 UTC (permalink / raw) To: linux-arm-kernel Cc: Alexander Shishkin, Chunyan Zhang, mathieu.poirier, mark.rutland, al.grant, corbet, zhang.lyra, linux-doc, linux-kernel, tor, broonie, mike.leach, linux-api, Russell King, pratikp, nicolas.guion On Friday 05 February 2016 15:06:20 Alexander Shishkin wrote: > Chunyan Zhang <zhang.chunyan@linaro.org> writes: > > > +#ifndef CONFIG_64BIT > > +static inline void __raw_writeq(u64 val, volatile void __iomem *addr) > > +{ > > + asm volatile("strd %1, %0" > > + : "+Qo" (*(volatile u64 __force *)addr) > > + : "r" (val)); > > +} > > Is it really ok to do this for all !64bit arms, inside a driver, just > like that? I'm not an expert, but I'm pretty sure there's more to it. It's normally device dependent whether this works or not, on 32-bit architectures, a 64-bit access to an I/O bus tends to get split into two 32 bit accesses and the order might not be the as what was intended. We have functions in include/linux/io-64-nonatomic-hi-lo.h and include/linux/io-64-nonatomic-lo-hi.h that are meant to do this right. Maybe the driver can be changed to use whichever one is correct for it. Arnd ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2016-02-22 18:01 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-03 8:15 [PATCH V2 0/6] Introduce CoreSight STM support Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 1/6] stm class: Add ioctl get_options interface Chunyan Zhang 2016-02-05 12:55 ` Alexander Shishkin 2016-02-03 8:15 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name Chunyan Zhang 2016-02-03 10:05 ` [PATCH] stm class: fix semicolon.cocci warnings kbuild test robot 2016-02-03 10:05 ` [PATCH V2 2/6] stm class: adds a loop to extract the first valid STM device name kbuild test robot 2016-02-04 8:56 ` Chunyan Zhang 2016-02-04 17:30 ` Alexander Shishkin 2016-02-05 3:18 ` Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 3/6] stm class: provision for statically assigned masterIDs Chunyan Zhang 2016-02-05 12:52 ` Alexander Shishkin 2016-02-05 16:31 ` Mike Leach 2016-02-08 10:52 ` Alexander Shishkin 2016-02-05 18:08 ` Mathieu Poirier 2016-02-08 13:26 ` Alexander Shishkin 2016-02-08 17:05 ` Mathieu Poirier 2016-02-08 17:44 ` Al Grant 2016-02-09 17:06 ` Mathieu Poirier 2016-02-12 15:54 ` Alexander Shishkin 2016-02-12 16:27 ` Alexander Shishkin 2016-02-12 20:33 ` Mathieu Poirier 2016-02-22 18:01 ` Mathieu Poirier 2016-02-03 8:15 ` [PATCH V2 4/6] Documentations: Add explanations of the case for non-configurable masters Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 5/6] coresight-stm: Bindings for System Trace Macrocell Chunyan Zhang 2016-02-03 8:15 ` [PATCH V2 6/6] coresight-stm: adding driver for CoreSight STM component Chunyan Zhang 2016-02-05 13:06 ` Alexander Shishkin 2016-02-05 14:30 ` Arnd Bergmann
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).