diff mbox series

[RFC,v3,7/9] coresight: syscfg: Add initial configfs support.

Message ID 20201030175655.30126-8-mike.leach@linaro.org (mailing list archive)
State New, archived
Headers show
Series CoreSight complex config support; ETM strobing | expand

Commit Message

Mike Leach Oct. 30, 2020, 5:56 p.m. UTC
Adds configfs subsystem and attributes to the configuration manager
to enable the listing of loaded configurations and features.

The default values of feature parameters can be accessed and altered
from these attributes to affect all installed devices using the feature.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/Makefile          |   2 +-
 .../hwtracing/coresight/coresight-config.c    |   2 +
 .../hwtracing/coresight/coresight-config.h    |   5 +-
 .../coresight/coresight-syscfg-configfs.c     | 418 ++++++++++++++++++
 .../coresight/coresight-syscfg-configfs.h     |  47 ++
 .../hwtracing/coresight/coresight-syscfg.c    |  31 ++
 .../hwtracing/coresight/coresight-syscfg.h    |   4 +
 7 files changed, 507 insertions(+), 2 deletions(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
 create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h

Comments

Mathieu Poirier Nov. 24, 2020, 8:57 p.m. UTC | #1
On Fri, Oct 30, 2020 at 05:56:53PM +0000, Mike Leach wrote:
> Adds configfs subsystem and attributes to the configuration manager
> to enable the listing of loaded configurations and features.
> 
> The default values of feature parameters can be accessed and altered
> from these attributes to affect all installed devices using the feature.
>

Please no dot (.) at the end of the title.  The same goes for 9/9.
 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/Makefile          |   2 +-
>  .../hwtracing/coresight/coresight-config.c    |   2 +
>  .../hwtracing/coresight/coresight-config.h    |   5 +-
>  .../coresight/coresight-syscfg-configfs.c     | 418 ++++++++++++++++++
>  .../coresight/coresight-syscfg-configfs.h     |  47 ++
>  .../hwtracing/coresight/coresight-syscfg.c    |  31 ++
>  .../hwtracing/coresight/coresight-syscfg.h    |   4 +
>  7 files changed, 507 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> 
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 9de5586cfd1a..4ec226c44f5b 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -5,7 +5,7 @@
>  obj-$(CONFIG_CORESIGHT) += coresight.o
>  coresight-y := coresight-core.o  coresight-etm-perf.o coresight-platform.o \
>  		coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> -		coresight-cfg-preload.o
> +		coresight-cfg-preload.o coresight-syscfg-configfs.o
>  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
>  coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
>  		      coresight-tmc-etr.o
> diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c
> index d911e0f083c1..04e7cb4ff769 100644
> --- a/drivers/hwtracing/coresight/coresight-config.c
> +++ b/drivers/hwtracing/coresight/coresight-config.c
> @@ -111,8 +111,10 @@ static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
>  	 * set the default values for all parameters and regs from the
>  	 * relevant static descriptors.
>  	 */
> +	spin_lock(&feat->desc->param_lock);
>  	for (i = 0; i < feat->nr_params; i++)
>  		feat->params[i].current_value = feat->desc->params[i].value;
> +	spin_unlock(&feat->desc->param_lock);
>  
>  	for (i = 0; i < feat->nr_regs; i++) {
>  		reg_desc = &feat->desc->regs[i];
> diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> index 372f29a59688..39fcac011aa0 100644
> --- a/drivers/hwtracing/coresight/coresight-config.h
> +++ b/drivers/hwtracing/coresight/coresight-config.h
> @@ -7,6 +7,7 @@
>  #ifndef _CORESIGHT_CORESIGHT_CONFIG_H
>  #define _CORESIGHT_CORESIGHT_CONFIG_H
>  
> +#include <linux/configfs.h>

I don't see a need for that header file.

>  #include <linux/coresight.h>
>  #include <linux/types.h>
>  
> @@ -89,6 +90,7 @@ struct cscfg_regval {
>   * @match_flags: matching information if loading into a device
>   * @nr_params:  number of parameters used.
>   * @params:	array of parameters used.
> + * @param_lock: protect params when accessing default values.
>   * @nr_regs:	number of registers used.
>   * @reg:	array of registers used.
>   */
> @@ -99,6 +101,7 @@ struct cscfg_feature_desc {
>  	u32 match_flags;
>  	int nr_params;
>  	struct cscfg_parameter_desc *params;
> +	spinlock_t param_lock;

I'll skip over this for now but all these locks are increasing the potential of
making the lockdep mechanic very angry, if it hasn't already done so.

>  	int nr_regs;
>  	struct cscfg_regval *regs;
>  };
> @@ -239,7 +242,7 @@ struct cscfg_feat_ops {
>   * @ops:		standard ops to enable and disable features on devices.
>   */
>  struct cscfg_feature_csdev {
> -	const struct cscfg_feature_desc *desc;
> +	struct cscfg_feature_desc *desc;

Please don't change what has aleady been added.  Simply skip the 'const' when
you first introduce the structure.

>  	struct coresight_device *csdev;
>  	struct list_head node;
>  	spinlock_t *csdev_spinlock;
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> new file mode 100644
> index 000000000000..ff7ea678100a
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 Linaro Limited, All rights reserved.
> + * Author: Mike Leach <mike.leach@linaro.org>
> + */
> +
> +#include <linux/configfs.h>
> +
> +#include "coresight-syscfg-configfs.h"
> +
> +/*
> + * configfs support for syscfg device.
> + *
> + * config fs is used to manage configurations and features on the
> + * coresight system.
> + */
> +
> +static struct cscfg_device *cscfg_fs_get_cscfg_dev(void);
> +
> +static struct device *cscfg_dev_to_dev(void)
> +{
> +	return cscfg_fs_get_cscfg_dev() ? &cscfg_fs_get_cscfg_dev()->dev : NULL;
> +}
> +
> +/*
> + * Declare the subsystem - with base attributes allowing listing of
> + * loaded configurations
> + */
> +static inline struct cscfg_fs_config *to_cscfg_fs_config_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_config, group);
> +}
> +
> +/* configurations sub-group */
> +
> +/* attributes for the config view group */
> +static ssize_t cscfg_cfg_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_config->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, description);
> +
> +static ssize_t cscfg_cfg_refs_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *desc = fs_config->desc;
> +	ssize_t ch_used = 0;
> +	int i;
> +
> +	if (desc->nr_refs) {
> +		ch_used += scnprintf(page, PAGE_SIZE, "references %d features:-\n", desc->nr_refs);
> +		for (i = 0; i < desc->nr_refs; i++) {
> +			ch_used += scnprintf(page + ch_used, PAGE_SIZE - ch_used,
> +					     "%s\n", desc->refs[i].name);
> +		}
> +	} else
> +		ch_used = scnprintf(page, PAGE_SIZE, "no features referenced\n");
> +	return ch_used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, refs);
> +
> +static ssize_t cscfg_cfg_nr_presets_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->desc->nr_presets);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, nr_presets);
> +
> +static ssize_t cscfg_cfg_preset_values_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *cfg = fs_config->desc;
> +	const struct cscfg_feature_desc *feat;
> +	ssize_t used = 0;
> +	int i, j, val_idx, preset_idx;
> +
> +	if (!fs_config->desc->nr_presets)
> +		return scnprintf(page, PAGE_SIZE, "No presets defined\n");
> +
> +	used = scnprintf(page, PAGE_SIZE, "%d presets, %d parameters per preset\n",
> +			 fs_config->desc->nr_presets, cfg->nr_total_params);
> +
> +	for (preset_idx = 0; preset_idx < fs_config->desc->nr_presets; preset_idx++) {
> +		/* start index on the correct array line */
> +		val_idx = cfg->nr_total_params * preset_idx;
> +
> +		/* preset indexes are used as 1 based by the user */
> +		used += scnprintf(page + used, PAGE_SIZE - used, "preset[%d]: ", preset_idx+1);
> +
> +		/*
> +		 * A set of presets is the sum of all params in used features,
> +		 * in order of declaration of features and params in the features
> +		 */
> +		for (i = 0; i < cfg->nr_refs; i++) {
> +			feat = cscfg_get_named_feat_desc(cfg->refs[i].name);
> +			for (j = 0; j < cfg->refs[i].nr_params; j++) {
> +				used += scnprintf(page + used, PAGE_SIZE - used,
> +						  "%s.%s = 0x%llx ",
> +						  cfg->refs[i].name, feat->params[j].name,
> +						  cfg->presets[val_idx++]);
> +			}
> +		}
> +		used += scnprintf(page + used, PAGE_SIZE - used, "\n");
> +	}
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, preset_values);
> +
> +static struct configfs_attribute *cscfg_config_view_attrs[] = {
> +	&cscfg_cfg_attr_description,
> +	&cscfg_cfg_attr_refs,
> +	&cscfg_cfg_attr_nr_presets,
> +	&cscfg_cfg_attr_preset_values,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_config_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_config_view_attrs,
> +};
> +
> +static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct cscfg_fs_config *cfg_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	cfg_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_config), GFP_KERNEL);
> +	if (!cfg_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cfg_view->desc = cfg_desc;
> +	config_group_init_type_name(&cfg_view->group, cfg_desc->name, &cscfg_config_view_type);
> +	return &cfg_view->group;
> +}
> +
> +static struct config_item_type cscfg_configs_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_configs_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "configurations",
> +			.ci_type = &cscfg_configs_type,
> +		},
> +	},
> +};
> +
> +

Extra line

