[PATCH v2 3/4] perf/arm_cspmu: Improve sub-module error reporting
Robin Murphy
robin.murphy at arm.com
Fri Jul 17 09:16:16 PDT 2026
On 17/07/2026 1:12 am, Jonathan Cameron wrote:
> On Thu, 16 Jul 2026 15:56:36 +0100
> Robin Murphy <robin.murphy at 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 at os.amperecomputing.com>
>> Signed-off-by: Robin Murphy <robin.murphy at 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);
>
More information about the linux-arm-kernel
mailing list