Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Fri, 22 Apr 2022 09:45:58 -0600
From:   Mathieu Poirier <mathieu.poirier@linaro.org>
To:     Shile Zhang <shile.zhang@linux.alibaba.com>
Cc:     Suzuki K Poulose <suzuki.poulose@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] coresight: etm4x: return 0 instead of using local ret
 variable
Message-ID: <20220422154558.GA1713394@p14s>
References: <20220422020239.37186-1-shile.zhang@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220422020239.37186-1-shile.zhang@linux.alibaba.com>
Precedence: bulk

On Fri, Apr 22, 2022 at 10:02:39AM +0800, Shile Zhang wrote:
> The function etm4_remove_dev() always return 0, and the former function
> etm4_remove has been changed to void in commit 3fd269e74f2fe ("amba: Make
> the remove callback return void"). But now its changed back to int type
> for some reason, which is different to the stable branch linux-5.10.y.

Please spend time understanding why function etm4_remove_dev()'s return value
has been changed back to an "int".  From there you will likely come to the
conclusion that adding the above to the changelog doesn't make sense.

> 
> Just let it return void and return 0 directly in it's caller function
> etm4_remove_platform_dev.

The only rational for this patch is that etm4_remove_dev() always returns '0'.
And even if it was to return anything else, the return value it not checked.
And even if the return value was checked, there is nothing to do about an error
condition since the driver is being removed.

> 
> Signed-off-by: Shile Zhang <shile.zhang@linux.alibaba.com>
> ---
> v2: re-work the commit log from Mathieu's suggestion.
> v1: https://lore.kernel.org/linux-arm-kernel/20220421164217.GB1596562@p14s/T/
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 7f416a12000eb..141f8209a152a 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -2104,7 +2104,7 @@ static void clear_etmdrvdata(void *info)
>  	etmdrvdata[cpu] = NULL;
>  }
>  
> -static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata)
> +static void __exit etm4_remove_dev(struct etmv4_drvdata *drvdata)
>  {
>  	etm_perf_symlink(drvdata->csdev, false);
>  	/*
> @@ -2125,8 +2125,6 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata)
>  
>  	cscfg_unregister_csdev(drvdata->csdev);
>  	coresight_unregister(drvdata->csdev);
> -
> -	return 0;
>  }
>  
>  static void __exit etm4_remove_amba(struct amba_device *adev)
> @@ -2139,13 +2137,14 @@ static void __exit etm4_remove_amba(struct amba_device *adev)
>  
>  static int __exit etm4_remove_platform_dev(struct platform_device *pdev)
>  {
> -	int ret = 0;
>  	struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
>  
>  	if (drvdata)
> -		ret = etm4_remove_dev(drvdata);
> +		etm4_remove_dev(drvdata);
> +
>  	pm_runtime_disable(&pdev->dev);
> -	return ret;
> +
> +	return 0;
>  }
>  
>  static const struct amba_id etm4_ids[] = {
> -- 
> 2.33.0.rc2
>