> +/* attributes for features view */
> +
> +static inline struct cscfg_fs_feature *to_cscfg_fs_feature_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_feature, group);
> +}
> +
> +static ssize_t cscfg_feat_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_feat->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, description);
> +
> +static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +	u32 match_flags = fs_feat->desc->match_flags;
> +	int used = 0;
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ALL)
> +		used = scnprintf(page, PAGE_SIZE, "SRC_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_ALL)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_CPU)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_CPU ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_SYS)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_SYS ");
> +
> +	used += scnprintf(page + used, PAGE_SIZE - used, "\n");
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, matches);
> +
> +static ssize_t cscfg_feat_nr_params_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_feat->desc->nr_params);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, nr_params);
> +
> +/* base feature desc attrib structures */
> +static struct configfs_attribute *cscfg_feature_view_attrs[] = {
> +	&cscfg_feat_attr_description,
> +	&cscfg_feat_attr_matches,
> +	&cscfg_feat_attr_nr_params,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_feature_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_feature_view_attrs,
> +};
> +
> +static inline struct cscfg_fs_param *to_cscfg_fs_param_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_param, group);
> +}
> +
> +static ssize_t cscfg_param_value_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "0x%llx\n", param_item->param->value);
> +}
> +
> +static ssize_t cscfg_param_value_store(struct config_item *item,
> +					       const char *page, size_t size)
> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
> +	u64 val;
> +	int err;
> +
> +	err = kstrtoull(page, 0, &val);
> +	if (err)
> +		return err;
> +
> +	spin_lock(&param_item->desc->param_lock);
> +	param_item->param->value = val;
> +	spin_unlock(&param_item->desc->param_lock);
> +
> +	return size;
> +}
> +CONFIGFS_ATTR(cscfg_param_, value);
> +
> +static struct configfs_attribute *cscfg_param_view_attrs[] = {
> +	&cscfg_param_attr_value,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_param_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_param_view_attrs,
> +};
> +
> +static struct config_item_type cscfg_feat_param_group_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +/*
> + * configfs has far less functionality provided to add attributes dynamically than sysfs,
> + * and the show and store fns pass the enclosing config_item so the actual attribute cannot
> + * be determined. Therefore we add each item as a group directory, with a value attribute.
> + */
> +static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
> +					   struct config_group *params_group)
> +{
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_param *param_item;
> +	int i;
> +
> +	/* parameter items - as groups with default_value attribute */
> +	for (i = 0; i < feat_desc->nr_params; i++) {
> +		param_item = devm_kzalloc(dev, sizeof(struct cscfg_fs_param), GFP_KERNEL);
> +		if (!param_item)
> +			return -ENOMEM;
> +		param_item->desc = feat_desc;
> +		param_item->param = &feat_desc->params[i];
> +		config_group_init_type_name(&param_item->group, param_item->param->name,
> +					    &cscfg_param_view_type);
> +		configfs_add_default_group(&param_item->group, params_group);
> +	}
> +	return 0;
> +}
> +
> +static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct cscfg_fs_feature *feat_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_def_group *params_group = NULL;
> +	int item_err;
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	feat_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +	if (!feat_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (feat_desc->nr_params) {
> +		params_group = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +		if (!params_group)
> +			return ERR_PTR(-ENOMEM);
> +	}
> +
> +	feat_view->desc = feat_desc;
> +	config_group_init_type_name(&feat_view->group,
> +				    feat_desc->name,
> +				    &cscfg_feature_view_type);
> +	if (params_group) {
> +		config_group_init_type_name(&params_group->group, "params",
> +					    &cscfg_feat_param_group_type);
> +		configfs_add_default_group(&params_group->group, &feat_view->group);
> +		item_err = cscfg_create_params_group_items(feat_desc, &params_group->group);
> +		if (item_err)
> +			return ERR_PTR(item_err);
> +	}
> +	return &feat_view->group;
> +}
> +
> +static struct config_item_type cscfg_features_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_features_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "features",
> +			.ci_type = &cscfg_features_type,
> +		},
> +	},

Can we do the following to avoid declaring cscfg_features_type:

static struct cscfg_fs_def_group cscfg_features_grp = {
	.group = {
                .cg_item = {
                        .ci_namebuf = "features",
                        .ci_type = {
                                .ct_owner = THIS_MODULE,
                        };
                },
        },

Same for configuration.

> +};
> +
> +/* add configuration to configurations group */
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_config_group(cfg_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_configs_grp.group, new_group);
> +	return err;
> +}
> +
> +/* add feature to features group */
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_feature_group(feat_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_features_grp.group, new_group);
> +	return err;
> +}
> +
> +static const struct config_item_type syscfg_fs_type = {
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_subsys syscfg_fs_subsys = {
> +	.cfgdev = NULL,
> +	.fs_subsys = {
> +		.su_group = {
> +			.cg_item = {
> +				.ci_namebuf = "coresight-syscfg",
> +				.ci_type = &syscfg_fs_type,
> +			},
> +		},
> +	},
> +};
> +
> +static inline struct cscfg_device *cscfg_fs_get_cscfg_dev(void)
> +{
> +	return syscfg_fs_subsys.cfgdev;
> +}
> +
> +int cscfg_configfs_init(struct cscfg_device *cscfg_dev)
> +{
> +	int ret = 0;
> +	struct configfs_subsystem *subsys = &syscfg_fs_subsys.fs_subsys;
> +
> +	if (!cscfg_dev)
> +		return -EINVAL;
> +
> +	config_group_init(&subsys->su_group);
> +	mutex_init(&subsys->su_mutex);
> +
> +	/* Add default groups to subsystem */
> +	config_group_init(&cscfg_configs_grp.group);
> +	configfs_add_default_group(&cscfg_configs_grp.group, &subsys->su_group);
> +
> +	config_group_init(&cscfg_features_grp.group);
> +	configfs_add_default_group(&cscfg_features_grp.group, &subsys->su_group);
> +
> +	ret = configfs_register_subsystem(subsys);
> +	if (!ret) {
> +		syscfg_fs_subsys.cfgdev = cscfg_dev;
> +		cscfg_dev->cfgfs_subsys = &syscfg_fs_subsys;
> +	}

Instead of doing the above I suggest to make cscfg_device::cfgfs_subsys a member
rather than a pointer and call containerof() to get back to the cscfg_device.
That way we avoid the static declaration of cscfg_configs_grp and
cscfg_configs_type above.

Moreover, now that this patchset is sinking in I would rename struct
cscfg_device to cscfg_manager.

> +
> +	return ret;
> +}
> +
> +void cscfg_configfs_release(struct cscfg_device *cscfg_dev)
> +{
> +	if (cscfg_dev && cscfg_dev->cfgfs_subsys) {
> +		configfs_unregister_subsystem(&cscfg_dev->cfgfs_subsys->fs_subsys);
> +		cscfg_dev->cfgfs_subsys = NULL;
> +	}
> +	syscfg_fs_subsys.cfgdev = NULL;
> +}
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> new file mode 100644
> index 000000000000..70cc3745649e
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Coresight system configuration driver - support for configfs.
> + */
> +
> +#ifndef CORESIGHT_SYSCFG_CONFIGFS_H
> +#define CORESIGHT_SYSCFG_CONFIGFS_H
> +
> +#include <linux/configfs.h>
> +#include "coresight-syscfg.h"
> +
> +/* container to associate the device and configfs subsystem */
> +struct cscfg_fs_subsys {
> +	struct cscfg_device *cfgdev;
> +	struct configfs_subsystem fs_subsys;
> +};
> +
> +/* container for default groups */
> +struct cscfg_fs_def_group {
> +	struct config_group group;
> +};

Please don't do that - there is enough new structures in this patchset as it is.

> +
> +/* container for configuration view */
> +struct cscfg_fs_config {
> +	struct cscfg_config_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for feature view */
> +struct cscfg_fs_feature {
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for parameter view */
> +struct cscfg_fs_param {
> +	struct cscfg_parameter_desc *param;
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +int cscfg_configfs_init(struct cscfg_device *dev);
> +void cscfg_configfs_release(struct cscfg_device *dev);
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc);
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
> +
> +#endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 8d52580daf04..2cf67a038cc8 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -9,6 +9,7 @@
>  #include "coresight-config.h"
>  #include "coresight-etm-perf.h"
>  #include "coresight-syscfg.h"
> +#include "coresight-syscfg-configfs.h"
>  
>  /*
>   * cscfg_ API manages configurations and features for the entire coresight
> @@ -260,6 +261,8 @@ static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
>  {
>  	int err;
>  
> +	spin_lock_init(&feat_desc->param_lock);
> +
>  	/* add feature to any matching registered devices */
>  	err = cscfg_add_feat_to_csdevs(feat_desc);
>  	if (err)
> @@ -294,6 +297,24 @@ static int cscfg_load_config(struct cscfg_config_desc *cfg_desc)
>  	return err;
>  }
>  
> +/* get a feature descriptor by name */
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name)
> +{
> +	const struct cscfg_feature_desc *feat = NULL, *feat_item;
> +
> +	mutex_lock(&cscfg_mutex);
> +
> +	list_for_each_entry(feat_item, &cscfg_data.feat_list, item) {
> +		if (strcmp(feat_item->name, name) == 0) {
> +			feat = feat_item;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&cscfg_mutex);
> +	return feat;
> +}
> +
>  /*
>   * External API function to load feature and config sets.
>   * Take a 0 terminated array of feature descriptors and/or configuration
> @@ -316,6 +337,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       feat_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_feature(feat_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -330,6 +352,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       cfg_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_config(cfg_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -502,6 +525,7 @@ struct device *to_device_cscfg(void)
>  /* Must have a release function or the kernel will complain on module unload */
>  void cscfg_dev_release(struct device *dev)
>  {
> +	cscfg_configfs_release(cscfg_dev);
>  	kfree(cscfg_dev);
>  	cscfg_dev = NULL;
>  }
> @@ -530,6 +554,7 @@ int cscfg_create_device(void)
>  	cscfg_dev->cfg_data = &cscfg_data;
>  
>  	err = device_register(dev);
> +

Extra line

>  	if (err)
>  		cscfg_dev_release(dev);
>  
> @@ -587,6 +612,10 @@ int __init cscfg_init(void)
>  	if (err)
>  		goto exit_drv_unregister;
>  
> +	err = cscfg_configfs_init(cscfg_dev);
> +	if (err)
> +		goto exit_dev_clear;
> +
>  	INIT_LIST_HEAD(&cscfg_data.dev_list);
>  	INIT_LIST_HEAD(&cscfg_data.feat_list);
>  	INIT_LIST_HEAD(&cscfg_data.config_list);
> @@ -595,6 +624,8 @@ int __init cscfg_init(void)
>  	dev_info(to_device_cscfg(), "CoreSight Configuration manager initialised");
>  	return 0;
>  
> +exit_dev_clear:
> +	cscfg_clear_device();
>  exit_drv_unregister:
>  	cscfg_driver_exit();
>  exit_bus_unregister:
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index e8f352599dd7..ce237a69677b 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -6,6 +6,7 @@
>  #ifndef CORESIGHT_SYSCFG_H
>  #define CORESIGHT_SYSCFG_H
>  
> +#include <linux/configfs.h>

Same here, I don't see what this header file is adding.

I'm out of time for today and will continue with the rest of this patch
tomorrow.

Thanks,
Mathieu

>  #include <linux/coresight.h>
>  #include <linux/device.h>
>  
> @@ -46,6 +47,7 @@ struct cscfg_csdev_register {
>  int __init cscfg_init(void);
>  void __exit cscfg_exit(void);
>  int cscfg_preload(void);
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name);
>  
>  /* syscfg external API */
>  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> @@ -64,10 +66,12 @@ void cscfg_unregister_csdev(struct coresight_device *csdev);
>   *
>   * @dev:	The device.
>   * @cfg_data:	A reference to the configuration and feature lists
> + * @cfgfs_subsys: configfs subsystem used to manage configurations.
>   */
>  struct cscfg_device {
>  	struct device dev;
>  	struct cscfg_api_data *cfg_data;
> +	struct cscfg_fs_subsys *cfgfs_subsys;
>  };
>  
>  /* basic device driver */
> -- 
> 2.17.1
>
Mathieu Poirier Nov. 25, 2020, 9:52 p.m. UTC | #2
On Fri, Oct 30, 2020 at 05:56:53PM +0000, Mike Leach wrote:
> Adds configfs subsystem and attributes to the configuration manager
> to enable the listing of loaded configurations and features.
> 
> The default values of feature parameters can be accessed and altered
> from these attributes to affect all installed devices using the feature.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/Makefile          |   2 +-
>  .../hwtracing/coresight/coresight-config.c    |   2 +
>  .../hwtracing/coresight/coresight-config.h    |   5 +-
>  .../coresight/coresight-syscfg-configfs.c     | 418 ++++++++++++++++++
>  .../coresight/coresight-syscfg-configfs.h     |  47 ++
>  .../hwtracing/coresight/coresight-syscfg.c    |  31 ++
>  .../hwtracing/coresight/coresight-syscfg.h    |   4 +
>  7 files changed, 507 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> 
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 9de5586cfd1a..4ec226c44f5b 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -5,7 +5,7 @@
>  obj-$(CONFIG_CORESIGHT) += coresight.o
>  coresight-y := coresight-core.o  coresight-etm-perf.o coresight-platform.o \
>  		coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> -		coresight-cfg-preload.o
> +		coresight-cfg-preload.o coresight-syscfg-configfs.o
>  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
>  coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
>  		      coresight-tmc-etr.o
> diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c
> index d911e0f083c1..04e7cb4ff769 100644
> --- a/drivers/hwtracing/coresight/coresight-config.c
> +++ b/drivers/hwtracing/coresight/coresight-config.c
> @@ -111,8 +111,10 @@ static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
>  	 * set the default values for all parameters and regs from the
>  	 * relevant static descriptors.
>  	 */
> +	spin_lock(&feat->desc->param_lock);
>  	for (i = 0; i < feat->nr_params; i++)
>  		feat->params[i].current_value = feat->desc->params[i].value;
> +	spin_unlock(&feat->desc->param_lock);
>  
>  	for (i = 0; i < feat->nr_regs; i++) {
>  		reg_desc = &feat->desc->regs[i];
> diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> index 372f29a59688..39fcac011aa0 100644
> --- a/drivers/hwtracing/coresight/coresight-config.h
> +++ b/drivers/hwtracing/coresight/coresight-config.h
> @@ -7,6 +7,7 @@
>  #ifndef _CORESIGHT_CORESIGHT_CONFIG_H
>  #define _CORESIGHT_CORESIGHT_CONFIG_H
>  
> +#include <linux/configfs.h>
>  #include <linux/coresight.h>
>  #include <linux/types.h>
>  
> @@ -89,6 +90,7 @@ struct cscfg_regval {
>   * @match_flags: matching information if loading into a device
>   * @nr_params:  number of parameters used.
>   * @params:	array of parameters used.
> + * @param_lock: protect params when accessing default values.
>   * @nr_regs:	number of registers used.
>   * @reg:	array of registers used.
>   */
> @@ -99,6 +101,7 @@ struct cscfg_feature_desc {
>  	u32 match_flags;
>  	int nr_params;
>  	struct cscfg_parameter_desc *params;
> +	spinlock_t param_lock;
>  	int nr_regs;
>  	struct cscfg_regval *regs;
>  };
> @@ -239,7 +242,7 @@ struct cscfg_feat_ops {
>   * @ops:		standard ops to enable and disable features on devices.
>   */
>  struct cscfg_feature_csdev {
> -	const struct cscfg_feature_desc *desc;
> +	struct cscfg_feature_desc *desc;
>  	struct coresight_device *csdev;
>  	struct list_head node;
>  	spinlock_t *csdev_spinlock;
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> new file mode 100644
> index 000000000000..ff7ea678100a
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 Linaro Limited, All rights reserved.
> + * Author: Mike Leach <mike.leach@linaro.org>
> + */
> +
> +#include <linux/configfs.h>
> +
> +#include "coresight-syscfg-configfs.h"
> +
> +/*
> + * configfs support for syscfg device.
> + *
> + * config fs is used to manage configurations and features on the
> + * coresight system.
> + */
> +
> +static struct cscfg_device *cscfg_fs_get_cscfg_dev(void);
> +
> +static struct device *cscfg_dev_to_dev(void)
> +{
> +	return cscfg_fs_get_cscfg_dev() ? &cscfg_fs_get_cscfg_dev()->dev : NULL;
> +}
> +
> +/*
> + * Declare the subsystem - with base attributes allowing listing of
> + * loaded configurations
> + */
> +static inline struct cscfg_fs_config *to_cscfg_fs_config_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_config, group);

Please just call container_of() when needed, it is much easier to understand.

> +}
> +
> +/* configurations sub-group */
> +
> +/* attributes for the config view group */
> +static ssize_t cscfg_cfg_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_config->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, description);
> +
> +static ssize_t cscfg_cfg_refs_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *desc = fs_config->desc;
> +	ssize_t ch_used = 0;
> +	int i;
> +
> +	if (desc->nr_refs) {
> +		ch_used += scnprintf(page, PAGE_SIZE, "references %d features:-\n", desc->nr_refs);

I would remove the above as it makes things harder to parse from a script.

> +		for (i = 0; i < desc->nr_refs; i++) {
> +			ch_used += scnprintf(page + ch_used, PAGE_SIZE - ch_used,
> +					     "%s\n", desc->refs[i].name);
> +		}
> +	} else
> +		ch_used = scnprintf(page, PAGE_SIZE, "no features referenced\n");
> +	return ch_used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, refs);
> +
> +static ssize_t cscfg_cfg_nr_presets_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->desc->nr_presets);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, nr_presets);

I wouldn't bother showing the presets - that information is already available in
"preset_values".

> +
> +static ssize_t cscfg_cfg_preset_values_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *cfg = fs_config->desc;
> +	const struct cscfg_feature_desc *feat;
> +	ssize_t used = 0;
> +	int i, j, val_idx, preset_idx;
> +
> +	if (!fs_config->desc->nr_presets)
> +		return scnprintf(page, PAGE_SIZE, "No presets defined\n");

Pseudo file systems don't show anything when no entries are available.

> +
> +	used = scnprintf(page, PAGE_SIZE, "%d presets, %d parameters per preset\n",
> +			 fs_config->desc->nr_presets, cfg->nr_total_params);

Same here, pseudo file systems are usually not chatty. 

> +
> +	for (preset_idx = 0; preset_idx < fs_config->desc->nr_presets; preset_idx++) {
> +		/* start index on the correct array line */
> +		val_idx = cfg->nr_total_params * preset_idx;
> +
> +		/* preset indexes are used as 1 based by the user */
> +		used += scnprintf(page + used, PAGE_SIZE - used, "preset[%d]: ", preset_idx+1);
> +
> +		/*
> +		 * A set of presets is the sum of all params in used features,
> +		 * in order of declaration of features and params in the features
> +		 */
> +		for (i = 0; i < cfg->nr_refs; i++) {
> +			feat = cscfg_get_named_feat_desc(cfg->refs[i].name);
> +			for (j = 0; j < cfg->refs[i].nr_params; j++) {
> +				used += scnprintf(page + used, PAGE_SIZE - used,
> +						  "%s.%s = 0x%llx ",
> +						  cfg->refs[i].name, feat->params[j].name,
> +						  cfg->presets[val_idx++]);
> +			}
> +		}
> +		used += scnprintf(page + used, PAGE_SIZE - used, "\n");

I'm weary the above doesn't scale well when multiple features, each with their
own parameters are present.  Here is a suggestion:

root@linaro:/sys/kernel/config/coresight-syscfg/configurations/autofdo# cat presets
preset0 preset1 preset2

root@linaro:/sys/kernel/config/coresight-syscfg/configurations/autofdo# cd preset1
root@linaro:/sys/kernel/config/coresight-syscfg/configurations/autofdo/preset1# ls
strobing
root@linaro:/sys/kernel/config/coresight-syscfg/configurations/autofdo/preset1# cd strobing
root@linaro:/sys/kernel/config/coresight-syscfg/configurations/autofdo/preset1/strobing# ls
window period

It is more cumbersome but guaranteed to work with any number of features and
parameters.

> +	}
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, preset_values);
> +
> +static struct configfs_attribute *cscfg_config_view_attrs[] = {
> +	&cscfg_cfg_attr_description,
> +	&cscfg_cfg_attr_refs,
> +	&cscfg_cfg_attr_nr_presets,
> +	&cscfg_cfg_attr_preset_values,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_config_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_config_view_attrs,
> +};
> +
> +static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct cscfg_fs_config *cfg_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	cfg_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_config), GFP_KERNEL);
> +	if (!cfg_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cfg_view->desc = cfg_desc;
> +	config_group_init_type_name(&cfg_view->group, cfg_desc->name, &cscfg_config_view_type);
> +	return &cfg_view->group;
> +}
> +
> +static struct config_item_type cscfg_configs_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_configs_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "configurations",
> +			.ci_type = &cscfg_configs_type,
> +		},
> +	},
> +};
> +
> +
> +/* attributes for features view */
> +
> +static inline struct cscfg_fs_feature *to_cscfg_fs_feature_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_feature, group);
> +}
> +
> +static ssize_t cscfg_feat_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_feat->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, description);
> +
> +static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +	u32 match_flags = fs_feat->desc->match_flags;
> +	int used = 0;
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ALL)
> +		used = scnprintf(page, PAGE_SIZE, "SRC_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_ALL)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_CPU)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_CPU ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_SYS)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_SYS ");
> +
> +	used += scnprintf(page + used, PAGE_SIZE - used, "\n");
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, matches);
> +
> +static ssize_t cscfg_feat_nr_params_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_feat->desc->nr_params);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, nr_params);
> +
> +/* base feature desc attrib structures */
> +static struct configfs_attribute *cscfg_feature_view_attrs[] = {
> +	&cscfg_feat_attr_description,
> +	&cscfg_feat_attr_matches,
> +	&cscfg_feat_attr_nr_params,

Did you have a specific need for nr_params?  To me it can be removed.

> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_feature_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_feature_view_attrs,
> +};
> +
> +static inline struct cscfg_fs_param *to_cscfg_fs_param_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_param, group);
> +}
> +
> +static ssize_t cscfg_param_value_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "0x%llx\n", param_item->param->value);
> +}
> +
> +static ssize_t cscfg_param_value_store(struct config_item *item,
> +					       const char *page, size_t size)
> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));

