[PATCH] drm/panfrost: Prefix interrupt handlers' names

[Robin Murphy]

On 13/12/2019 1:18 pm, Neil Armstrong wrote:
> Hi,
> 
> On 13/12/2019 13:39, Ezequiel Garcia wrote:
>> Currently, the interrupt lines requested by Panfrost
>> use ambiguous names, which adds some obscurity
>> to interrupt introspection (i.e. any tool based
>> on procfs' interrupts file).
>>
>> In order to improve this, prefix each requested
>> interrupt with either the module name or the device
>> name, where possible.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel at collabora.com>
>> ---
>>   drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
>>   drivers/gpu/drm/panfrost/panfrost_job.c | 2 +-
>>   drivers/gpu/drm/panfrost/panfrost_mmu.c | 6 ++++--
>>   3 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
>> index f67ed925c0ef..0355c4a78eaa 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
>> @@ -348,7 +348,7 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
>>   		return -ENODEV;
>>   
>>   	err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler,
>> -			       IRQF_SHARED, "gpu", pfdev);
>> +			       IRQF_SHARED, dev_name(pfdev->dev), pfdev);
>>   	if (err) {
>>   		dev_err(pfdev->dev, "failed to request gpu irq");
>>   		return err;
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c
>> index 05c85f45a0de..3bd79ebb6c40 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_job.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
>> @@ -480,7 +480,7 @@ int panfrost_job_init(struct panfrost_device *pfdev)
>>   		return -ENODEV;
>>   
>>   	ret = devm_request_irq(pfdev->dev, irq, panfrost_job_irq_handler,
>> -			       IRQF_SHARED, "job", pfdev);
>> +			       IRQF_SHARED, KBUILD_MODNAME "-job", pfdev);
>>   	if (ret) {
>>   		dev_err(pfdev->dev, "failed to request job irq");
>>   		return ret;
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> index 842bdd7cf6be..806958434726 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> @@ -612,9 +612,11 @@ int panfrost_mmu_init(struct panfrost_device *pfdev)
>>   	if (irq <= 0)
>>   		return -ENODEV;
>>   
>> -	err = devm_request_threaded_irq(pfdev->dev, irq, panfrost_mmu_irq_handler,
>> +	err = devm_request_threaded_irq(pfdev->dev, irq,
>> +					panfrost_mmu_irq_handler,
>>   					panfrost_mmu_irq_handler_thread,
>> -					IRQF_SHARED, "mmu", pfdev);
>> +					IRQF_SHARED, KBUILD_MODNAME "-mmu",
>> +					pfdev);
>>   
>>   	if (err) {
>>   		dev_err(pfdev->dev, "failed to request mmu irq");
>>
> 
> Why don't you use dev_name(pfdev->dev) everywhere ?

Agreed, while the current implementation may be confusing it is at least 
self-consistent. TBH it would probably be sufficient to save the bother 
of allocating strings and just settle on "panfrost-{gpu,job,mmu}", since 
upstream users are unlikely to ever come across a system with more than 
one Mali in it ;)

And FWIW note that "GPU" really is the specific hardware name of that 
IRQ output; it's not just a generic fill-in for "the one that isn't the 
Job or MMU IRQ".

Thanks,
Robin.