[RFC PATCH v3 3/9] coresight: config: Add configuration and feature generic functions
Mathieu Poirier
mathieu.poirier at linaro.org
Tue Nov 17 14:00:11 EST 2020
On Fri, Oct 30, 2020 at 05:56:49PM +0000, Mike Leach wrote:
> Adds a set of generic support functions that allow devices to set and save
> features values on the device, and enable and disable configurations.
>
> Additional functions for other common operations including feature
> reset.
>
> Signed-off-by: Mike Leach <mike.leach at linaro.org>
> ---
> drivers/hwtracing/coresight/Makefile | 2 +-
> .../hwtracing/coresight/coresight-config.c | 362 ++++++++++++++++++
> .../hwtracing/coresight/coresight-config.h | 14 +
> .../hwtracing/coresight/coresight-syscfg.c | 4 +
> 4 files changed, 381 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwtracing/coresight/coresight-config.c
>
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 4ce854c434b1..daad9f103a78 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -4,7 +4,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-sysfs.o coresight-syscfg.o coresight-config.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
> new file mode 100644
> index 000000000000..d911e0f083c1
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-config.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(C) 2020 Linaro Limited. All rights reserved.
> + * Author: Mike Leach <mike.leach at linaro.org>
> + */
> +
> +#include <linux/sysfs.h>
> +#include "coresight-config.h"
> +#include "coresight-priv.h"
> +
> +/*
> + * Write the value held in the register structure into the driver internal memory
> + * location.
> + */
> +static void cscfg_set_reg(struct cscfg_reg_csdev *reg)
> +{
> + u32 *p_val32 = (u32 *)reg->drv_store;
> + u32 tmp32;
> +
> + if (!(reg->value.flags & CS_CFG_REG_VAL_64BIT)) {
> + if (reg->value.flags & CS_CFG_REG_VAL_MASK) {
> + tmp32 = *p_val32;
> + tmp32 &= ~reg->value.mask32;
> + tmp32 |= reg->value.val32 & reg->value.mask32;
> + *p_val32 = tmp32;
> + } else
> + *p_val32 = reg->value.val32;
> + } else
> + *((u64 *)reg->drv_store) = reg->value.val64;
> +}
> +
> +/*
> + * Read the driver value into the reg if this is marked as one we want to save.
> + */
> +static void cscfg_save_reg(struct cscfg_reg_csdev *reg)
> +{
> + if (!(reg->value.flags & CS_CFG_REG_VAL_SAVE))
> + return;
> + if (reg->value.flags & CS_CFG_REG_VAL_64BIT)
> + reg->value.val64 = *(u64 *)(reg->drv_store);
> + else
> + reg->value.val32 = *(u32 *)(reg->drv_store);
> +}
> +
> +static inline void cscfg_init_reg_val(struct cscfg_reg_csdev *reg,
> + const struct cscfg_regval *desc)
> +{
> + reg->value.val64 = desc->val64;
> +}
> +
> +static inline void cscfg_init_reg_flags(struct cscfg_reg_csdev *reg,
> + const struct cscfg_regval *desc)
> +{
> + reg->value.flags = desc->flags;
> +}
> +
> +static void cscfg_init_reg_param(struct cscfg_parameter_csdev *param,
> + struct cscfg_reg_csdev *reg)
> +{
> + param->reg = reg;
> + param->val64 = reg->value.flags & CS_CFG_REG_VAL_64BIT;
> +
> + if (param->val64)
> + param->reg->value.val64 = param->current_value;
> + else
> + param->reg->value.val32 = (u32)param->current_value;
> +}
> +
> +/* default set - will set values without resource checking */
> +static int cscfg_set_on_enable(struct cscfg_feature_csdev *feat, const bool force_set)
> +{
> + int i;
> +
> + spin_lock(feat->csdev_spinlock);
> + if (feat->enabled || force_set) {
> + for (i = 0; i < feat->nr_regs; i++)
> + cscfg_set_reg(&feat->regs[i]);
> + }
> + dev_dbg(&feat->csdev->dev, "Feature %s: %s", feat->desc->name,
> + feat->enabled || force_set ? "Set enabled" : "Skip disabled");
> + spin_unlock(feat->csdev_spinlock);
> + return 0;
> +}
> +
> +static void cscfg_save_on_disable(struct cscfg_feature_csdev *feat, const bool force_save)
> +{
> + int i;
> +
> + spin_lock(feat->csdev_spinlock);
> + if (feat->enabled || force_save) {
> + for (i = 0; i < feat->nr_regs; i++)
> + cscfg_save_reg(&feat->regs[i]);
> + }
> + dev_dbg(&feat->csdev->dev, "Feature %s: %s", feat->desc->name,
> + feat->enabled || force_save ? "Save disabled" : "Skip disabled");
> + spin_unlock(feat->csdev_spinlock);
> +}
> +
> +/* default reset - restore default values, disable feature */
> +static void cscfg_reset_feat(struct cscfg_feature_csdev *feat)
> +{
> + struct cscfg_reg_csdev *reg;
> + struct cscfg_regval *reg_desc;
> + struct cscfg_parameter_csdev *param;
> + int i;
> +
> + spin_lock(feat->csdev_spinlock);
> + feat->enabled = false;
> +
> + /*
> + * set the default values for all parameters and regs from the
> + * relevant static descriptors.
> + */
> + for (i = 0; i < feat->nr_params; i++)
> + feat->params[i].current_value = feat->desc->params[i].value;
> +
> + for (i = 0; i < feat->nr_regs; i++) {
> + reg_desc = &feat->desc->regs[i];
> + reg = &feat->regs[i];
> + cscfg_init_reg_flags(reg, reg_desc);
> +
> + /* check if reg set from a parameter otherwise desc default */
> + if (reg_desc->flags & CS_CFG_REG_VAL_PARAM) {
> + param = &feat->params[reg_desc->val32];
> + cscfg_init_reg_param(param, reg);
> + } else
> + cscfg_init_reg_val(reg, reg_desc);
> + }
> + spin_unlock(feat->csdev_spinlock);
> +}
> +
> +void cscfg_set_def_ops(struct cscfg_feature_csdev *feat)
> +{
> + feat->ops.set_on_enable = cscfg_set_on_enable;
> + feat->ops.save_on_disable = cscfg_save_on_disable;
> + feat->ops.reset = cscfg_reset_feat;
> +}
> +
> +int cscfg_csdev_set_enabled_feats(struct coresight_device *csdev)
> +{
> + struct cscfg_feature_csdev *feat;
> + int err = 0;
> +
> + spin_lock(&csdev->cfg_lock);
> + if (list_empty(&csdev->feat_list)) {
> + spin_unlock(&csdev->cfg_lock);
> + return 0;
> + }
> +
> + list_for_each_entry(feat, &csdev->feat_list, node) {
> + dev_dbg(&csdev->dev, "Found feature:%s", feat->desc->name);
> +
> + if (feat->ops.set_on_enable) {
> + err = feat->ops.set_on_enable(feat, false);
> + dev_dbg(&csdev->dev, "Feature %s: %s",
> + feat->desc->name, err ? "Set failed" : "OK");
> + if (!err)
> + break;
> + }
> + }
> + spin_unlock(&csdev->cfg_lock);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(cscfg_csdev_set_enabled_feats);
> +
> +void cscfg_csdev_save_enabled_feats(struct coresight_device *csdev)
> +{
> + 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 (feat->ops.save_on_disable)
> + feat->ops.save_on_disable(feat, false);
> + }
> + spin_unlock(&csdev->cfg_lock);
> +}
> +EXPORT_SYMBOL_GPL(cscfg_csdev_save_enabled_feats);
> +
> +void cscfg_csdev_reset_feats(struct coresight_device *csdev)
> +{
> + 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 (feat->ops.reset)
> + feat->ops.reset(feat);
> + }
> + spin_unlock(&csdev->cfg_lock);
> +}
> +EXPORT_SYMBOL_GPL(cscfg_csdev_reset_feats);
> +
> +static int cscfg_update_presets(struct cscfg_config_csdev *cfg, int preset)
> +{
> + int i, j, line_offset = 0, val_idx = 0, max_idx;
> + u64 val;
> + struct cscfg_feature_csdev *feat;
> + struct cscfg_parameter_csdev *param;
> + const char *name;
int i, j, line_offset = 0, val_idx = 0, max_idx;
struct cscfg_parameter_csdev *param;
struct cscfg_feature_csdev *feat;
const char *name;
u64 val;
> +
> + if (preset > cfg->desc->nr_presets)
Extra space between if and ()
> + return -EINVAL;
> + /*
> + * Go through the array of features, assigning preset values to
> + * feature parameters in the order they appear.
> + * There should be precisely the same number of preset values as the
> + * sum of number of parameters over all the features - but we will
> + * ensure there is no overrun.
> + */
> + line_offset = (preset-1) * cfg->desc->nr_total_params;
If I understand correctly this computation won't work in situations where there
is more than one feature with varying number of parameters.
> + max_idx = cfg->desc->nr_total_params;
> + for (i = 0; i < cfg->nr_feat; i++) {
> + feat = cfg->feats[i];
> + if (feat->nr_params) {
if (!feat->nr_params)
continue;
> + spin_lock(feat->csdev_spinlock);
> + for (j = 0; j < feat->nr_params; j++) {
> + param = &feat->params[j];
> + name = feat->desc->params[j].name;
> + val = cfg->desc->presets[line_offset + val_idx++];
> + if (param->val64) {
> + dev_dbg(&cfg->csdev->dev,
> + "set param %s (%lld)", name, val);
> + param->reg->value.val64 = val;
> + } else {
> + param->reg->value.val32 = (u32)val;
> + dev_dbg(&cfg->csdev->dev,
> + "set param %s (%d)", name, (u32)val);
> + }
> + if (val_idx >= max_idx)
> + break;
> + }
> + spin_unlock(feat->csdev_spinlock);
> + }
> +
> + /* don't overrun the preset array line */
> + if (val_idx >= max_idx)
> + break;
> + }
> + return 0;
> +}
> +
> +/*
> + * if we are not using a preset, then need to update the feature params
> + * with current values.
> + */
> +static int cscfg_update_curr_params(struct cscfg_config_csdev *cfg)
> +{
> + int i, j;
> + struct cscfg_feature_csdev *feat;
> + struct cscfg_parameter_csdev *param;
> + const char *name;
> + u64 val;
> +
> + for (i = 0; i < cfg->nr_feat; i++) {
> + feat = cfg->feats[i];
> + if (feat->nr_params) {
if (!feat->nr_params)
continue;
> + spin_lock(feat->csdev_spinlock);
Please add a comment on why taking the spinlock is needed, i.e what kind of
action would corrupt the data.
> + for (j = 0; j < feat->nr_params; j++) {
> + param = &feat->params[j];
> + name = feat->desc->params[j].name;
> + val = param->current_value;
> + if (param->val64) {
> + dev_dbg(&cfg->csdev->dev,
> + "set param %s (%lld)", name, val);
> + param->reg->value.val64 = val;
> + } else {
> + param->reg->value.val32 = (u32)val;
> + dev_dbg(&cfg->csdev->dev,
> + "set param %s (%d)", name, (u32)val);
> + }
> + }
> + spin_unlock(feat->csdev_spinlock);
> + }
> + }
> + return 0;
> +}
> +
> +static int cscfg_prog_config(struct cscfg_config_csdev *cfg, bool enable)
> +{
> + int i, err = 0;
> + struct cscfg_feature_csdev *feat;
> + struct coresight_device *csdev;
> +
> + for (i = 0; i < cfg->nr_feat; i++) {
> + feat = cfg->feats[i];
> + csdev = feat->csdev;
> + dev_dbg(&csdev->dev, "cfg %s; %s feature:%s", cfg->desc->name,
> + enable ? "enable" : "disable", feat->desc->name);
> +
> + if (enable) {
> + if (feat->ops.set_on_enable)
> + err = feat->ops.set_on_enable(feat, true);
> + } else {
> + if (feat->ops.save_on_disable)
> + feat->ops.save_on_disable(feat, true);
> + }
> + if (err)
> + break;
> + }
> + return err;
> +}
> +
> +/**
> + * Enable configuration for the device.
> + * Match the config id and optionally set preset values for parameters.
> + *
> + * @csdev: coresight device to set config on.
> + * @cfg_id: hash id of configuration.
> + * @preset: preset values to use - 0 for default.
> + */
> +int cscfg_csdev_enable_config(struct coresight_device *csdev,
> + unsigned long cfg_id,
> + int preset)
Please avoid stacking when possible. Here @preset fits on the previous line.
The same applies for the function declaration in coresigt-config.h
I will continue with this patch tomorrow.
Thanks,
Mathieu
> +{
> + struct cscfg_config_csdev *cfg;
> + int err = 0;
> +
> + dev_dbg(&csdev->dev, "Check for config %lx, preset %d", cfg_id, preset);
> +
> + spin_lock(&csdev->cfg_lock);
> + list_for_each_entry(cfg, &csdev->syscfg_list, node) {
> + dev_dbg(&csdev->dev, "checking %s", cfg->desc->name);
> + if (cfg->id_hash == cfg_id) {
> + if (preset)
> + err = cscfg_update_presets(cfg, preset);
> + else
> + err = cscfg_update_curr_params(cfg);
> + if (!err)
> + err = cscfg_prog_config(cfg, true);
> + if (!err)
> + cfg->enabled = true;
> + break;
> + }
> + }
> + spin_unlock(&csdev->cfg_lock);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(cscfg_csdev_enable_config);
> +
> +void cscfg_csdev_disable_config(struct coresight_device *csdev)
> +{
> + struct cscfg_config_csdev *cfg;
> +
> + spin_lock(&csdev->cfg_lock);
> + list_for_each_entry(cfg, &csdev->syscfg_list, node) {
> + if (cfg->enabled) {
> + cscfg_prog_config(cfg, false);
> + cfg->enabled = false;
> + }
> + }
> + spin_unlock(&csdev->cfg_lock);
> +}
> +EXPORT_SYMBOL_GPL(cscfg_csdev_disable_config);
> diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> index 8fd7ff991ced..a3d374ebb70f 100644
> --- a/drivers/hwtracing/coresight/coresight-config.h
> +++ b/drivers/hwtracing/coresight/coresight-config.h
> @@ -287,4 +287,18 @@ struct cscfg_csdev_feat_ops {
> struct cscfg_feature_csdev *feat);
> };
>
> +/* coresight config API functions */
> +
> +/* helper functions for feature manipulation */
> +void cscfg_set_def_ops(struct cscfg_feature_csdev *feat);
> +
> +/* enable / disable features or configs on a device */
> +int cscfg_csdev_set_enabled_feats(struct coresight_device *csdev);
> +void cscfg_csdev_save_enabled_feats(struct coresight_device *csdev);
> +void cscfg_csdev_reset_feats(struct coresight_device *csdev);
> +int cscfg_csdev_enable_config(struct coresight_device *csdev,
> + unsigned long cfg_id,
> + int preset);
> +void cscfg_csdev_disable_config(struct coresight_device *csdev);
> +
> #endif /* _CORESIGHT_CORESIGHT_CONFIG_H */
> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> index 53bc1d551171..98ee78e3412a 100644
> --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> @@ -166,6 +166,7 @@ cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc
> feat->csdev = csdev;
> for (i = 0; i < feat->nr_params; i++)
> feat->params[i].feat = feat;
> + cscfg_set_def_ops(feat);
>
> return feat;
> }
> @@ -195,6 +196,9 @@ static int cscfg_load_feat_csdev(struct coresight_device *csdev,
> list_add(&feat_csdev->node, &csdev->feat_list);
> spin_unlock(&csdev->cfg_lock);
>
> + /* force load of default parameter values into registers */
> + feat_csdev->ops.reset(feat_csdev);
> +
> return 0;
> }
>
> --
> 2.17.1
>
More information about the linux-arm-kernel
mailing list