Mathieu, Suzuki, Leo,
Changes made largely in line with your suggestions. of_ support rationalised in later patches to use fwnode wherever possible. removed superfluous parameter from cti_enable/disable_hw() calls.
Thanks for the feedback.
Mike
On Tue, 1 Oct 2019 at 20:42, Mathieu Poirier mathieu.poirier@linaro.org wrote:
More on this - see below...
On Mon, 19 Aug 2019 at 15:14, Mike Leach mike.leach@linaro.org wrote:
+/*
- Platform data for the CTI does not have the same connection info
- as the trace path components. Implement specific function to
- avoid warnings of missing connections.
- */
+int of_get_coresight_cti_platform_data(struct device *dev,
struct coresight_platform_data *pdata)+{
int ret = 0;struct device_node *node = dev->of_node;struct cti_drvdata *drvdata = dev_get_drvdata(dev);/* get some CTI specifics */ret = of_cti_get_hw_data(dev, node, drvdata);return ret;+} +#else +inline int +of_get_coresight_cti_platform_data(struct device *dev,
struct coresight_platform_data *pdata)+{
return -ENOENT;+}
+#endif
+struct coresight_platform_data * +coresight_cti_get_platform_data(struct device *dev) +{
int ret = -ENOENT;struct coresight_platform_data *pdata = NULL;struct fwnode_handle *fwnode = dev_fwnode(dev);if (IS_ERR_OR_NULL(fwnode))goto error;pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);if (!pdata) {ret = -ENOMEM;goto error;}if (is_of_node(fwnode))ret = of_get_coresight_cti_platform_data(dev, pdata);I've been spending a fair amount of time looking at the code in patch 06. There is nothing wrong with it but in my opinion we can keep things as they currently are and do a massive refactoring when introducing support for ACPI or just change our ways now. We carry 'dev' through calls to of_cti_create_v8_connections() and of_cti_create_v8_etm_connections(). Both function carry a lot of code that will be common to the future ACPI support.
Taking a closer look at of_cti_create_v8_connections(), only of_cti_get_cpu_at_node() is OF dependent. I suggest to create a generic function that would do a is_of_node() and from there call of_cti_get_cpu_at_node(), opening an easy path to ACPI support. The same process applies to of_cti_create_v8_etm_connection(). I suggest to spin off a new function to check for the presence of the associated device that would do the right thing based on the FW method used. Same for getting the associated device. That way of_cti_create_v8_connections() and of_cti_create_v8_etm_connection() become cti_create_v8_connections() and cti_create_v8_etm_connection(), providing a common core and a quick path for ACPI support.
We will have to do something like that anyway, so might as well do it right now. You can also call me a bike shedder.
Mathieu
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK