Re: [PATCH v2 3/4] perf/arm_cspmu: Improve sub-module error reporting

On Thu, 16 Jul 2026 15:56:36 +0100
Robin Murphy <[email protected]> wrote:

> When waiting for a sub-module to register, we return a bare
> -EPROBE_DEFER that ends up showing the end user:
> 
>   platform arm-cs-arch-pmu.1: deferred probe pending (no reason)
> 
> wherein it's not necessarily clear that they might need to take some
> action to ensure the appropriate module is available to load. Let's use
> dev_err_probe() here so we can show exactly what we're waiting for.
> 
> Similarly, in the case where something's gone horribly wrong with an
> already-registered module, we can use dev_WARN() to standardise the
> device/driver attribution rather than just open-coding "arm_cspmu".
> 
> Reviewed-by: Ilkka Koskinen <[email protected]>
> Signed-off-by: Robin Murphy <[email protected]>

Hi Robin,

Maybe sprinkle some cleanup.h magic and use a guard(mutex)() and direct
returns.  Simplifies this function that you are touching anyway.

Looking more broadly it'd either take a scoped_guard() or some refactoring
if you want to go that way cross the file.

Jonathan

> 
> ---
> v2: No change
> ---
>  drivers/perf/arm_cspmu/arm_cspmu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 8c2dda17d73e..caad9a36651d 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -437,13 +437,15 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
>  				if (ret)
>  					module_put(match->module);
>  			} else {
> -				WARN(1, "arm_cspmu failed to get module: %s\n",
> +				dev_WARN(cspmu->dev, "Failed to get module: %s\n",
>  					match->module_name);
>  				ret = -EINVAL;
>  			}
>  		} else {
>  			request_module_nowait(match->module_name);
> -			ret = -EPROBE_DEFER;
> +			ret = dev_err_probe(cspmu->dev, -EPROBE_DEFER,
> +					    "Waiting for module %s to load\n",
> +					    match->module_name);
>  		}
>  
>  		mutex_unlock(&arm_cspmu_lock);