[v2,04/10] drivers: perf: Rename riscv pmu driver

Message ID 20230512085321.13259-5-alexghiti@rivosinc.com
State New
Headers
Series riscv: Allow userspace to directly access perf counters |

Commit Message

Alexandre Ghiti May 12, 2023, 8:53 a.m. UTC
  In addition to being more pretty, it will be useful in upcoming commits
to distinguish those pmu drivers from the other pmu drivers.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 drivers/perf/riscv_pmu_legacy.c | 2 +-
 drivers/perf/riscv_pmu_sbi.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Andrew Jones May 31, 2023, 2:09 p.m. UTC | #1
On Fri, May 12, 2023 at 10:53:15AM +0200, Alexandre Ghiti wrote:
> In addition to being more pretty, it will be useful in upcoming commits
> to distinguish those pmu drivers from the other pmu drivers.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  drivers/perf/riscv_pmu_legacy.c | 2 +-
>  drivers/perf/riscv_pmu_sbi.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
> index 0d8c9d8849ee..ffe09d857366 100644
> --- a/drivers/perf/riscv_pmu_legacy.c
> +++ b/drivers/perf/riscv_pmu_legacy.c
> @@ -95,7 +95,7 @@ static void pmu_legacy_init(struct riscv_pmu *pmu)
>  	pmu->ctr_clear_idx = NULL;
>  	pmu->ctr_read = pmu_legacy_read_ctr;
>  
> -	perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
> +	perf_pmu_register(&pmu->pmu, RISCV_PMU_LEGACY_PDEV_NAME, PERF_TYPE_RAW);
>  }
>  
>  static int pmu_legacy_device_probe(struct platform_device *pdev)
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 70cb50fd41c2..3b0ee2148054 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -897,7 +897,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out_unregister;
>  
> -	ret = perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
> +	ret = perf_pmu_register(&pmu->pmu, RISCV_PMU_PDEV_NAME, PERF_TYPE_RAW);

Should we include "sbi" in this name?

>  	if (ret)
>  		goto out_unregister;
>  
> -- 
> 2.37.2
> 

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
  
Alexandre Ghiti June 15, 2023, 7:25 a.m. UTC | #2
On 31/05/2023 16:09, Andrew Jones wrote:
> On Fri, May 12, 2023 at 10:53:15AM +0200, Alexandre Ghiti wrote:
>> In addition to being more pretty, it will be useful in upcoming commits
>> to distinguish those pmu drivers from the other pmu drivers.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>> ---
>>   drivers/perf/riscv_pmu_legacy.c | 2 +-
>>   drivers/perf/riscv_pmu_sbi.c    | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
>> index 0d8c9d8849ee..ffe09d857366 100644
>> --- a/drivers/perf/riscv_pmu_legacy.c
>> +++ b/drivers/perf/riscv_pmu_legacy.c
>> @@ -95,7 +95,7 @@ static void pmu_legacy_init(struct riscv_pmu *pmu)
>>   	pmu->ctr_clear_idx = NULL;
>>   	pmu->ctr_read = pmu_legacy_read_ctr;
>>   
>> -	perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
>> +	perf_pmu_register(&pmu->pmu, RISCV_PMU_LEGACY_PDEV_NAME, PERF_TYPE_RAW);
>>   }
>>   
>>   static int pmu_legacy_device_probe(struct platform_device *pdev)
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 70cb50fd41c2..3b0ee2148054 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
>> @@ -897,7 +897,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto out_unregister;
>>   
>> -	ret = perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
>> +	ret = perf_pmu_register(&pmu->pmu, RISCV_PMU_PDEV_NAME, PERF_TYPE_RAW);
> Should we include "sbi" in this name?


I'd say that it is safe to do so and I understand your point, @Atish WDYT?


>
>>   	if (ret)
>>   		goto out_unregister;
>>   
>> -- 
>> 2.37.2
>>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
  