I would much rather see a direct call to container_of() than a new function that
calls container_of()...

> +	u64 val;
> +	int err;
> +
> +	err = kstrtoull(page, 0, &val);
> +	if (err)
> +		return err;
> +
> +	spin_lock(&param_item->desc->param_lock);
> +	param_item->param->value = val;
> +	spin_unlock(&param_item->desc->param_lock);
> +
> +	return size;
> +}
> +CONFIGFS_ATTR(cscfg_param_, value);
> +
> +static struct configfs_attribute *cscfg_param_view_attrs[] = {
> +	&cscfg_param_attr_value,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_param_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_param_view_attrs,
> +};
> +
> +static struct config_item_type cscfg_feat_param_group_type = {
> +	.ct_owner = THIS_MODULE,

Out of curiosity, what happens if .ct_owner isn't set to THIS_MODULE?

Thanks,
Mathieu

> +};
> +
> +/*
> + * configfs has far less functionality provided to add attributes dynamically than sysfs,
> + * and the show and store fns pass the enclosing config_item so the actual attribute cannot
> + * be determined. Therefore we add each item as a group directory, with a value attribute.
> + */
> +static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
> +					   struct config_group *params_group)
> +{
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_param *param_item;
> +	int i;
> +
> +	/* parameter items - as groups with default_value attribute */
> +	for (i = 0; i < feat_desc->nr_params; i++) {
> +		param_item = devm_kzalloc(dev, sizeof(struct cscfg_fs_param), GFP_KERNEL);
> +		if (!param_item)
> +			return -ENOMEM;
> +		param_item->desc = feat_desc;
> +		param_item->param = &feat_desc->params[i];
> +		config_group_init_type_name(&param_item->group, param_item->param->name,
> +					    &cscfg_param_view_type);
> +		configfs_add_default_group(&param_item->group, params_group);
> +	}
> +	return 0;
> +}
> +
> +static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct cscfg_fs_feature *feat_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_def_group *params_group = NULL;
> +	int item_err;
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	feat_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +	if (!feat_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (feat_desc->nr_params) {
> +		params_group = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +		if (!params_group)
> +			return ERR_PTR(-ENOMEM);
> +	}
> +
> +	feat_view->desc = feat_desc;
> +	config_group_init_type_name(&feat_view->group,
> +				    feat_desc->name,
> +				    &cscfg_feature_view_type);
> +	if (params_group) {
> +		config_group_init_type_name(&params_group->group, "params",
> +					    &cscfg_feat_param_group_type);
> +		configfs_add_default_group(&params_group->group, &feat_view->group);
> +		item_err = cscfg_create_params_group_items(feat_desc, &params_group->group);
> +		if (item_err)
> +			return ERR_PTR(item_err);
> +	}
> +	return &feat_view->group;
> +}
> +
> +static struct config_item_type cscfg_features_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_features_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "features",
> +			.ci_type = &cscfg_features_type,
> +		},
> +	},
> +};
> +
> +/* add configuration to configurations group */
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_config_group(cfg_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_configs_grp.group, new_group);
> +	return err;
> +}
> +
> +/* add feature to features group */
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_feature_group(feat_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_features_grp.group, new_group);
> +	return err;
> +}
> +
> +static const struct config_item_type syscfg_fs_type = {
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_subsys syscfg_fs_subsys = {
> +	.cfgdev = NULL,
> +	.fs_subsys = {
> +		.su_group = {
> +			.cg_item = {
> +				.ci_namebuf = "coresight-syscfg",
> +				.ci_type = &syscfg_fs_type,
> +			},
> +		},
> +	},
> +};
> +
> +static inline struct cscfg_device *cscfg_fs_get_cscfg_dev(void)
> +{
> +	return syscfg_fs_subsys.cfgdev;
> +}
> +
> +int cscfg_configfs_init(struct cscfg_device *cscfg_dev)
> +{
> +	int ret = 0;
> +	struct configfs_subsystem *subsys = &syscfg_fs_subsys.fs_subsys;
> +
> +	if (!cscfg_dev)
> +		return -EINVAL;
> +
> +	config_group_init(&subsys->su_group);
> +	mutex_init(&subsys->su_mutex);
> +
> +	/* Add default groups to subsystem */
> +	config_group_init(&cscfg_configs_grp.group);
> +	configfs_add_default_group(&cscfg_configs_grp.group, &subsys->su_group);
> +
> +	config_group_init(&cscfg_features_grp.group);
> +	configfs_add_default_group(&cscfg_features_grp.group, &subsys->su_group);
> +
> +	ret = configfs_register_subsystem(subsys);
> +	if (!ret) {
> +		syscfg_fs_subsys.cfgdev = cscfg_dev;
> +		cscfg_dev->cfgfs_subsys = &syscfg_fs_subsys;
> +	}
> +
> +	return ret;
> +}
> +
> +void cscfg_configfs_release(struct cscfg_device *cscfg_dev)
> +{
> +	if (cscfg_dev && cscfg_dev->cfgfs_subsys) {
> +		configfs_unregister_subsystem(&cscfg_dev->cfgfs_subsys->fs_subsys);
> +		cscfg_dev->cfgfs_subsys = NULL;
> +	}
> +	syscfg_fs_subsys.cfgdev = NULL;
> +}
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> new file mode 100644
> index 000000000000..70cc3745649e
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Coresight system configuration driver - support for configfs.
> + */
> +
> +#ifndef CORESIGHT_SYSCFG_CONFIGFS_H
> +#define CORESIGHT_SYSCFG_CONFIGFS_H
> +
> +#include <linux/configfs.h>
> +#include "coresight-syscfg.h"
> +
> +/* container to associate the device and configfs subsystem */
> +struct cscfg_fs_subsys {
> +	struct cscfg_device *cfgdev;
> +	struct configfs_subsystem fs_subsys;
> +};
> +
> +/* container for default groups */
> +struct cscfg_fs_def_group {
> +	struct config_group group;
> +};
> +
> +/* container for configuration view */
> +struct cscfg_fs_config {
> +	struct cscfg_config_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for feature view */
> +struct cscfg_fs_feature {
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for parameter view */
> +struct cscfg_fs_param {
> +	struct cscfg_parameter_desc *param;
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +int cscfg_configfs_init(struct cscfg_device *dev);
> +void cscfg_configfs_release(struct cscfg_device *dev);
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc);
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
> +
> +#endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 8d52580daf04..2cf67a038cc8 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -9,6 +9,7 @@
>  #include "coresight-config.h"
>  #include "coresight-etm-perf.h"
>  #include "coresight-syscfg.h"
> +#include "coresight-syscfg-configfs.h"
>  
>  /*
>   * cscfg_ API manages configurations and features for the entire coresight
> @@ -260,6 +261,8 @@ static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
>  {
>  	int err;
>  
> +	spin_lock_init(&feat_desc->param_lock);
> +
>  	/* add feature to any matching registered devices */
>  	err = cscfg_add_feat_to_csdevs(feat_desc);
>  	if (err)
> @@ -294,6 +297,24 @@ static int cscfg_load_config(struct cscfg_config_desc *cfg_desc)
>  	return err;
>  }
>  
> +/* get a feature descriptor by name */
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name)
> +{
> +	const struct cscfg_feature_desc *feat = NULL, *feat_item;
> +
> +	mutex_lock(&cscfg_mutex);
> +
> +	list_for_each_entry(feat_item, &cscfg_data.feat_list, item) {
> +		if (strcmp(feat_item->name, name) == 0) {
> +			feat = feat_item;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&cscfg_mutex);
> +	return feat;
> +}
> +
>  /*
>   * External API function to load feature and config sets.
>   * Take a 0 terminated array of feature descriptors and/or configuration
> @@ -316,6 +337,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       feat_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_feature(feat_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -330,6 +352,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       cfg_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_config(cfg_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -502,6 +525,7 @@ struct device *to_device_cscfg(void)
>  /* Must have a release function or the kernel will complain on module unload */
>  void cscfg_dev_release(struct device *dev)
>  {
> +	cscfg_configfs_release(cscfg_dev);
>  	kfree(cscfg_dev);
>  	cscfg_dev = NULL;
>  }
> @@ -530,6 +554,7 @@ int cscfg_create_device(void)
>  	cscfg_dev->cfg_data = &cscfg_data;
>  
>  	err = device_register(dev);
> +
>  	if (err)
>  		cscfg_dev_release(dev);
>  
> @@ -587,6 +612,10 @@ int __init cscfg_init(void)
>  	if (err)
>  		goto exit_drv_unregister;
>  
> +	err = cscfg_configfs_init(cscfg_dev);
> +	if (err)
> +		goto exit_dev_clear;
> +
>  	INIT_LIST_HEAD(&cscfg_data.dev_list);
>  	INIT_LIST_HEAD(&cscfg_data.feat_list);
>  	INIT_LIST_HEAD(&cscfg_data.config_list);
> @@ -595,6 +624,8 @@ int __init cscfg_init(void)
>  	dev_info(to_device_cscfg(), "CoreSight Configuration manager initialised");
>  	return 0;
>  
> +exit_dev_clear:
> +	cscfg_clear_device();
>  exit_drv_unregister:
>  	cscfg_driver_exit();
>  exit_bus_unregister:
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index e8f352599dd7..ce237a69677b 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -6,6 +6,7 @@
>  #ifndef CORESIGHT_SYSCFG_H
>  #define CORESIGHT_SYSCFG_H
>  
> +#include <linux/configfs.h>
>  #include <linux/coresight.h>
>  #include <linux/device.h>
>  
> @@ -46,6 +47,7 @@ struct cscfg_csdev_register {
>  int __init cscfg_init(void);
>  void __exit cscfg_exit(void);
>  int cscfg_preload(void);
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name);
>  
>  /* syscfg external API */
>  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> @@ -64,10 +66,12 @@ void cscfg_unregister_csdev(struct coresight_device *csdev);
>   *
>   * @dev:	The device.
>   * @cfg_data:	A reference to the configuration and feature lists
> + * @cfgfs_subsys: configfs subsystem used to manage configurations.
>   */
>  struct cscfg_device {
>  	struct device dev;
>  	struct cscfg_api_data *cfg_data;
> +	struct cscfg_fs_subsys *cfgfs_subsys;
>  };
>  
>  /* basic device driver */
> -- 
> 2.17.1
>
Mathieu Poirier Nov. 26, 2020, 4:51 p.m. UTC | #3
On Fri, Oct 30, 2020 at 05:56:53PM +0000, Mike Leach wrote:
> Adds configfs subsystem and attributes to the configuration manager
> to enable the listing of loaded configurations and features.
> 
> The default values of feature parameters can be accessed and altered
> from these attributes to affect all installed devices using the feature.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/Makefile          |   2 +-
>  .../hwtracing/coresight/coresight-config.c    |   2 +
>  .../hwtracing/coresight/coresight-config.h    |   5 +-
>  .../coresight/coresight-syscfg-configfs.c     | 418 ++++++++++++++++++
>  .../coresight/coresight-syscfg-configfs.h     |  47 ++
>  .../hwtracing/coresight/coresight-syscfg.c    |  31 ++
>  .../hwtracing/coresight/coresight-syscfg.h    |   4 +
>  7 files changed, 507 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.c
>  create mode 100644 drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> 
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 9de5586cfd1a..4ec226c44f5b 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -5,7 +5,7 @@
>  obj-$(CONFIG_CORESIGHT) += coresight.o
>  coresight-y := coresight-core.o  coresight-etm-perf.o coresight-platform.o \
>  		coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> -		coresight-cfg-preload.o
> +		coresight-cfg-preload.o coresight-syscfg-configfs.o
>  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
>  coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
>  		      coresight-tmc-etr.o
> diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c
> index d911e0f083c1..04e7cb4ff769 100644
> --- a/drivers/hwtracing/coresight/coresight-config.c
> +++ b/drivers/hwtracing/coresight/coresight-config.c
> @@ -111,8 +111,10 @@ static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
>  	 * set the default values for all parameters and regs from the
>  	 * relevant static descriptors.
>  	 */
> +	spin_lock(&feat->desc->param_lock);
>  	for (i = 0; i < feat->nr_params; i++)
>  		feat->params[i].current_value = feat->desc->params[i].value;
> +	spin_unlock(&feat->desc->param_lock);
>  
>  	for (i = 0; i < feat->nr_regs; i++) {
>  		reg_desc = &feat->desc->regs[i];
> diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> index 372f29a59688..39fcac011aa0 100644
> --- a/drivers/hwtracing/coresight/coresight-config.h
> +++ b/drivers/hwtracing/coresight/coresight-config.h
> @@ -7,6 +7,7 @@
>  #ifndef _CORESIGHT_CORESIGHT_CONFIG_H
>  #define _CORESIGHT_CORESIGHT_CONFIG_H
>  
> +#include <linux/configfs.h>
>  #include <linux/coresight.h>
>  #include <linux/types.h>
>  
> @@ -89,6 +90,7 @@ struct cscfg_regval {
>   * @match_flags: matching information if loading into a device
>   * @nr_params:  number of parameters used.
>   * @params:	array of parameters used.
> + * @param_lock: protect params when accessing default values.
>   * @nr_regs:	number of registers used.
>   * @reg:	array of registers used.
>   */
> @@ -99,6 +101,7 @@ struct cscfg_feature_desc {
>  	u32 match_flags;
>  	int nr_params;
>  	struct cscfg_parameter_desc *params;
> +	spinlock_t param_lock;
>  	int nr_regs;
>  	struct cscfg_regval *regs;
>  };
> @@ -239,7 +242,7 @@ struct cscfg_feat_ops {
>   * @ops:		standard ops to enable and disable features on devices.
>   */
>  struct cscfg_feature_csdev {
> -	const struct cscfg_feature_desc *desc;
> +	struct cscfg_feature_desc *desc;
>  	struct coresight_device *csdev;
>  	struct list_head node;
>  	spinlock_t *csdev_spinlock;
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> new file mode 100644
> index 000000000000..ff7ea678100a
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 Linaro Limited, All rights reserved.
> + * Author: Mike Leach <mike.leach@linaro.org>
> + */
> +
> +#include <linux/configfs.h>
> +
> +#include "coresight-syscfg-configfs.h"
> +
> +/*
> + * configfs support for syscfg device.
> + *
> + * config fs is used to manage configurations and features on the
> + * coresight system.
> + */
> +
> +static struct cscfg_device *cscfg_fs_get_cscfg_dev(void);
> +
> +static struct device *cscfg_dev_to_dev(void)
> +{
> +	return cscfg_fs_get_cscfg_dev() ? &cscfg_fs_get_cscfg_dev()->dev : NULL;
> +}
> +
> +/*
> + * Declare the subsystem - with base attributes allowing listing of
> + * loaded configurations
> + */
> +static inline struct cscfg_fs_config *to_cscfg_fs_config_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_config, group);
> +}
> +
> +/* configurations sub-group */
> +
> +/* attributes for the config view group */
> +static ssize_t cscfg_cfg_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_config->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, description);
> +
> +static ssize_t cscfg_cfg_refs_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *desc = fs_config->desc;
> +	ssize_t ch_used = 0;
> +	int i;
> +
> +	if (desc->nr_refs) {
> +		ch_used += scnprintf(page, PAGE_SIZE, "references %d features:-\n", desc->nr_refs);
> +		for (i = 0; i < desc->nr_refs; i++) {
> +			ch_used += scnprintf(page + ch_used, PAGE_SIZE - ch_used,
> +					     "%s\n", desc->refs[i].name);
> +		}
> +	} else
> +		ch_used = scnprintf(page, PAGE_SIZE, "no features referenced\n");
> +	return ch_used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, refs);
> +
> +static ssize_t cscfg_cfg_nr_presets_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->desc->nr_presets);
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, nr_presets);
> +
> +static ssize_t cscfg_cfg_preset_values_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
> +	const struct cscfg_config_desc *cfg = fs_config->desc;
> +	const struct cscfg_feature_desc *feat;
> +	ssize_t used = 0;
> +	int i, j, val_idx, preset_idx;
> +
> +	if (!fs_config->desc->nr_presets)
> +		return scnprintf(page, PAGE_SIZE, "No presets defined\n");
> +
> +	used = scnprintf(page, PAGE_SIZE, "%d presets, %d parameters per preset\n",
> +			 fs_config->desc->nr_presets, cfg->nr_total_params);
> +
> +	for (preset_idx = 0; preset_idx < fs_config->desc->nr_presets; preset_idx++) {
> +		/* start index on the correct array line */
> +		val_idx = cfg->nr_total_params * preset_idx;
> +
> +		/* preset indexes are used as 1 based by the user */
> +		used += scnprintf(page + used, PAGE_SIZE - used, "preset[%d]: ", preset_idx+1);
> +
> +		/*
> +		 * A set of presets is the sum of all params in used features,
> +		 * in order of declaration of features and params in the features
> +		 */
> +		for (i = 0; i < cfg->nr_refs; i++) {
> +			feat = cscfg_get_named_feat_desc(cfg->refs[i].name);
> +			for (j = 0; j < cfg->refs[i].nr_params; j++) {
> +				used += scnprintf(page + used, PAGE_SIZE - used,
> +						  "%s.%s = 0x%llx ",
> +						  cfg->refs[i].name, feat->params[j].name,
> +						  cfg->presets[val_idx++]);
> +			}
> +		}
> +		used += scnprintf(page + used, PAGE_SIZE - used, "\n");
> +	}
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_cfg_, preset_values);
> +
> +static struct configfs_attribute *cscfg_config_view_attrs[] = {
> +	&cscfg_cfg_attr_description,
> +	&cscfg_cfg_attr_refs,
> +	&cscfg_cfg_attr_nr_presets,
> +	&cscfg_cfg_attr_preset_values,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_config_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_config_view_attrs,
> +};
> +
> +static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct cscfg_fs_config *cfg_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	cfg_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_config), GFP_KERNEL);
> +	if (!cfg_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cfg_view->desc = cfg_desc;
> +	config_group_init_type_name(&cfg_view->group, cfg_desc->name, &cscfg_config_view_type);
> +	return &cfg_view->group;
> +}
> +
> +static struct config_item_type cscfg_configs_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_configs_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "configurations",
> +			.ci_type = &cscfg_configs_type,
> +		},
> +	},
> +};
> +
> +
> +/* attributes for features view */
> +
> +static inline struct cscfg_fs_feature *to_cscfg_fs_feature_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_feature, group);
> +}
> +
> +static ssize_t cscfg_feat_description_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%s\n", fs_feat->desc->brief);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, description);
> +
> +static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +	u32 match_flags = fs_feat->desc->match_flags;
> +	int used = 0;
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ALL)
> +		used = scnprintf(page, PAGE_SIZE, "SRC_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_ALL)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_ALL ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_CPU)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_CPU ");
> +
> +	if (match_flags & CS_CFG_MATCH_CLASS_CTI_SYS)
> +		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_SYS ");
> +
> +	used += scnprintf(page + used, PAGE_SIZE - used, "\n");
> +	return used;
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, matches);
> +
> +static ssize_t cscfg_feat_nr_params_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "%d\n", fs_feat->desc->nr_params);
> +}
> +CONFIGFS_ATTR_RO(cscfg_feat_, nr_params);
> +
> +/* base feature desc attrib structures */
> +static struct configfs_attribute *cscfg_feature_view_attrs[] = {
> +	&cscfg_feat_attr_description,
> +	&cscfg_feat_attr_matches,
> +	&cscfg_feat_attr_nr_params,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_feature_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_feature_view_attrs,
> +};
> +
> +static inline struct cscfg_fs_param *to_cscfg_fs_param_group(struct config_group *group)
> +{
> +	return container_of(group, struct cscfg_fs_param, group);
> +}
> +
> +static ssize_t cscfg_param_value_show(struct config_item *item, char *page)
> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
> +
> +	return scnprintf(page, PAGE_SIZE, "0x%llx\n", param_item->param->value);
> +}
> +
> +static ssize_t cscfg_param_value_store(struct config_item *item,
> +					       const char *page, size_t size)

