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

Robin Murphy <[email protected]>
Newsgroups org.kernel.vger.linux-acpi,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-perf-users
Message-ID <[email protected]>
On 17/07/2026 1:12 am, Jonathan Cameron wrote:
> 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.

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);
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.