[RFC PATCH v3 2/9] coresight: syscfg: Add registration and feature loading for cs devices
Mike Leach
mike.leach at linaro.org
Thu Dec 24 14:21:11 EST 2020
HI MAthieu,
Thanks - I've done what you suggest in this patch.
Mike
On Mon, 16 Nov 2020 at 18:47, Mathieu Poirier
<mathieu.poirier at linaro.org> wrote:
>
> On Fri, Oct 30, 2020 at 05:56:48PM +0000, Mike Leach wrote:
> > API for individual devices to register with the syscfg management
> > system is added.
> >
> > Devices register with matching information, and any features or
> > configurations that match will be loaded into the device.
> >
> > The feature and configuration loading is extended so that on load these
> > are loaded into any currently registered devices. This allows
> > configuration loading after devices have been registered.
> >
> > Signed-off-by: Mike Leach <mike.leach at linaro.org>
> > ---
> > .../hwtracing/coresight/coresight-config.h | 120 ++++++
> > .../hwtracing/coresight/coresight-syscfg.c | 346 ++++++++++++++++++
> > .../hwtracing/coresight/coresight-syscfg.h | 20 +
> > include/linux/coresight.h | 7 +
> > 4 files changed, 493 insertions(+)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> > index 0d9ae1dc4517..8fd7ff991ced 100644
> > --- a/drivers/hwtracing/coresight/coresight-config.h
> > +++ b/drivers/hwtracing/coresight/coresight-config.h
> > @@ -167,4 +167,124 @@ struct cscfg_config_desc {
> > const u64 *presets; /* nr_presets * nr_total_params */
> > };
> >
> > +/**
> > + * config register instance - part of a loaded feature.
> > + * maps register values to csdev driver structures
> > + *
> > + * @value: value to use when setting feature on device / store for
> > + * readback of volatile values.
> > + * @drv_store: pointer to internal driver element used to set the value
> > + * in hardware.
> > + */
> > +struct cscfg_reg_csdev {
> > + struct cscfg_regval value;
> > + void *drv_store;
> > +};
> > +
> > +/**
> > + * config parameter instance - part of a loaded feature.
> > + *
> > + * @feat: parent feature
> > + * @reg: register value updated by this parameter.
> > + * @current_value: current value of parameter - may be set by user via
> > + * sysfs, or modified during device operation.
> > + * @val64: true if 64 bit value
> > + */
> > +struct cscfg_parameter_csdev {
> > + struct cscfg_feature_csdev *feat;
> > + struct cscfg_reg_csdev *reg;
> > + u64 current_value;
> > + bool val64;
> > +};
> > +
> > +/**
> > + * cscfg_feat_ops - standard operations for loaded features.
> > + *
> > + * CS config core provides basic defaults for these, which can be overridden by
> > + * device specific versions.
> > + *
> > + * @set_on_enable: Set the feature on the driver before hw enable.
> > + * @save_on_disable: Save any volatile register values after hw disable.
> > + * @reset: Reset feature to default values.
> > + */
> > +struct cscfg_feat_ops {
> > + int (*set_on_enable)(struct cscfg_feature_csdev *feat, const bool force_set);
> > + void (*save_on_disable)(struct cscfg_feature_csdev *feat, const bool force_save);
> > + void (*reset)(struct cscfg_feature_csdev *feat);
> > +};
> > +
> > +/**
> > + * Feature instance loaded into a CoreSight device.
> > + *
> > + * When a feature is loaded into a specific device, then this structure holds
> > + * the connections between the register / parameter values used and the
> > + * internal data structures that are written when the feature is enabled.
> > + *
> > + * Since applying a feature modifies internal data structures in the device,
> > + * then we have a reference to the device spinlock to protect access to these
> > + * structures (@csdev_spinlock).
> > + *
> > + * @desc: pointer to the static descriptor for this feature.
> > + * @csdev: parent CoreSight device instance.
> > + * @node: list entry into feature list for this device.
> > + * @csdev_spinlock: device spinlock from csdev instance..
> > + * @enabled: feature is enabled on this device.
> > + * @nr_params: number of parameters.
> > + * @params: current parameter values on this device
> > + * @nr_regs: number of registers to be programmed.
> > + * @regs: Programming details for the registers
> > + * @ops: standard ops to enable and disable features on devices.
> > + */
> > +struct cscfg_feature_csdev {
> > + const struct cscfg_feature_desc *desc;
> > + struct coresight_device *csdev;
> > + struct list_head node;
> > + spinlock_t *csdev_spinlock;
> > + bool enabled;
> > + int nr_params;
> > + struct cscfg_parameter_csdev *params;
> > + int nr_regs;
> > + struct cscfg_reg_csdev *regs;
> > + struct cscfg_feat_ops ops;
> > +};
> > +
> > +/**
> > + * Configuration instance when loaded into a CoreSight device.
> > + *
> > + * The instance contains references to loaded features on this device that are
> > + * used by the configuration.
> > + *
> > + * @desc: reference to the descriptor for this configuration
> > + * @csdev: parent coresight device for this configuration instance.
> > + * @node: list entry within the coresight device
> > + * @id_hash: hash value of configuration name used for selection.
> > + * @nr_feat: Number of features on this device that are used in the
> > + * configuration.
> > + * @feats: reference to the device features to enable.
> > + * @enabled: true if configuration is enabled on this device.
> > + */
> > +struct cscfg_config_csdev {
> > + const struct cscfg_config_desc *desc;
> > + struct coresight_device *csdev;
> > + struct list_head node;
> > + unsigned long id_hash;
> > + int nr_feat;
> > + struct cscfg_feature_csdev **feats;
> > + bool enabled;
> > +};
> > +
> > +/**
> > + * Coresight device operations..
> > + *
>
> Two dots
>
> > + * Registered coresight devices provide these operations to manage feature
> > + * instances compatible with the device hardware and drivers
> > + *
> > + * @load_feat: Pass a feature descriptor into the device and create the
> > + * create the loaded feature instance (struct cscfg_feature_csdev).
>
> "create the" is there twice.
>
> > + */
> > +struct cscfg_csdev_feat_ops {
> > + int (*load_feat)(struct coresight_device *csdev,
> > + struct cscfg_feature_csdev *feat);
> > +};
> > +
> > #endif /* _CORESIGHT_CORESIGHT_CONFIG_H */
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> > index 6f89541af7a0..53bc1d551171 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> > @@ -27,6 +27,198 @@ struct device *to_device_cscfg(void);
> >
> > /* load features and configuations into the lists */
> >
> > +/* get name feature instance from a coresight device list of features */
> > +static struct cscfg_feature_csdev *
> > +cscfg_get_feat_csdev(struct coresight_device *csdev, const char *name)
> > +{
> > + struct cscfg_feature_csdev *feat = NULL;
> > +
> > + list_for_each_entry(feat, &csdev->feat_list, node) {
> > + if (strcmp(feat->desc->name, name) == 0)
> > + return feat;
> > + }
> > + return NULL;
> > +}
> > +
> > +/* allocate the device config instance - with max number of used features */
> > +static struct cscfg_config_csdev *
> > +cscfg_alloc_csdev_cfg(struct coresight_device *csdev, int nr_feats)
> > +{
> > + struct cscfg_config_csdev *dev_cfg = NULL;
> > + struct device *dev = csdev->dev.parent;
> > +
> > + /* this is being allocated using the devm for the coresight device */
> > + dev_cfg = devm_kzalloc(dev, sizeof(struct cscfg_config_csdev), GFP_KERNEL);
> > + if (!dev_cfg)
> > + return NULL;
> > +
> > + dev_cfg->csdev = csdev;
> > + dev_cfg->feats = devm_kcalloc(dev, nr_feats,
> > + sizeof(struct cscfg_feature_csdev *),
> > + GFP_KERNEL);
> > + if (!dev_cfg->feats)
> > + dev_cfg = NULL;
> > + return dev_cfg;
> > +}
> > +
> > +/* check match info between used feature from the config and a regisered device */
> > +static bool cscfg_match_feat_info(struct cscfg_match_info *used_cmp,
> > + struct cscfg_match_info *reg_dev)
> > +{
> > + /* if flags don't match then fail early */
> > + if (!(used_cmp->match_flags & reg_dev->match_flags))
> > + return false;
> > +
> > + /* if input used_cmp has a name, then check this too */
> > + if (used_cmp->dev_name) {
> > + if (strcmp(used_cmp->dev_name, reg_dev->dev_name) != 0)
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > +/* Load a config into a device if there are feature matches between config and device */
> > +static int cscfg_add_csdev_cfg(struct coresight_device *csdev,
> > + struct cscfg_match_info *match_info,
> > + struct cscfg_config_desc *cfg_desc)
> > +{
> > + struct cscfg_config_csdev *dev_cfg = NULL;
> > + struct cscfg_config_feat_ref *feat_ref;
> > + struct cscfg_feature_csdev *feat;
> > + int checked;
> > +
> > + /* look at each required feature and see if it matches any feature on the device */
> > + for (checked = 0; checked < cfg_desc->nr_refs; checked++) {
> > + feat_ref = &cfg_desc->refs[checked];
> > + if (cscfg_match_feat_info(&feat_ref->match, match_info)) {
> > + /* device matched - get the feature */
> > + feat = cscfg_get_feat_csdev(csdev, feat_ref->name);
> > + if (!feat)
> > + return -EINVAL;
> > + if (!dev_cfg) {
> > + dev_cfg = cscfg_alloc_csdev_cfg(csdev, cfg_desc->nr_refs);
> > + if (!dev_cfg)
> > + return -ENOMEM;
> > + dev_cfg->desc = cfg_desc;
> > + }
> > + dev_cfg->feats[dev_cfg->nr_feat++] = feat;
> > + }
> > + }
> > + /* if matched features, add config to device.*/
> > + if (dev_cfg)
> > + list_add(&dev_cfg->node, &csdev->syscfg_list);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * Add the config to the set of registered devices - call with mutex locked.
> > + * Iterates through devices - any device that matches one or more of the
> > + * configuration features will load it, the others will ignore it.
> > + */
> > +static int cscfg_add_cfg_to_csdevs(struct cscfg_config_desc *cfg_desc)
> > +{
> > + struct cscfg_csdev_register *curr_item;
> > + int err;
> > +
> > + list_for_each_entry(curr_item, &cscfg_data.dev_list, item) {
> > + err = cscfg_add_csdev_cfg(curr_item->csdev, &curr_item->match_info, cfg_desc);
> > + if (err)
> > + return err;
> > + }
> > + return 0;
> > +}
> > +
> > +/*
> > + * Allocate a feature object for load into a csdev.
> > + * memory allocated using the csdev->dev object using devm managed allocator.
> > + */
> > +static struct cscfg_feature_csdev *
> > +cscfg_alloc_csdev_feat(struct coresight_device *csdev, struct cscfg_feature_desc *feat_desc)
> > +{
> > + struct cscfg_feature_csdev *feat = NULL;
> > + struct device *dev = csdev->dev.parent;
> > + int i;
> > +
> > + feat = devm_kzalloc(dev, sizeof(struct cscfg_feature_csdev), GFP_KERNEL);
> > + if (!feat)
> > + return NULL;
> > +
> > + /* parameters are optional - could be 0 */
> > + feat->nr_params = feat_desc->nr_params;
> > + if (feat->nr_params) {
> > + feat->params = devm_kcalloc(dev, feat->nr_params,
> > + sizeof(struct cscfg_parameter_csdev),
> > + GFP_KERNEL);
> > + if (!feat->params)
> > + return NULL;
> > + }
> > +
> > + /* always have registers to program */
> > + feat->nr_regs = feat_desc->nr_regs;
> > + feat->regs = devm_kcalloc(dev, feat->nr_regs,
> > + sizeof(struct cscfg_reg_csdev), GFP_KERNEL);
>
> Arrays feat->params and feat-regs are allocated but not initialised with
> anything. It would be nice to have a comment that mentions where values will be
> set.
>
> > + if (!feat->regs)
> > + return NULL;
> > +
> > + /* load the feature default values */
> > + feat->desc = feat_desc;
> > + feat->csdev = csdev;
> > + for (i = 0; i < feat->nr_params; i++)
> > + feat->params[i].feat = feat;
> > +
> > + return feat;
> > +}
> > +
> > +/* load one feature into one coresight device */
> > +static int cscfg_load_feat_csdev(struct coresight_device *csdev,
> > + struct cscfg_feature_desc *feat_desc,
> > + struct cscfg_csdev_feat_ops *ops)
> > +{
> > + struct cscfg_feature_csdev *feat_csdev;
> > + int err;
> > +
> > + if (!ops->load_feat)
> > + return -EINVAL;
> > +
> > + feat_csdev = cscfg_alloc_csdev_feat(csdev, feat_desc);
> > + if (!feat_csdev)
> > + return -ENOMEM;
> > +
> > + /* load the feature into the device - may modify default ops*/
> > + err = ops->load_feat(csdev, feat_csdev);
> > + if (err)
> > + return err;
> > +
> > + /* add to internal csdev feature list */
> > + spin_lock(&csdev->cfg_lock);
> > + list_add(&feat_csdev->node, &csdev->feat_list);
> > + spin_unlock(&csdev->cfg_lock);
> > +
> > + return 0;
> > +}
> > +
> > +
>
> Extra line
>
> > +/*
> > + * Add feature to any matching devices - call with mutex locked.
> > + * Iterates through devices - any device that matches the feature will be
> > + * called to load it.
> > + */
> > +static int cscfg_add_feat_to_csdevs(struct cscfg_feature_desc *feat_desc)
> > +{
> > + struct cscfg_csdev_register *curr_item;
> > + int err;
> > +
> > + list_for_each_entry(curr_item, &cscfg_data.dev_list, item) {
> > + if (curr_item->match_info.match_flags & feat_desc->match_flags) {
> > + err = cscfg_load_feat_csdev(curr_item->csdev, feat_desc, &curr_item->ops);
> > + if (err)
> > + return err;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > /* check feature list for a named feature - call with mutex locked. */
> > static bool cscfg_match_list_feat(const char *name)
> > {
> > @@ -55,6 +247,13 @@ static int cscfg_check_feat_for_cfg(struct cscfg_config_desc *cfg_desc)
> > */
> > static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
> > {
> > + int err;
> > +
> > + /* add feature to any matching registered devices */
> > + err = cscfg_add_feat_to_csdevs(feat_desc);
> > + if (err)
> > + return err;
> > +
> > list_add(&feat_desc->item, &cscfg_data.feat_list);
> >
> > return 0;
> > @@ -73,6 +272,11 @@ static int cscfg_load_config(struct cscfg_config_desc *cfg_desc)
> > if (err)
> > return err;
> >
> > + /* add config to any matching registered device */
> > + err = cscfg_add_cfg_to_csdevs(cfg_desc);
> > + if (err)
> > + return err;
> > +
> > list_add(&cfg_desc->item, &cscfg_data.config_list);
> >
> > return 0;
> > @@ -124,6 +328,148 @@ int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> > }
> > EXPORT_SYMBOL_GPL(cscfg_load_config_sets);
> >
> > +/* Handle coresight device registration and add configs and features to devices */
> > +
> > +/* iterate through config lists and load matching configs to device */
> > +static int cscfg_add_cfgs_csdev(struct coresight_device *csdev,
> > + struct cscfg_match_info *info)
> > +{
> > + struct cscfg_config_desc *curr_item;
> > + int err = 0;
> > +
> > + list_for_each_entry(curr_item, &cscfg_data.config_list, item) {
> > + err = cscfg_add_csdev_cfg(csdev, info, curr_item);
> > + if (err)
> > + break;
> > + }
> > + return err;
> > +}
> > +
> > +/* iterate through feature lists and load matching features to device */
> > +static int cscfg_add_feats_csdev(struct coresight_device *csdev,
> > + struct cscfg_match_info *info,
> > + struct cscfg_csdev_feat_ops *ops)
> > +{
> > + struct cscfg_feature_desc *curr_item;
> > + int err = 0;
> > +
> > + if (!ops->load_feat)
> > + return -EINVAL;
> > +
> > + list_for_each_entry(curr_item, &cscfg_data.feat_list, item) {
> > + if (curr_item->match_flags & info->match_flags) {
> > + err = cscfg_load_feat_csdev(csdev, curr_item, ops);
> > + if (err)
> > + break;
> > + }
> > + }
> > + return err;
> > +}
> > +
> > +/* Add coresight device to list and copy its matching info */
> > +static int cscfg_list_add_csdev(struct coresight_device *csdev,
> > + struct cscfg_match_info *info,
> > + struct cscfg_csdev_feat_ops *ops)
> > +{
> > + struct cscfg_csdev_register *list_entry;
> > + const char *name = NULL;
> > +
> > + /* allocate the list entry structure */
> > + list_entry = kzalloc(sizeof(struct cscfg_csdev_register), GFP_KERNEL);
> > + if (!list_entry)
> > + return -ENOMEM;
> > + if (info->dev_name) {
> > + name = kstrdup(info->dev_name, GFP_KERNEL);
> > + if (!name) {
> > + kfree(list_entry);
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + list_entry->csdev = csdev;
> > + list_entry->match_info.match_flags = info->match_flags;
> > + list_entry->match_info.dev_name = name;
> > + list_entry->ops.load_feat = ops->load_feat;
> > + list_add(&list_entry->item, &cscfg_data.dev_list);
> > +
> > + INIT_LIST_HEAD(&csdev->feat_list);
> > + INIT_LIST_HEAD(&csdev->syscfg_list);
> > + spin_lock_init(&csdev->cfg_lock);
> > + cscfg_data.nr_csdev++;
> > +
> > + return 0;
> > +}
> > +
> > +/* remove a coresight device from the list and free data */
> > +static void cscfg_list_remove_csdev(struct coresight_device *csdev)
> > +{
> > + struct cscfg_csdev_register *curr_item, *tmp;
> > +
> > + list_for_each_entry_safe(curr_item, tmp, &cscfg_data.dev_list, item) {
> > + if (curr_item->csdev == csdev) {
> > + list_del(&curr_item->item);
> > + kfree(curr_item->match_info.dev_name);
> > + kfree(curr_item);
> > + cscfg_data.nr_csdev--;
> > + break;
> > + }
> > + }
> > +}
> > +
> > +/* register a coresight device with the syscfg api */
> > +int cscfg_register_csdev(struct coresight_device *csdev,
> > + struct cscfg_match_info *info,
> > + struct cscfg_csdev_feat_ops *ops)
> > +{
> > + int ret = 0;
> > +
> > + mutex_lock(&cscfg_mutex);
> > +
> > + /* add device to list of registered devices */
> > + ret = cscfg_list_add_csdev(csdev, info, ops);
> > + if (ret)
> > + goto reg_csdev_unlock;
> > +
> > + /* now load any registered features and configs matching the device. */
> > + ret = cscfg_add_feats_csdev(csdev, info, ops);
> > + if (ret) {
> > + cscfg_list_remove_csdev(csdev);
> > + goto reg_csdev_unlock;
> > + }
> > +
> > + ret = cscfg_add_cfgs_csdev(csdev, info);
> > + if (ret) {
> > + cscfg_list_remove_csdev(csdev);
> > + goto reg_csdev_unlock;
> > + }
> > +
> > + pr_info("CSCFG registered %s", dev_name(&csdev->dev));
> > +
> > +reg_csdev_unlock:
> > + mutex_unlock(&cscfg_mutex);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(cscfg_register_csdev);
> > +
> > +/* remove coresight device and update counts - syscfg device at 0 */
> > +void cscfg_unregister_csdev(struct coresight_device *csdev)
> > +{
> > + struct cscfg_csdev_register *curr_item, *tmp;
> > +
> > + mutex_lock(&cscfg_mutex);
> > + list_for_each_entry_safe(curr_item, tmp, &cscfg_data.dev_list, item) {
> > + if (curr_item->csdev == csdev) {
> > + list_del(&curr_item->item);
> > + kfree(curr_item->match_info.dev_name);
> > + kfree(curr_item);
> > + cscfg_data.nr_csdev--;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&cscfg_mutex);
> > +}
> > +EXPORT_SYMBOL_GPL(cscfg_unregister_csdev);
> > +
> > /* Initialise system configuration management driver. */
> > /* only one of these */
> > static struct cscfg_device *cscfg_dev;
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> > index 5a7896426eab..ecf4aac7d712 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> > @@ -26,6 +26,22 @@ struct cscfg_api_data {
> > int nr_csdev;
> > };
> >
> > +/**
> > + * List entry for Coresight devices that are registered as supporting complex
> > + * config operations.
> > + *
> > + * @csdev: The registered device.
> > + * @match_info: The matching type information.
> > + * @ops: Operations supported by the registered device.
> > + * @item: list entry.
> > + */
> > +struct cscfg_csdev_register {
> > + struct coresight_device *csdev;
> > + struct cscfg_match_info match_info;
> > + struct cscfg_csdev_feat_ops ops;
> > + struct list_head item;
> > +};
> > +
> > /* internal core operations for cscfg */
> > int __init cscfg_init(void);
> > void __exit cscfg_exit(void);
> > @@ -33,6 +49,10 @@ void __exit cscfg_exit(void);
> > /* syscfg external API */
> > int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> > struct cscfg_feature_desc **feat_descs);
> > +int cscfg_register_csdev(struct coresight_device *csdev,
> > + struct cscfg_match_info *info,
> > + struct cscfg_csdev_feat_ops *ops);
> > +void cscfg_unregister_csdev(struct coresight_device *csdev);
> >
> > /* system configuration device driver and API */
> > /**
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 7d3c87e5b97c..bade9e8ac20c 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -189,6 +189,9 @@ struct coresight_sysfs_link {
> > * @nr_links: number of sysfs links created to other components from this
> > * device. These will appear in the "connections" group.
> > * @has_conns_grp: Have added a "connections" group for sysfs links.
> > + * @feat_list: List of complex feature programming added to the device.
> > + * @syscfg_list: List of system configurations added to the device.
> > + * @cfg_lock: spinlock to protect feature and config lists for this device.
> > */
> > struct coresight_device {
> > struct coresight_platform_data *pdata;
> > @@ -209,6 +212,10 @@ struct coresight_device {
> > int nr_links;
> > bool has_conns_grp;
> > bool ect_enabled; /* true only if associated ect device is enabled */
> > + /* complex config and feature lists */
> > + struct list_head feat_list;
> > + struct list_head syscfg_list;
>
> I would name the above feature_csdev_list and config_csdev_list. That way it
> matches the declaration of cscfg_feature_csdev and cscf_config_csdev and we know
> exactly what is enqueued on the lists.
>
> > + spinlock_t cfg_lock;
> > };
> >
> > /*
> > --
> > 2.17.1
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
More information about the linux-arm-kernel
mailing list