Indentation problem

> +{
> +	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
> +	u64 val;
> +	int err;
> +
> +	err = kstrtoull(page, 0, &val);
> +	if (err)
> +		return err;
> +
> +	spin_lock(&param_item->desc->param_lock);
> +	param_item->param->value = val;
> +	spin_unlock(&param_item->desc->param_lock);
> +
> +	return size;
> +}
> +CONFIGFS_ATTR(cscfg_param_, value);
> +
> +static struct configfs_attribute *cscfg_param_view_attrs[] = {
> +	&cscfg_param_attr_value,
> +	NULL,
> +};
> +
> +static struct config_item_type cscfg_param_view_type = {
> +	.ct_owner = THIS_MODULE,
> +	.ct_attrs = cscfg_param_view_attrs,
> +};
> +
> +static struct config_item_type cscfg_feat_param_group_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +/*
> + * configfs has far less functionality provided to add attributes dynamically than sysfs,
> + * and the show and store fns pass the enclosing config_item so the actual attribute cannot
> + * be determined. Therefore we add each item as a group directory, with a value attribute.
> + */
> +static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
> +					   struct config_group *params_group)
> +{
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_param *param_item;
> +	int i;
> +
> +	/* parameter items - as groups with default_value attribute */
> +	for (i = 0; i < feat_desc->nr_params; i++) {
> +		param_item = devm_kzalloc(dev, sizeof(struct cscfg_fs_param), GFP_KERNEL);
> +		if (!param_item)
> +			return -ENOMEM;
> +		param_item->desc = feat_desc;
> +		param_item->param = &feat_desc->params[i];
> +		config_group_init_type_name(&param_item->group, param_item->param->name,
> +					    &cscfg_param_view_type);
> +		configfs_add_default_group(&param_item->group, params_group);
> +	}
> +	return 0;
> +}
> +
> +static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct cscfg_fs_feature *feat_view;
> +	struct device *dev = cscfg_dev_to_dev();
> +	struct cscfg_fs_def_group *params_group = NULL;
> +	int item_err;
> +
> +	if (!dev)
> +		return ERR_PTR(-EINVAL);
> +
> +	feat_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +	if (!feat_view)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (feat_desc->nr_params) {
> +		params_group = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
> +		if (!params_group)
> +			return ERR_PTR(-ENOMEM);
> +	}
> +
> +	feat_view->desc = feat_desc;
> +	config_group_init_type_name(&feat_view->group,
> +				    feat_desc->name,
> +				    &cscfg_feature_view_type);
> +	if (params_group) {
> +		config_group_init_type_name(&params_group->group, "params",
> +					    &cscfg_feat_param_group_type);
> +		configfs_add_default_group(&params_group->group, &feat_view->group);
> +		item_err = cscfg_create_params_group_items(feat_desc, &params_group->group);
> +		if (item_err)
> +			return ERR_PTR(item_err);
> +	}
> +	return &feat_view->group;
> +}
> +
> +static struct config_item_type cscfg_features_type = {
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_def_group cscfg_features_grp = {
> +	.group = {
> +		.cg_item = {
> +			.ci_namebuf = "features",
> +			.ci_type = &cscfg_features_type,
> +		},
> +	},
> +};
> +
> +/* add configuration to configurations group */
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_config_group(cfg_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_configs_grp.group, new_group);
> +	return err;
> +}
> +
> +/* add feature to features group */
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
> +{
> +	struct config_group *new_group;
> +	int err;
> +
> +	new_group = cscfg_create_feature_group(feat_desc);
> +	if (IS_ERR(new_group))
> +		return PTR_ERR(new_group);
> +	err =  configfs_register_group(&cscfg_features_grp.group, new_group);
> +	return err;
> +}
> +
> +static const struct config_item_type syscfg_fs_type = {
> +	.ct_owner	= THIS_MODULE,
> +};
> +
> +static struct cscfg_fs_subsys syscfg_fs_subsys = {
> +	.cfgdev = NULL,
> +	.fs_subsys = {
> +		.su_group = {
> +			.cg_item = {
> +				.ci_namebuf = "coresight-syscfg",
> +				.ci_type = &syscfg_fs_type,
> +			},
> +		},
> +	},
> +};
> +
> +static inline struct cscfg_device *cscfg_fs_get_cscfg_dev(void)
> +{
> +	return syscfg_fs_subsys.cfgdev;
> +}
> +
> +int cscfg_configfs_init(struct cscfg_device *cscfg_dev)
> +{
> +	int ret = 0;
> +	struct configfs_subsystem *subsys = &syscfg_fs_subsys.fs_subsys;
> +
> +	if (!cscfg_dev)
> +		return -EINVAL;
> +
> +	config_group_init(&subsys->su_group);
> +	mutex_init(&subsys->su_mutex);
> +
> +	/* Add default groups to subsystem */
> +	config_group_init(&cscfg_configs_grp.group);
> +	configfs_add_default_group(&cscfg_configs_grp.group, &subsys->su_group);
> +
> +	config_group_init(&cscfg_features_grp.group);
> +	configfs_add_default_group(&cscfg_features_grp.group, &subsys->su_group);
> +
> +	ret = configfs_register_subsystem(subsys);
> +	if (!ret) {
> +		syscfg_fs_subsys.cfgdev = cscfg_dev;
> +		cscfg_dev->cfgfs_subsys = &syscfg_fs_subsys;
> +	}
> +
> +	return ret;
> +}
> +
> +void cscfg_configfs_release(struct cscfg_device *cscfg_dev)
> +{
> +	if (cscfg_dev && cscfg_dev->cfgfs_subsys) {
> +		configfs_unregister_subsystem(&cscfg_dev->cfgfs_subsys->fs_subsys);
> +		cscfg_dev->cfgfs_subsys = NULL;
> +	}
> +	syscfg_fs_subsys.cfgdev = NULL;
> +}
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> new file mode 100644
> index 000000000000..70cc3745649e
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Coresight system configuration driver - support for configfs.
> + */
> +
> +#ifndef CORESIGHT_SYSCFG_CONFIGFS_H
> +#define CORESIGHT_SYSCFG_CONFIGFS_H
> +
> +#include <linux/configfs.h>
> +#include "coresight-syscfg.h"
> +
> +/* container to associate the device and configfs subsystem */
> +struct cscfg_fs_subsys {
> +	struct cscfg_device *cfgdev;
> +	struct configfs_subsystem fs_subsys;
> +};
> +
> +/* container for default groups */
> +struct cscfg_fs_def_group {
> +	struct config_group group;
> +};
> +
> +/* container for configuration view */
> +struct cscfg_fs_config {
> +	struct cscfg_config_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for feature view */
> +struct cscfg_fs_feature {
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +/* container for parameter view */
> +struct cscfg_fs_param {
> +	struct cscfg_parameter_desc *param;
> +	struct cscfg_feature_desc *desc;
> +	struct config_group group;
> +};
> +
> +int cscfg_configfs_init(struct cscfg_device *dev);
> +void cscfg_configfs_release(struct cscfg_device *dev);
> +int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc);
> +int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
> +
> +#endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 8d52580daf04..2cf67a038cc8 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -9,6 +9,7 @@
>  #include "coresight-config.h"
>  #include "coresight-etm-perf.h"
>  #include "coresight-syscfg.h"
> +#include "coresight-syscfg-configfs.h"
>  
>  /*
>   * cscfg_ API manages configurations and features for the entire coresight
> @@ -260,6 +261,8 @@ static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
>  {
>  	int err;
>  
> +	spin_lock_init(&feat_desc->param_lock);
> +
>  	/* add feature to any matching registered devices */
>  	err = cscfg_add_feat_to_csdevs(feat_desc);
>  	if (err)
> @@ -294,6 +297,24 @@ static int cscfg_load_config(struct cscfg_config_desc *cfg_desc)
>  	return err;
>  }
>  
> +/* get a feature descriptor by name */
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name)
> +{
> +	const struct cscfg_feature_desc *feat = NULL, *feat_item;
> +
> +	mutex_lock(&cscfg_mutex);
> +
> +	list_for_each_entry(feat_item, &cscfg_data.feat_list, item) {
> +		if (strcmp(feat_item->name, name) == 0) {
> +			feat = feat_item;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&cscfg_mutex);
> +	return feat;
> +}
> +
>  /*
>   * External API function to load feature and config sets.
>   * Take a 0 terminated array of feature descriptors and/or configuration
> @@ -316,6 +337,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       feat_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_feature(feat_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -330,6 +352,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
>  				       cfg_descs[i]->name);
>  				goto do_unlock;
>  			}
> +			cscfg_configfs_add_config(cfg_descs[i]);
>  			i++;
>  		}
>  	}
> @@ -502,6 +525,7 @@ struct device *to_device_cscfg(void)
>  /* Must have a release function or the kernel will complain on module unload */
>  void cscfg_dev_release(struct device *dev)
>  {
> +	cscfg_configfs_release(cscfg_dev);
>  	kfree(cscfg_dev);
>  	cscfg_dev = NULL;
>  }
> @@ -530,6 +554,7 @@ int cscfg_create_device(void)
>  	cscfg_dev->cfg_data = &cscfg_data;
>  
>  	err = device_register(dev);
> +
>  	if (err)
>  		cscfg_dev_release(dev);
>  
> @@ -587,6 +612,10 @@ int __init cscfg_init(void)
>  	if (err)
>  		goto exit_drv_unregister;
>  
> +	err = cscfg_configfs_init(cscfg_dev);
> +	if (err)
> +		goto exit_dev_clear;
> +
>  	INIT_LIST_HEAD(&cscfg_data.dev_list);
>  	INIT_LIST_HEAD(&cscfg_data.feat_list);
>  	INIT_LIST_HEAD(&cscfg_data.config_list);
> @@ -595,6 +624,8 @@ int __init cscfg_init(void)
>  	dev_info(to_device_cscfg(), "CoreSight Configuration manager initialised");
>  	return 0;
>  
> +exit_dev_clear:
> +	cscfg_clear_device();
>  exit_drv_unregister:
>  	cscfg_driver_exit();
>  exit_bus_unregister:
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index e8f352599dd7..ce237a69677b 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -6,6 +6,7 @@
>  #ifndef CORESIGHT_SYSCFG_H
>  #define CORESIGHT_SYSCFG_H
>  
> +#include <linux/configfs.h>
>  #include <linux/coresight.h>
>  #include <linux/device.h>
>  
> @@ -46,6 +47,7 @@ struct cscfg_csdev_register {
>  int __init cscfg_init(void);
>  void __exit cscfg_exit(void);
>  int cscfg_preload(void);
> +const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name);
>  
>  /* syscfg external API */
>  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> @@ -64,10 +66,12 @@ void cscfg_unregister_csdev(struct coresight_device *csdev);
>   *
>   * @dev:	The device.
>   * @cfg_data:	A reference to the configuration and feature lists
> + * @cfgfs_subsys: configfs subsystem used to manage configurations.
>   */
>  struct cscfg_device {
>  	struct device dev;
>  	struct cscfg_api_data *cfg_data;
> +	struct cscfg_fs_subsys *cfgfs_subsys;
>  };
>  
>  /* basic device driver */
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 9de5586cfd1a..4ec226c44f5b 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -5,7 +5,7 @@ 
 obj-$(CONFIG_CORESIGHT) += coresight.o
 coresight-y := coresight-core.o  coresight-etm-perf.o coresight-platform.o \
 		coresight-sysfs.o coresight-syscfg.o coresight-config.o \
-		coresight-cfg-preload.o
+		coresight-cfg-preload.o coresight-syscfg-configfs.o
 obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
 coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
 		      coresight-tmc-etr.o
diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c
index d911e0f083c1..04e7cb4ff769 100644
--- a/drivers/hwtracing/coresight/coresight-config.c
+++ b/drivers/hwtracing/coresight/coresight-config.c
@@ -111,8 +111,10 @@  static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
 	 * set the default values for all parameters and regs from the
 	 * relevant static descriptors.
 	 */
+	spin_lock(&feat->desc->param_lock);
 	for (i = 0; i < feat->nr_params; i++)
 		feat->params[i].current_value = feat->desc->params[i].value;
+	spin_unlock(&feat->desc->param_lock);
 
 	for (i = 0; i < feat->nr_regs; i++) {
 		reg_desc = &feat->desc->regs[i];
diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
index 372f29a59688..39fcac011aa0 100644
--- a/drivers/hwtracing/coresight/coresight-config.h
+++ b/drivers/hwtracing/coresight/coresight-config.h
@@ -7,6 +7,7 @@ 
 #ifndef _CORESIGHT_CORESIGHT_CONFIG_H
 #define _CORESIGHT_CORESIGHT_CONFIG_H
 
+#include <linux/configfs.h>
 #include <linux/coresight.h>
 #include <linux/types.h>
 
@@ -89,6 +90,7 @@  struct cscfg_regval {
  * @match_flags: matching information if loading into a device
  * @nr_params:  number of parameters used.
  * @params:	array of parameters used.
+ * @param_lock: protect params when accessing default values.
  * @nr_regs:	number of registers used.
  * @reg:	array of registers used.
  */
@@ -99,6 +101,7 @@  struct cscfg_feature_desc {
 	u32 match_flags;
 	int nr_params;
 	struct cscfg_parameter_desc *params;
+	spinlock_t param_lock;
 	int nr_regs;
 	struct cscfg_regval *regs;
 };
@@ -239,7 +242,7 @@  struct cscfg_feat_ops {
  * @ops:		standard ops to enable and disable features on devices.
  */
 struct cscfg_feature_csdev {
-	const struct cscfg_feature_desc *desc;
+	struct cscfg_feature_desc *desc;
 	struct coresight_device *csdev;
 	struct list_head node;
 	spinlock_t *csdev_spinlock;
diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
new file mode 100644
index 000000000000..ff7ea678100a
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
@@ -0,0 +1,418 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include <linux/configfs.h>
+
+#include "coresight-syscfg-configfs.h"
+
+/*
+ * configfs support for syscfg device.
+ *
+ * config fs is used to manage configurations and features on the
+ * coresight system.
+ */
+
+static struct cscfg_device *cscfg_fs_get_cscfg_dev(void);
+
+static struct device *cscfg_dev_to_dev(void)
+{
+	return cscfg_fs_get_cscfg_dev() ? &cscfg_fs_get_cscfg_dev()->dev : NULL;
+}
+
+/*
+ * Declare the subsystem - with base attributes allowing listing of
+ * loaded configurations
+ */
+static inline struct cscfg_fs_config *to_cscfg_fs_config_group(struct config_group *group)
+{
+	return container_of(group, struct cscfg_fs_config, group);
+}
+
+/* configurations sub-group */
+
+/* attributes for the config view group */
+static ssize_t cscfg_cfg_description_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
+
+	return scnprintf(page, PAGE_SIZE, "%s\n", fs_config->desc->brief);
+}
+CONFIGFS_ATTR_RO(cscfg_cfg_, description);
+
+static ssize_t cscfg_cfg_refs_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
+	const struct cscfg_config_desc *desc = fs_config->desc;
+	ssize_t ch_used = 0;
+	int i;
+
+	if (desc->nr_refs) {
+		ch_used += scnprintf(page, PAGE_SIZE, "references %d features:-\n", desc->nr_refs);
+		for (i = 0; i < desc->nr_refs; i++) {
+			ch_used += scnprintf(page + ch_used, PAGE_SIZE - ch_used,
+					     "%s\n", desc->refs[i].name);
+		}
+	} else
+		ch_used = scnprintf(page, PAGE_SIZE, "no features referenced\n");
+	return ch_used;
+}
+CONFIGFS_ATTR_RO(cscfg_cfg_, refs);
+
+static ssize_t cscfg_cfg_nr_presets_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
+
+	return scnprintf(page, PAGE_SIZE, "%d\n", fs_config->desc->nr_presets);
+}
+CONFIGFS_ATTR_RO(cscfg_cfg_, nr_presets);
+
+static ssize_t cscfg_cfg_preset_values_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_config *fs_config = to_cscfg_fs_config_group(to_config_group(item));
+	const struct cscfg_config_desc *cfg = fs_config->desc;
+	const struct cscfg_feature_desc *feat;
+	ssize_t used = 0;
+	int i, j, val_idx, preset_idx;
+
+	if (!fs_config->desc->nr_presets)
+		return scnprintf(page, PAGE_SIZE, "No presets defined\n");
+
+	used = scnprintf(page, PAGE_SIZE, "%d presets, %d parameters per preset\n",
+			 fs_config->desc->nr_presets, cfg->nr_total_params);
+
+	for (preset_idx = 0; preset_idx < fs_config->desc->nr_presets; preset_idx++) {
+		/* start index on the correct array line */
+		val_idx = cfg->nr_total_params * preset_idx;
+
+		/* preset indexes are used as 1 based by the user */
+		used += scnprintf(page + used, PAGE_SIZE - used, "preset[%d]: ", preset_idx+1);
+
+		/*
+		 * A set of presets is the sum of all params in used features,
+		 * in order of declaration of features and params in the features
+		 */
+		for (i = 0; i < cfg->nr_refs; i++) {
+			feat = cscfg_get_named_feat_desc(cfg->refs[i].name);
+			for (j = 0; j < cfg->refs[i].nr_params; j++) {
+				used += scnprintf(page + used, PAGE_SIZE - used,
+						  "%s.%s = 0x%llx ",
+						  cfg->refs[i].name, feat->params[j].name,
+						  cfg->presets[val_idx++]);
+			}
+		}
+		used += scnprintf(page + used, PAGE_SIZE - used, "\n");
+	}
+	return used;
+}
+CONFIGFS_ATTR_RO(cscfg_cfg_, preset_values);
+
+static struct configfs_attribute *cscfg_config_view_attrs[] = {
+	&cscfg_cfg_attr_description,
+	&cscfg_cfg_attr_refs,
+	&cscfg_cfg_attr_nr_presets,
+	&cscfg_cfg_attr_preset_values,
+	NULL,
+};
+
+static struct config_item_type cscfg_config_view_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_attrs = cscfg_config_view_attrs,
+};
+
+static struct config_group *cscfg_create_config_group(struct cscfg_config_desc *cfg_desc)
+{
+	struct cscfg_fs_config *cfg_view;
+	struct device *dev = cscfg_dev_to_dev();
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	cfg_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_config), GFP_KERNEL);
+	if (!cfg_view)
+		return ERR_PTR(-ENOMEM);
+
+	cfg_view->desc = cfg_desc;
+	config_group_init_type_name(&cfg_view->group, cfg_desc->name, &cscfg_config_view_type);
+	return &cfg_view->group;
+}
+
+static struct config_item_type cscfg_configs_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+static struct cscfg_fs_def_group cscfg_configs_grp = {
+	.group = {
+		.cg_item = {
+			.ci_namebuf = "configurations",
+			.ci_type = &cscfg_configs_type,
+		},
+	},
+};
+
+
+/* attributes for features view */
+
+static inline struct cscfg_fs_feature *to_cscfg_fs_feature_group(struct config_group *group)
+{
+	return container_of(group, struct cscfg_fs_feature, group);
+}
+
+static ssize_t cscfg_feat_description_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
+
+	return scnprintf(page, PAGE_SIZE, "%s\n", fs_feat->desc->brief);
+}
+CONFIGFS_ATTR_RO(cscfg_feat_, description);
+
+static ssize_t cscfg_feat_matches_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
+	u32 match_flags = fs_feat->desc->match_flags;
+	int used = 0;
+
+	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ALL)
+		used = scnprintf(page, PAGE_SIZE, "SRC_ALL ");
+
+	if (match_flags & CS_CFG_MATCH_CLASS_SRC_ETM4)
+		used += scnprintf(page + used, PAGE_SIZE - used, "SRC_ETMV4 ");
+
+	if (match_flags & CS_CFG_MATCH_CLASS_CTI_ALL)
+		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_ALL ");
+
+	if (match_flags & CS_CFG_MATCH_CLASS_CTI_CPU)
+		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_CPU ");
+
+	if (match_flags & CS_CFG_MATCH_CLASS_CTI_SYS)
+		used += scnprintf(page + used, PAGE_SIZE - used, "CTI_SYS ");
+
+	used += scnprintf(page + used, PAGE_SIZE - used, "\n");
+	return used;
+}
+CONFIGFS_ATTR_RO(cscfg_feat_, matches);
+
+static ssize_t cscfg_feat_nr_params_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_feature *fs_feat = to_cscfg_fs_feature_group(to_config_group(item));
+
+	return scnprintf(page, PAGE_SIZE, "%d\n", fs_feat->desc->nr_params);
+}
+CONFIGFS_ATTR_RO(cscfg_feat_, nr_params);
+
+/* base feature desc attrib structures */
+static struct configfs_attribute *cscfg_feature_view_attrs[] = {
+	&cscfg_feat_attr_description,
+	&cscfg_feat_attr_matches,
+	&cscfg_feat_attr_nr_params,
+	NULL,
+};
+
+static struct config_item_type cscfg_feature_view_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_attrs = cscfg_feature_view_attrs,
+};
+
+static inline struct cscfg_fs_param *to_cscfg_fs_param_group(struct config_group *group)
+{
+	return container_of(group, struct cscfg_fs_param, group);
+}
+
+static ssize_t cscfg_param_value_show(struct config_item *item, char *page)
+{
+	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
+
+	return scnprintf(page, PAGE_SIZE, "0x%llx\n", param_item->param->value);
+}
+
+static ssize_t cscfg_param_value_store(struct config_item *item,
+					       const char *page, size_t size)
+{
+	struct cscfg_fs_param *param_item = to_cscfg_fs_param_group(to_config_group(item));
+	u64 val;
+	int err;
+
+	err = kstrtoull(page, 0, &val);
+	if (err)
+		return err;
+
+	spin_lock(&param_item->desc->param_lock);
+	param_item->param->value = val;
+	spin_unlock(&param_item->desc->param_lock);
+
+	return size;
+}
+CONFIGFS_ATTR(cscfg_param_, value);
+
+static struct configfs_attribute *cscfg_param_view_attrs[] = {
+	&cscfg_param_attr_value,
+	NULL,
+};
+
+static struct config_item_type cscfg_param_view_type = {
+	.ct_owner = THIS_MODULE,
+	.ct_attrs = cscfg_param_view_attrs,
+};
+
+static struct config_item_type cscfg_feat_param_group_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+/*
+ * configfs has far less functionality provided to add attributes dynamically than sysfs,
+ * and the show and store fns pass the enclosing config_item so the actual attribute cannot
+ * be determined. Therefore we add each item as a group directory, with a value attribute.
+ */
+static int cscfg_create_params_group_items(struct cscfg_feature_desc *feat_desc,
+					   struct config_group *params_group)
+{
+	struct device *dev = cscfg_dev_to_dev();
+	struct cscfg_fs_param *param_item;
+	int i;
+
+	/* parameter items - as groups with default_value attribute */
+	for (i = 0; i < feat_desc->nr_params; i++) {
+		param_item = devm_kzalloc(dev, sizeof(struct cscfg_fs_param), GFP_KERNEL);
+		if (!param_item)
+			return -ENOMEM;
+		param_item->desc = feat_desc;
+		param_item->param = &feat_desc->params[i];
+		config_group_init_type_name(&param_item->group, param_item->param->name,
+					    &cscfg_param_view_type);
+		configfs_add_default_group(&param_item->group, params_group);
+	}
+	return 0;
+}
+
+static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc *feat_desc)
+{
+	struct cscfg_fs_feature *feat_view;
+	struct device *dev = cscfg_dev_to_dev();
+	struct cscfg_fs_def_group *params_group = NULL;
+	int item_err;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	feat_view = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
+	if (!feat_view)
+		return ERR_PTR(-ENOMEM);
+
+	if (feat_desc->nr_params) {
+		params_group = devm_kzalloc(dev, sizeof(struct cscfg_fs_feature), GFP_KERNEL);
+		if (!params_group)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	feat_view->desc = feat_desc;
+	config_group_init_type_name(&feat_view->group,
+				    feat_desc->name,
+				    &cscfg_feature_view_type);
+	if (params_group) {
+		config_group_init_type_name(&params_group->group, "params",
+					    &cscfg_feat_param_group_type);
+		configfs_add_default_group(&params_group->group, &feat_view->group);
+		item_err = cscfg_create_params_group_items(feat_desc, &params_group->group);
+		if (item_err)
+			return ERR_PTR(item_err);
+	}
+	return &feat_view->group;
+}
+
+static struct config_item_type cscfg_features_type = {
+	.ct_owner = THIS_MODULE,
+};
+
+static struct cscfg_fs_def_group cscfg_features_grp = {
+	.group = {
+		.cg_item = {
+			.ci_namebuf = "features",
+			.ci_type = &cscfg_features_type,
+		},
+	},
+};
+
+/* add configuration to configurations group */
+int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc)
+{
+	struct config_group *new_group;
+	int err;
+
+	new_group = cscfg_create_config_group(cfg_desc);
+	if (IS_ERR(new_group))
+		return PTR_ERR(new_group);
+	err =  configfs_register_group(&cscfg_configs_grp.group, new_group);
+	return err;
+}
+
+/* add feature to features group */
+int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
+{
+	struct config_group *new_group;
+	int err;
+
+	new_group = cscfg_create_feature_group(feat_desc);
+	if (IS_ERR(new_group))
+		return PTR_ERR(new_group);
+	err =  configfs_register_group(&cscfg_features_grp.group, new_group);
+	return err;
+}
+
+static const struct config_item_type syscfg_fs_type = {
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct cscfg_fs_subsys syscfg_fs_subsys = {
+	.cfgdev = NULL,
+	.fs_subsys = {
+		.su_group = {
+			.cg_item = {
+				.ci_namebuf = "coresight-syscfg",
+				.ci_type = &syscfg_fs_type,
+			},
+		},
+	},
+};
+
+static inline struct cscfg_device *cscfg_fs_get_cscfg_dev(void)
+{
+	return syscfg_fs_subsys.cfgdev;
+}
+
+int cscfg_configfs_init(struct cscfg_device *cscfg_dev)
+{
+	int ret = 0;
+	struct configfs_subsystem *subsys = &syscfg_fs_subsys.fs_subsys;
+
+	if (!cscfg_dev)
+		return -EINVAL;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+
+	/* Add default groups to subsystem */
+	config_group_init(&cscfg_configs_grp.group);
+	configfs_add_default_group(&cscfg_configs_grp.group, &subsys->su_group);
+
+	config_group_init(&cscfg_features_grp.group);
+	configfs_add_default_group(&cscfg_features_grp.group, &subsys->su_group);
+
+	ret = configfs_register_subsystem(subsys);
+	if (!ret) {
+		syscfg_fs_subsys.cfgdev = cscfg_dev;
+		cscfg_dev->cfgfs_subsys = &syscfg_fs_subsys;
+	}
+
+	return ret;
+}
+
+void cscfg_configfs_release(struct cscfg_device *cscfg_dev)
+{
+	if (cscfg_dev && cscfg_dev->cfgfs_subsys) {
+		configfs_unregister_subsystem(&cscfg_dev->cfgfs_subsys->fs_subsys);
+		cscfg_dev->cfgfs_subsys = NULL;
+	}
+	syscfg_fs_subsys.cfgdev = NULL;
+}
diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
new file mode 100644
index 000000000000..70cc3745649e
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
@@ -0,0 +1,47 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Coresight system configuration driver - support for configfs.
+ */
+
+#ifndef CORESIGHT_SYSCFG_CONFIGFS_H
+#define CORESIGHT_SYSCFG_CONFIGFS_H
+
+#include <linux/configfs.h>
+#include "coresight-syscfg.h"
+
+/* container to associate the device and configfs subsystem */
+struct cscfg_fs_subsys {
+	struct cscfg_device *cfgdev;
+	struct configfs_subsystem fs_subsys;
+};
+
+/* container for default groups */
+struct cscfg_fs_def_group {
+	struct config_group group;
+};
+
+/* container for configuration view */
+struct cscfg_fs_config {
+	struct cscfg_config_desc *desc;
+	struct config_group group;
+};
+
+/* container for feature view */
+struct cscfg_fs_feature {
+	struct cscfg_feature_desc *desc;
+	struct config_group group;
+};
+
+/* container for parameter view */
+struct cscfg_fs_param {
+	struct cscfg_parameter_desc *param;
+	struct cscfg_feature_desc *desc;
+	struct config_group group;
+};
+
+int cscfg_configfs_init(struct cscfg_device *dev);
+void cscfg_configfs_release(struct cscfg_device *dev);
+int cscfg_configfs_add_config(struct cscfg_config_desc *cfg_desc);
+int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
+
+#endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index 8d52580daf04..2cf67a038cc8 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -9,6 +9,7 @@ 
 #include "coresight-config.h"
 #include "coresight-etm-perf.h"
 #include "coresight-syscfg.h"
+#include "coresight-syscfg-configfs.h"
 
 /*
  * cscfg_ API manages configurations and features for the entire coresight
@@ -260,6 +261,8 @@  static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
 {
 	int err;
 
+	spin_lock_init(&feat_desc->param_lock);
+
 	/* add feature to any matching registered devices */
 	err = cscfg_add_feat_to_csdevs(feat_desc);
 	if (err)
@@ -294,6 +297,24 @@  static int cscfg_load_config(struct cscfg_config_desc *cfg_desc)
 	return err;
 }
 
