[PATCH v4 15/15] coresight: Move CPU hotplug callbacks to core layer

[James Clark]

On 19/11/2025 3:36 pm, Leo Yan wrote:
> On Wed, Nov 19, 2025 at 03:19:55PM +0000, James Clark wrote:
> 
> [...]
> 
>>>>> +static int coresight_dying_cpu(unsigned int cpu)
>>>>> +{
>>>>> +	struct coresight_device *source = per_cpu(csdev_source, cpu);
>>>>> +	struct coresight_path *path;
>>>>> +
>>>>> +	if (!source || !source->path)
>>>>> +		return 0;
>>>>> +
>>>>> +	/*
>>>>> +	 * The perf event layer will disable PMU events in the CPU hotplug.
>>>>> +	 * CoreSight driver should never handle the CS_MODE_PERF case.
>>>>> +	 */
>>>>> +	if (coresight_get_mode(source) != CS_MODE_SYSFS)
>>>>> +		return 0;
>>>>> +
>>>>> +	/*
>>>>> +	 * Save 'source->path' here, as it will be cleared in
>>>>> +	 * coresight_disable_source().
>>>>> +	 */
>>>>> +	path = source->path;
>>>>> +
>>>>> +	coresight_disable_source(source, NULL);
>>>>> +	coresight_disable_path(path);
>>>>> +	return 0;
>>>>
>>>> If the user is expected to re-enable and this new state is visible, don't
>>>> you need to use the regular coresight_disable_sysfs() function? It calls
>>>> coresight_disable_source_sysfs() which updates a refcount.
>>>
>>> Good point!  We only need to maintain refcnt for system tracers (e.g.,
>>> STM).  The per-CPU tracer is only binary states (on or off), we can
>>> simply use "mode" to track state and no need refcnt.
>>>
>>> I will use a separate patch to refactor refcnt.
>>
>> If you remove the refcount then wouldn't it break scripts that make multiple
>> enable calls? I thought the same logic for the system sources applies to CPU
>> sources.
> 
> I am not removing the refcount. Since we have csdev->mode to track the
> device state, I just keep refcount == 0 for the CPU source instead of
> refcount == 1.  This is transparnt for user space (no any change).
> 
> Before my change:
> 
>    echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 1
> 
>    echo 0 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
> After my change:
> 
>    echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
>    echo 1 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
>    echo 0 > /sys/bus/coresight/devices/etm2/enable_source
>    refcount = 0
> 
> As a result, the CPU PM flow doesn't need to bother refcount when to
> disable the path.
> 
> Thanks,
> Leo
> 

Oh ok I missed that it's not incremented for ETMs, that's fine then.

It's confusing that it behaves differently depending on some hidden 
property of the device. Not something that users should be expected to 
work out.

>> TBH I don't really know what problem the refcount and multiple balanced
>> enable/disable calls solves. But even if we decide it's not needed, it's too
>> late to change now.