public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: yabinc@google.com, coresight@lists.linaro.org,
	linux-arm-kernel@lists.infradead.org, suzuki.poulose@arm.com
Subject: Re: [RFC PATCH v3 8/9] coresight: syscfg: Allow update of feature params from configfs
Date: Thu, 26 Nov 2020 10:17:43 -0700	[thread overview]
Message-ID: <20201126171743.GB757228@xps15> (raw)
In-Reply-To: <20201030175655.30126-9-mike.leach@linaro.org>

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@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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-11-26 17:19 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-30 17:56 [RFC PATCH v3 0/9] CoreSight complex config support; ETM strobing Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 1/9] coresight: syscfg: Initial coresight system configuration Mike Leach
2020-11-12 22:09   ` Mathieu Poirier
2020-11-13 17:27   ` Mathieu Poirier
2020-11-16 18:32   ` Mathieu Poirier
2020-11-26 18:35   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 2/9] coresight: syscfg: Add registration and feature loading for cs devices Mike Leach
2020-11-16 18:47   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 3/9] coresight: config: Add configuration and feature generic functions Mike Leach
2020-11-17 19:00   ` Mathieu Poirier
2020-11-19 22:29   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 4/9] coresight: etm-perf: update to handle configuration selection Mike Leach
2020-11-20 18:52   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 5/9] coresight: etm4x: Add complex configuration handlers to etmv4 Mike Leach
2020-11-23 21:58   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 6/9] coresight: config: Add preloaded configurations Mike Leach
2020-11-24 17:51   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 7/9] coresight: syscfg: Add initial configfs support Mike Leach
2020-11-24 20:57   ` Mathieu Poirier
2020-11-25 21:52   ` Mathieu Poirier
2020-11-26 16:51   ` Mathieu Poirier
2020-10-30 17:56 ` [RFC PATCH v3 8/9] coresight: syscfg: Allow update of feature params from configfs Mike Leach
2020-11-26 17:17   ` Mathieu Poirier [this message]
2020-12-24 19:21     ` Mike Leach
2020-10-30 17:56 ` [RFC PATCH v3 9/9] coresight: docs: Add documentation for CoreSight config Mike Leach
2020-11-26 17:52   ` Mathieu Poirier
2020-12-24 19:21     ` Mike Leach
2020-11-26 18:52 ` [RFC PATCH v3 0/9] CoreSight complex config support; ETM strobing Mathieu Poirier
2020-12-24 19:20   ` Mike Leach
2021-01-06 21:15     ` Mathieu Poirier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201126171743.GB757228@xps15 \
    --to=mathieu.poirier@linaro.org \
    --cc=coresight@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mike.leach@linaro.org \
    --cc=suzuki.poulose@arm.com \
    --cc=yabinc@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox