[RFC PATCH v3 8/9] coresight: syscfg: Allow update of feature params from configfs

[Mathieu Poirier]

On Fri, Oct 30, 2020 at 05:56:54PM +0000, Mike Leach wrote:
> Add in functionality to allow the user to update feature default parameter
> values from the configfs interface.
> 
> This updates all the device instances with the new values, removing the
> need to set all devices individually via sysfs.
> 
> Signed-off-by: Mike Leach <mike.leach at linaro.org>
> ---
>  .../hwtracing/coresight/coresight-config.c    | 36 ++++++++++++++++---
>  .../hwtracing/coresight/coresight-config.h    |  1 +
>  .../coresight/coresight-syscfg-configfs.c     |  3 ++
>  .../hwtracing/coresight/coresight-syscfg.c    | 15 ++++++++
>  .../hwtracing/coresight/coresight-syscfg.h    |  1 +
>  5 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-config.c b/drivers/hwtracing/coresight/coresight-config.c
> index 04e7cb4ff769..7d30a415f2ff 100644
> --- a/drivers/hwtracing/coresight/coresight-config.c
> +++ b/drivers/hwtracing/coresight/coresight-config.c
> @@ -96,6 +96,17 @@ static void cscfg_save_on_disable(struct cscfg_feature_csdev *feat, const bool f
>  	spin_unlock(feat->csdev_spinlock);
>  }
>  
> +/* load default values into params */
> +static void cscfg_set_param_defaults(struct cscfg_feature_csdev *feat)
> +{
> +	int i;
> +
> +	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);
> +}
> +
>  /* default reset - restore default values, disable feature */
>  static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
>  {
> @@ -111,10 +122,7 @@ 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);
> +	cscfg_set_param_defaults(feat);
>  
>  	for (i = 0; i < feat->nr_regs; i++) {
>  		reg_desc = &feat->desc->regs[i];
> @@ -131,6 +139,26 @@ static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
>  	spin_unlock(feat->csdev_spinlock);
>  }
>  
> +/* update the parameters in a named feature from their defaults for the supplied device */
> +void cscfg_csdev_set_param_defaults(struct coresight_device *csdev, const char *feat_name)
> +{
> +	struct cscfg_feature_csdev *feat;
> +
> +	spin_lock(&csdev->cfg_lock);
> +	if (list_empty(&csdev->feat_list)) {
> +		spin_unlock(&csdev->cfg_lock);
> +		return;
> +	}
> +
> +	list_for_each_entry(feat, &csdev->feat_list, node) {
> +		if (!strcmp(feat_name, feat->desc->name)) {
> +			cscfg_set_param_defaults(feat);
> +			break;
> +		}
> +	}
> +	spin_unlock(&csdev->cfg_lock);
> +}
> +
>  void cscfg_set_def_ops(struct cscfg_feature_csdev *feat)
>  {
>  	feat->ops.set_on_enable = cscfg_set_on_enable;
> diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> index 39fcac011aa0..1c6f0f903861 100644
> --- a/drivers/hwtracing/coresight/coresight-config.h
> +++ b/drivers/hwtracing/coresight/coresight-config.h
> @@ -297,6 +297,7 @@ struct cscfg_csdev_feat_ops {
>  
>  /* helper functions for feature manipulation */
>  void cscfg_set_def_ops(struct cscfg_feature_csdev *feat);
> +void cscfg_csdev_set_param_defaults(struct coresight_device *csdev, const char *feat_name);
>  
>  /* enable / disable features or configs on a device */
>  int cscfg_csdev_set_enabled_feats(struct coresight_device *csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> index ff7ea678100a..1595c0c61db1 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> @@ -242,6 +242,9 @@ static ssize_t cscfg_param_value_store(struct config_item *item,
>  	param_item->param->value = val;
>  	spin_unlock(&param_item->desc->param_lock);
>  
> +	/* push new value out to devices */
> +	cscfg_update_named_feat_csdevs(param_item->desc->name);
> +
>  	return size;
>  }
>  CONFIGFS_ATTR(cscfg_param_, value);
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 2cf67a038cc8..c42374342806 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -315,6 +315,21 @@ const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name)
>  	return feat;
>  }
>  
> +/*
> + * Set all parameter defaults for named feature.
> + * Iterates through csdev list and updates param defaults on named feature.
> + */
> +void cscfg_update_named_feat_csdevs(const char *feat_name)
> +{
> +	struct cscfg_csdev_register *curr_item;
> +
> +	mutex_lock(&cscfg_mutex);
> +	list_for_each_entry(curr_item, &cscfg_data.dev_list, item) {
> +		cscfg_csdev_set_param_defaults(curr_item->csdev, feat_name);
> +	}
> +	mutex_unlock(&cscfg_mutex);

This is what I was referring to when expressing my worries about the number of
locks to manage.  Here we have a mutex holding a spinlock holding another
spinlock.  It is a matter of time before we lockup the system.  I think RCUs
will be our only option but that will seriously impede readability.  

I currently don't have anything better to suggest and as such will accept to
move forward with the current situation.  We may think of something better as
this set continue to mature.

> +}
> +
>  /*
>   * External API function to load feature and config sets.
>   * Take a 0 terminated array of feature descriptors and/or configuration
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> index ce237a69677b..d07a0f11097f 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> @@ -48,6 +48,7 @@ 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);
> +void cscfg_update_named_feat_csdevs(const char *feat_name);
>  
>  /* syscfg external API */
>  int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> -- 
> 2.17.1
>