Atish Patra June 15, 2023, 8:34 a.m. UTC | #3
On Thu, Jun 15, 2023 at 12:25 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
>
> On 31/05/2023 16:09, Andrew Jones wrote:
> > On Fri, May 12, 2023 at 10:53:15AM +0200, Alexandre Ghiti wrote:
> >> In addition to being more pretty, it will be useful in upcoming commits
> >> to distinguish those pmu drivers from the other pmu drivers.
> >>
> >> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> >> ---
> >>   drivers/perf/riscv_pmu_legacy.c | 2 +-
> >>   drivers/perf/riscv_pmu_sbi.c    | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
> >> index 0d8c9d8849ee..ffe09d857366 100644
> >> --- a/drivers/perf/riscv_pmu_legacy.c
> >> +++ b/drivers/perf/riscv_pmu_legacy.c
> >> @@ -95,7 +95,7 @@ static void pmu_legacy_init(struct riscv_pmu *pmu)
> >>      pmu->ctr_clear_idx = NULL;
> >>      pmu->ctr_read = pmu_legacy_read_ctr;
> >>
> >> -    perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
> >> +    perf_pmu_register(&pmu->pmu, RISCV_PMU_LEGACY_PDEV_NAME, PERF_TYPE_RAW);
> >>   }
> >>
> >>   static int pmu_legacy_device_probe(struct platform_device *pdev)
> >> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> >> index 70cb50fd41c2..3b0ee2148054 100644
> >> --- a/drivers/perf/riscv_pmu_sbi.c
> >> +++ b/drivers/perf/riscv_pmu_sbi.c
> >> @@ -897,7 +897,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
> >>      if (ret)
> >>              goto out_unregister;
> >>
> >> -    ret = perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
> >> +    ret = perf_pmu_register(&pmu->pmu, RISCV_PMU_PDEV_NAME, PERF_TYPE_RAW);
> > Should we include "sbi" in this name?
>
>
> I'd say that it is safe to do so and I understand your point, @Atish WDYT?
>

Actually, the argument in perf_pmu_register is about the pmu instance
name rather than the driver name.
Both legacy & SBI PMU drivers are just ways to access the "cpu" pmu instance.

In future we may have separate drivers for counter delegation
extensions that won't use the SBI PMU extension
at all for supported hardware. However, the PMU would still be cpu pmu.

There will be different SoC PMU drivers which will have different
names because it will represent SoC PMU instead of cpu pmu.

>
> >
> >>      if (ret)
> >>              goto out_unregister;
> >>
> >> --
> >> 2.37.2
> >>
> > Otherwise,
> >
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
  

Patch

diff --git a/drivers/perf/riscv_pmu_legacy.c b/drivers/perf/riscv_pmu_legacy.c
index 0d8c9d8849ee..ffe09d857366 100644
--- a/drivers/perf/riscv_pmu_legacy.c
+++ b/drivers/perf/riscv_pmu_legacy.c
@@ -95,7 +95,7 @@  static void pmu_legacy_init(struct riscv_pmu *pmu)
 	pmu->ctr_clear_idx = NULL;
 	pmu->ctr_read = pmu_legacy_read_ctr;
 
-	perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
+	perf_pmu_register(&pmu->pmu, RISCV_PMU_LEGACY_PDEV_NAME, PERF_TYPE_RAW);
 }
 
 static int pmu_legacy_device_probe(struct platform_device *pdev)
diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
index 70cb50fd41c2..3b0ee2148054 100644
--- a/drivers/perf/riscv_pmu_sbi.c
+++ b/drivers/perf/riscv_pmu_sbi.c
@@ -897,7 +897,7 @@  static int pmu_sbi_device_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_unregister;
 
-	ret = perf_pmu_register(&pmu->pmu, "cpu", PERF_TYPE_RAW);
+	ret = perf_pmu_register(&pmu->pmu, RISCV_PMU_PDEV_NAME, PERF_TYPE_RAW);
 	if (ret)
 		goto out_unregister;