Re: [PATCH v3 8/8] coresight: Unify error handling in coresight_register()

[James Clark <james.clark@linaro.org>]

On 02/02/2026 9:34 am, Leo Yan wrote:
> Unify error handling during registration:
> 
> 1) Failures before device registration are handled by err_out, which
>     releases platform data.
> 
> 2) Jump to the out_unlock label on failures after device registration
>     to unwind the flow via coresight_unregister().
> 
> The "registered" variable is no longer used, remove it.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>   drivers/hwtracing/coresight/coresight-core.c | 17 +++++------------
>   1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 955af43010446803030973c72f07315492b2fcf3..65cf975493c86de42515845147d90497aa20c595 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1326,7 +1326,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   {
>   	int ret;
>   	struct coresight_device *csdev;
> -	bool registered = false;
>   
>   	csdev = kzalloc(sizeof(*csdev), GFP_KERNEL);
>   	if (!csdev) {
> @@ -1380,7 +1379,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   		 * All resources are free'd explicitly via
>   		 * coresight_device_release(), triggered from put_device().
>   		 */
> -		goto out_unlock;
> +		mutex_unlock(&coresight_mutex);
> +		goto err_out;

I'm not sure if replacing the "registered" system with extra calls to 
unlock is necessarily better. I think the whole point of out_unlock was 
to have a single call to unlock so it couldn't be forgotten or didn't 
need to be duplicated.

Probably a better way to clean this up would be to pull out a function 
for all the stuff that needs to be locked and use guard(). Then do the 
stuff that doesn't need to be locked after that function. Either way it 
doesn't look wrong.

>   	}
>   
>   	ret = etm_perf_add_symlink_sink(csdev);
> @@ -1390,12 +1390,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   	 * coresight_device_release() triggered from put_device(), which is in
>   	 * turn called from function device_unregister().
>   	 */
> -	if (ret && ret != -EOPNOTSUPP) {
> -		device_unregister(&csdev->dev);
> +	if (ret && ret != -EOPNOTSUPP)
>   		goto out_unlock;
> -	}
> -	/* Device is now registered */
> -	registered = true;
>   
>   	ret = coresight_create_conns_sysfs_group(csdev);
>   	if (ret)
> @@ -1415,11 +1411,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   out_unlock:
>   	mutex_unlock(&coresight_mutex);
>   
> -	/* Unregister the device if needed */
> -	if (registered) {
> -		coresight_unregister(csdev);
> -		return ERR_PTR(ret);
> -	}
> +	coresight_unregister(csdev);
> +	return ERR_PTR(ret);
>   
>   err_out:
>   	coresight_release_platform_data(desc->dev, desc->pdata);
>