[PATCH v4 01/13] coresight: Fix loss of connection info when a module is unloaded

James Clark posted 13 patches 2 years, 10 months ago
There is a newer version of this series
[PATCH v4 01/13] coresight: Fix loss of connection info when a module is unloaded
Posted by James Clark 2 years, 10 months ago
child_fwnode should be a read only property based on the DT. If it's
cleared on the parent device when a child is unloaded, then when the
child is loaded again the connection won't be remade.

child_dev should be cleared instead which signifies that the connection
should be remade when the child_fwnode registers a new coresight_device.

Similarly the reference count shouldn't be decremented as long as the
parent device exists. The correct place to drop the reference is in
coresight_release_platform_data() which is already done.

Reproducible on Juno with the following steps:

  # load all coresight modules.
  $ cd /sys/bus/coresight/devices/
  $ echo 1 > tmc_etr0/enable_sink
  $ echo 1 > etm0/enable_source
  # Works fine ^

  $ echo 0 > etm0/enable_source
  $ rmmod coresight-funnel
  $ modprobe coresight-funnel
  $ echo 1 > etm0/enable_source
  -bash: echo: write error: Invalid argument

Fixes: 37ea1ffddffa ("coresight: Use fwnode handle instead of device names")
Fixes: 2af89ebacf29 ("coresight: Clear the connection field properly")
Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-core.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index d3bf82c0de1d..5733294ce5cd 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1419,13 +1419,8 @@ static int coresight_remove_match(struct device *dev, void *data)
 		if (csdev->dev.fwnode == conn->child_fwnode) {
 			iterator->orphan = true;
 			coresight_remove_links(iterator, conn);
-			/*
-			 * Drop the reference to the handle for the remote
-			 * device acquired in parsing the connections from
-			 * platform data.
-			 */
-			fwnode_handle_put(conn->child_fwnode);
-			conn->child_fwnode = NULL;
+
+			conn->child_dev = NULL;
 			/* No need to continue */
 			break;
 		}
-- 
2.34.1
Re: [PATCH v4 01/13] coresight: Fix loss of connection info when a module is unloaded
Posted by Suzuki K Poulose 2 years, 10 months ago
On 04/04/2023 14:53, James Clark wrote:
> child_fwnode should be a read only property based on the DT. If it's

minor nit: Not restricted to DT. This also covers the ACPI.

> cleared on the parent device when a child is unloaded, then when the
> child is loaded again the connection won't be remade.
> 
> child_dev should be cleared instead which signifies that the connection
> should be remade when the child_fwnode registers a new coresight_device.
> 
> Similarly the reference count shouldn't be decremented as long as the
> parent device exists. The correct place to drop the reference is in
> coresight_release_platform_data() which is already done.
> 
> Reproducible on Juno with the following steps:
> 
>    # load all coresight modules.
>    $ cd /sys/bus/coresight/devices/
>    $ echo 1 > tmc_etr0/enable_sink
>    $ echo 1 > etm0/enable_source
>    # Works fine ^
> 
>    $ echo 0 > etm0/enable_source
>    $ rmmod coresight-funnel
>    $ modprobe coresight-funnel
>    $ echo 1 > etm0/enable_source
>    -bash: echo: write error: Invalid argument
> 
> Fixes: 37ea1ffddffa ("coresight: Use fwnode handle instead of device names")
> Fixes: 2af89ebacf29 ("coresight: Clear the connection field properly")
> Signed-off-by: James Clark <james.clark@arm.com>

Looks good to me.

Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>