+/* get a feature descriptor by name */
+const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name)
+{
+	const struct cscfg_feature_desc *feat = NULL, *feat_item;
+
+	mutex_lock(&cscfg_mutex);
+
+	list_for_each_entry(feat_item, &cscfg_data.feat_list, item) {
+		if (strcmp(feat_item->name, name) == 0) {
+			feat = feat_item;
+			break;
+		}
+	}
+
+	mutex_unlock(&cscfg_mutex);
+	return feat;
+}
+
 /*
  * External API function to load feature and config sets.
  * Take a 0 terminated array of feature descriptors and/or configuration
@@ -316,6 +337,7 @@  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
 				       feat_descs[i]->name);
 				goto do_unlock;
 			}
+			cscfg_configfs_add_feature(feat_descs[i]);
 			i++;
 		}
 	}
@@ -330,6 +352,7 @@  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
 				       cfg_descs[i]->name);
 				goto do_unlock;
 			}
+			cscfg_configfs_add_config(cfg_descs[i]);
 			i++;
 		}
 	}
@@ -502,6 +525,7 @@  struct device *to_device_cscfg(void)
 /* Must have a release function or the kernel will complain on module unload */
 void cscfg_dev_release(struct device *dev)
 {
+	cscfg_configfs_release(cscfg_dev);
 	kfree(cscfg_dev);
 	cscfg_dev = NULL;
 }
