Hi Mathieu,
On 21/06/2019 16:53, Mathieu Poirier wrote:
Hi Suzuki,
On Fri, 21 Jun 2019 at 09:26, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 05/06/2019 20:03, Mike Leach wrote:
The CoreSight subsystem enables a path of devices from source to sink. Any CTI devices associated with the path devices must be enabled at the same time.
This patch adds an associated device element to the main coresight device structure, and uses this to create associations between the CTI and other devices based on the device tree data. The assoc dev element is used to enable CTI in conjunction with the path elements. CTI are reference counted so where a single CTI is associated with multiple elements on the path, it will be enabled on the first associated device enable, and disabled with the last associated device disable.
...
@@ -339,9 +378,14 @@ static int coresight_enable_source(struct coresight_device *csdev, u32 mode)
if (!csdev->enable) { if (source_ops(csdev)->enable) {
ret = source_ops(csdev)->enable(csdev, NULL, mode);
ret = coresight_control_assoc_ectdev(csdev, 1); if (ret) return ret;ret = source_ops(csdev)->enable(csdev, NULL, mode);if (ret) {coresight_control_assoc_ectdev(csdev, 0);return ret;}; } csdev->enable = true; }Instead of spilling the call for sink/link/source call backs, could we not add this to the coresight_enable/disable_path() where these gets called from ? For each device we could add "enable" associated device. This could be done for the source even. Otherwise looks good to me.
If I remember correctly that is how Mike proceeded in his first patchset and doing error handling got really messy. The advantage in handling associated devices in callbacks is that operations look atomic to the core, i.e they either succeed or fail regardless of whether the component itself or the associated device caused the fault. It is also consistent with what was done for CATU devices where handling is done locally.
Ok. Thanks for the explanation. Lets leave it as it is now then.
Cheers Suzuki