Re: [PATCH v3 2/6] cxl/region: Fix a checkpatch warning

[Jonathan Cameron]

On Tue, 10 Oct 2023 17:26:04 +0900
Jeongtae Park <jtp.park@xxxxxxxxxxx> wrote:

> WARNING: else is not generally useful after a break or return
> 
> Since cpu_cache_invalidate_memregion() already checks for
> support of invalidaton operation, it can be removed.
> This change would make more efficient or small codes
> when 'CONFIG_CXL_REGION_INVALIDATION_TEST' is not set.

Oddly short line wrap - aim for 75 chars ish.

> 
> Signed-off-by: Jeongtae Park <jtp.park@xxxxxxxxxxx>


> ---
>  drivers/cxl/core/region.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e115ba382e04..0eb7a12badb9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -127,21 +127,15 @@ static struct cxl_region_ref *cxl_rr_load(struct cxl_port *port,
>  
>  static int cxl_region_invalidate_memregion(struct cxl_region *cxlr)
>  {
> -	if (!cpu_cache_has_invalidate_memregion()) {
> -		if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) {
> -			dev_warn_once(
> -				&cxlr->dev,
> -				"Bypassing cpu_cache_invalidate_memregion() for testing!\n");
> -			return 0;
> -		} else {
> -			dev_err(&cxlr->dev,
> -				"Failed to synchronize CPU cache state\n");
> -			return -ENXIO;
> -		}
> +	if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)
> +			&& cpu_cache_has_invalidate_memregion()) {
> +		dev_warn_once(
> +			&cxlr->dev,
> +			"Bypassing cpu_cache_invalidate_memregion() for testing!\n");
> +		return 0;
>  	}
>  
> -	cpu_cache_invalidate_memregion(IORES_DESC_CXL);
> -	return 0;
> +	return cpu_cache_invalidate_memregion(IORES_DESC_CXL);

This is an arch specific call.  Whilst today on x86 (only option) the only way to
return an error is if it's not supported, that's not necessarily going to be the
case for other architectures. So I'd prefer to keep the dev_err if this fails.

Jonathan

>  }
>  
>  static int cxl_region_decode_reset(struct cxl_region *cxlr, int count)