@@ -530,6 +554,7 @@  int cscfg_create_device(void)
 	cscfg_dev->cfg_data = &cscfg_data;
 
 	err = device_register(dev);
+
 	if (err)
 		cscfg_dev_release(dev);
 
@@ -587,6 +612,10 @@  int __init cscfg_init(void)
 	if (err)
 		goto exit_drv_unregister;
 
+	err = cscfg_configfs_init(cscfg_dev);
+	if (err)
+		goto exit_dev_clear;
+
 	INIT_LIST_HEAD(&cscfg_data.dev_list);
 	INIT_LIST_HEAD(&cscfg_data.feat_list);
 	INIT_LIST_HEAD(&cscfg_data.config_list);
@@ -595,6 +624,8 @@  int __init cscfg_init(void)
 	dev_info(to_device_cscfg(), "CoreSight Configuration manager initialised");
 	return 0;
 
+exit_dev_clear:
+	cscfg_clear_device();
 exit_drv_unregister:
 	cscfg_driver_exit();
 exit_bus_unregister:
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
index e8f352599dd7..ce237a69677b 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.h
+++ b/drivers/hwtracing/coresight/coresight-syscfg.h
@@ -6,6 +6,7 @@ 
 #ifndef CORESIGHT_SYSCFG_H
 #define CORESIGHT_SYSCFG_H
 
+#include <linux/configfs.h>
 #include <linux/coresight.h>
 #include <linux/device.h>
 
@@ -46,6 +47,7 @@  struct cscfg_csdev_register {
 int __init cscfg_init(void);
 void __exit cscfg_exit(void);
 int cscfg_preload(void);
+const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name);
 
 /* syscfg external API */
 int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
@@ -64,10 +66,12 @@  void cscfg_unregister_csdev(struct coresight_device *csdev);
  *
  * @dev:	The device.
  * @cfg_data:	A reference to the configuration and feature lists
+ * @cfgfs_subsys: configfs subsystem used to manage configurations.
  */
 struct cscfg_device {
 	struct device dev;
 	struct cscfg_api_data *cfg_data;
+	struct cscfg_fs_subsys *cfgfs_subsys;
 };
 
 /* basic device driver */