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

[Robin Murphy <robin.murphy@arm.com>]

On 17/07/2026 1:12 am, Jonathan Cameron wrote:
> On Thu, 16 Jul 2026 15:56:36 +0100
> Robin Murphy <robin.murphy@arm.com> 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 <ilkka@os.amperecomputing.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> 
> 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.

Heh, feel free to try some cosmetic refactoring if you fancy - for 
starters, "if (!match) return 0" would save a whole needless level of 
indentation here - but this patch is purely about a functional 
improvement to the user-visible logging. I'd rather not tangle that up 
in subjective non-functional changes.

(And to be brutally honest I'm not much of a fan of the cleanup.h stuff 
anyway - maybe for complex nested resource acquisition, but it's not 
like there are any gotos here, so for the sake of ~3 lines I'd 
personally rather have locking that looks like locking...)

Thanks,
Robin.

> 
> 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);
>