On Wed, 16 Jan 2019 at 04:19, Mike Leach mike.leach@linaro.org wrote:
Hi Mathieu,
On Tue, 15 Jan 2019 at 17:46, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Hi Mike,
I have started to work on your set. I expect the process to take a few days and as such will explicitly let you know when I am done.
No problem.
On Wed, Jan 09, 2019 at 10:54:34PM +0000, Mike Leach wrote:
Adds Embedded Cross Trigger (ECT) type and CTI sub-type to CoreSight enums in main coresight headers.
Adds in new global functions for CTI drivers.
The goal of a changelog is to mention why you are doing something rather than what you are doing. With this patch it is a little hard to do that but see my comment below about how to introduce functions - proceeding as suggested will help write changelogs that fall within the guideines of the community.
Signed-off-by: Mike Leach mike.leach@linaro.org
include/linux/coresight.h | 42 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 46c67a764877..6f85ac0cec55 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -41,6 +41,7 @@ enum coresight_dev_type { CORESIGHT_DEV_TYPE_LINKSINK, CORESIGHT_DEV_TYPE_SOURCE, CORESIGHT_DEV_TYPE_HELPER,
CORESIGHT_DEV_TYPE_ECT,};
enum coresight_dev_subtype_sink { @@ -68,6 +69,12 @@ enum coresight_dev_subtype_helper { CORESIGHT_DEV_SUBTYPE_HELPER_CATU, };
+/* Embedded Cross Trigger (ECT) sub-types */ +enum coresight_dev_subtype_ect {
CORESIGHT_DEV_SUBTYPE_ECT_NONE,CORESIGHT_DEV_SUBTYPE_ECT_CTI,+};
/**
- union coresight_dev_subtype - further characterisation of a type
- @sink_subtype: type of sink this component is, as defined
@@ -78,6 +85,8 @@ enum coresight_dev_subtype_helper {
by @coresight_dev_subtype_source.- @helper_subtype: type of helper this component is, as defined
by @coresight_dev_subtype_helper.
- @ect_subtype: type of cross trigger this component is, as
This needs to be a tab rather than a series of spaces.
define by @coresight_dev_subtype_ects/define/defined
*/ union coresight_dev_subtype { /* We have some devices which acts as LINK and SINK */ @@ -87,6 +96,7 @@ union coresight_dev_subtype { }; enum coresight_dev_subtype_source source_subtype; enum coresight_dev_subtype_helper helper_subtype;
enum coresight_dev_subtype_ect ect_subtype;};
/** @@ -156,6 +166,8 @@ struct coresight_connection {
- @activated: 'true' only if a _sink_ has been activated. A sink can be activated but not yet enabled. Enabling for a _sink_ happens when a source has been selected for that it.
- @ect_dev: Associated cross trigger device. Not part of the trace data
*/
path or connections.struct coresight_device { struct coresight_connection *conns; @@ -169,6 +181,7 @@ struct coresight_device { bool orphan; bool enable; /* true only if configured as part of a path */ bool activated; /* true only if a sink is part of a path */
struct coresight_device *ect_dev;};
#define to_coresight_device(d) container_of(d, struct coresight_device, dev) @@ -177,6 +190,7 @@ struct coresight_device { #define sink_ops(csdev) csdev->ops->sink_ops #define link_ops(csdev) csdev->ops->link_ops #define helper_ops(csdev) csdev->ops->helper_ops +#define ect_ops(csdev) csdev->ops->ect_ops
/**
- struct coresight_ops_sink - basic operations for a sink
@@ -242,11 +256,26 @@ struct coresight_ops_helper { int (*disable)(struct coresight_device *csdev, void *data); };
+/**
- struct coresight_ops_ect - Ops for an embedded cross trigger device
- All operations could pass in a device specific data, which could
- help the ect device to determine what to do.
- @enable : Enable the device
- @disable : Disable the device
- */
+struct coresight_ops_ect {
int (*enable)(struct coresight_device *csdev, void *data);int (*disable)(struct coresight_device *csdev, void *data);+};
As pointed out by Suzuki using the existing CATU framework may be a better way but it is much too early in this set to make a final call on that. More to come later.
struct coresight_ops { const struct coresight_ops_sink *sink_ops; const struct coresight_ops_link *link_ops; const struct coresight_ops_source *source_ops; const struct coresight_ops_helper *helper_ops;
const struct coresight_ops_ect *ect_ops;};
#ifdef CONFIG_CORESIGHT @@ -290,14 +319,27 @@ static inline void coresight_disclaim_device_unlocked(void __iomem *base) {}
#ifdef CONFIG_OF extern int of_coresight_get_cpu(const struct device_node *node); +extern int of_coresight_ect_get_cpu(const struct device_node *node); extern struct coresight_platform_data * of_get_coresight_platform_data(struct device *dev, const struct device_node *node); +extern struct coresight_platform_data * +of_get_coresight_ect_platform_data(struct device *dev,
const struct device_node *node);+extern struct coresight_device * +of_coresight_get_cs_device_by_node(struct device_node *node); #else static inline int of_coresight_get_cpu(const struct device_node *node) { return 0; } +static inline int of_coresight_ect_get_cpu(const struct device_node *node) +{ return 0; } static inline struct coresight_platform_data *of_get_coresight_platform_data( struct device *dev, const struct device_node *node) { return NULL; } +static inline struct coresight_platform_data * +of_get_coresight_ect_platform_data(struct device *dev,
const struct device_node *node) { return NULL; }+static inline struct coresight_device *of_coresight_get_cs_device_by_node(
struct device_node *node) { return NULL; }#endif
Unfortunately you can't just add function stubs like that. Stubs and their function definition need to be introduced gradually as they are needed. I understand the end result will be the same but you will be called out on the public mailing list so might as well do it now.
OK - I was trying to avoid adding too much in one patch - incremental changes to existing code that enable a later block of functionality - but it seems I have gone a little too far,
Most, if not all the changes from patches 0003-0006 introduce code used in the main driver patch 0007.
These can easily be merged together if that is the accepted way to proceed.
Alternatively, if it is only the new functions that are the issue, and the data structure / header changes can be separate that can be done too - I'm just a little unsure here.
I have commented more on this in other patches. Ultimately it comes down to grouping code by functionality. So you can introduce a very minimal driver that doesn't much other than being discovered and probed. From there you incrementally add functionality along with the functions that implements it.
Touch base with me on IRC if things are still unclear or if you want more guidance on how to split things. I suggest to just come up with another set and we'll refine further from there.
Mathieu
Thanks
Mike
#endif
2.19.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK