Hi Mathieu,
On Fri, 19 Feb 2021 at 18:43, Mathieu Poirier
<mathieu.poir...@linaro.org> wrote:
>
> [...]
>
> > +/**
> > + * 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_desc match_info;
> > + struct cscfg_csdev_feat_ops ops;
> > + struct list_head item;
> > +};
>
> I would call this structure cscfg_registered_csdev and move it to
> coresight-config.h. That way it is consistent with cscfg_config_csdev and
> cscfg_feature_csdev and located all in the same file.
>
I was trying to separate structures that are used to define
configurations and features, with those that are used to manage the
same. Hence, most things in coresight_config.h define configurations,
or their device loaded instance equivalents, and things in
coresight_syscfg.h are management items. I am happy to change the name
but would prefer is stay in coresight_syscfg.h
Thanks
Mike
> I may have to come back to this patch but for now it holds together.
>
> More comments to come on Monday.
>
> Thanks,
> Mathieu
>
> > +
> > /* 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 manager 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_desc *info,
> > + struct cscfg_csdev_feat_ops *ops);
> > +void cscfg_unregister_csdev(struct coresight_device *csdev);
> >
> > /**
> > * System configuration manager device.
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 976ec2697610..d0126ed326a6 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -219,6 +219,8 @@ 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.
> > + * @feature_csdev_list: List of complex feature programming added to the
> > device.
> > + * @config_csdev_list: List of system configurations added to the device.
> > */
> > struct coresight_device {
> > struct coresight_platform_data *pdata;
> > @@ -240,6 +242,9 @@ struct coresight_device {
> > int nr_links;
> > bool has_conns_grp;
> > bool ect_enabled; /* true only if associated ect device is enabled */
> > + /* system configuration and feature lists */
> > + struct list_head feature_csdev_list;
> > + struct list_head config_csdev_list;
> > };
> >
> > /*
> > --
> > 2.17.1
> >
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
