* Re: [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit)
2012-09-05 2:48 [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit) Tetsuyuki Kobayshi
@ 2012-09-05 2:58 ` Simon Horman
2012-09-05 4:25 ` Tetsuyuki Kobayashi
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2012-09-05 2:58 UTC (permalink / raw)
To: linux-sh
On Wed, Sep 05, 2012 at 11:48:37AM +0900, Tetsuyuki Kobayshi wrote:
> From: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>
> This patch enables PMU(Performance Monitoring Unit) for emev2 when compiled with CONFIG_HW_PERF_EVENTS.
Hi Kobayshi-san,
Is it appropriate to enable this in the defconfig?
If so, could you send an updated patch?
>
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
> arch/arm/mach-shmobile/setup-emev2.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm/mach-shmobile/setup-emev2.c b/arch/arm/mach-shmobile/setup-emev2.c
> index dae9aa6..f3ff171 100644
> --- a/arch/arm/mach-shmobile/setup-emev2.c
> +++ b/arch/arm/mach-shmobile/setup-emev2.c
> @@ -36,6 +36,7 @@
> #include <asm/mach/map.h>
> #include <asm/mach/time.h>
> #include <asm/hardware/gic.h>
> +#include <asm/pmu.h>
>
> static struct map_desc emev2_io_desc[] __initdata = {
> #ifdef CONFIG_SMP
> @@ -356,6 +357,28 @@ static struct platform_device gio4_device = {
> },
> };
>
> +#ifdef CONFIG_HW_PERF_EVENTS
> +static struct resource pmu_resources[] = {
> + [0] = {
> + .start = 152,
> + .end = 152,
> + .flags = IORESOURCE_IRQ,
> + },
> + [1] = {
> + .start = 153,
> + .end = 153,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device pmu_device = {
> + .name = "arm-pmu",
> + .id = ARM_PMU_DEVICE_CPU,
> + .num_resources = ARRAY_SIZE(pmu_resources),
> + .resource = pmu_resources,
> +};
> +#endif
> +
> static struct platform_device *emev2_early_devices[] __initdata = {
> &uart0_device,
> &uart1_device,
> @@ -370,6 +393,9 @@ static struct platform_device *emev2_late_devices[] __initdata = {
> &gio2_device,
> &gio3_device,
> &gio4_device,
> +#ifdef CONFIG_HW_PERF_EVENTS
> + &pmu_device,
> +#endif
> };
>
> void __init emev2_add_standard_devices(void)
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit)
2012-09-05 2:48 [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit) Tetsuyuki Kobayshi
2012-09-05 2:58 ` Simon Horman
@ 2012-09-05 4:25 ` Tetsuyuki Kobayashi
2012-09-05 8:20 ` Magnus Damm
2012-09-05 9:00 ` Tetsuyuki Kobayashi
3 siblings, 0 replies; 6+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-09-05 4:25 UTC (permalink / raw)
To: linux-sh
Simon-san,
(2012/09/05 11:58), Simon Horman wrote:
> On Wed, Sep 05, 2012 at 11:48:37AM +0900, Tetsuyuki Kobayshi wrote:
>> From: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>>
>> This patch enables PMU(Performance Monitoring Unit) for emev2 when compiled with CONFIG_HW_PERF_EVENTS.
>
> Hi Kobayshi-san,
>
> Is it appropriate to enable this in the defconfig?
> If so, could you send an updated patch?
>
OK. I will post v2 patch with defconfig.
>>
>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>> ---
>> arch/arm/mach-shmobile/setup-emev2.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/mach-shmobile/setup-emev2.c b/arch/arm/mach-shmobile/setup-emev2.c
>> index dae9aa6..f3ff171 100644
>> --- a/arch/arm/mach-shmobile/setup-emev2.c
>> +++ b/arch/arm/mach-shmobile/setup-emev2.c
>> @@ -36,6 +36,7 @@
>> #include <asm/mach/map.h>
>> #include <asm/mach/time.h>
>> #include <asm/hardware/gic.h>
>> +#include <asm/pmu.h>
>>
>> static struct map_desc emev2_io_desc[] __initdata = {
>> #ifdef CONFIG_SMP
>> @@ -356,6 +357,28 @@ static struct platform_device gio4_device = {
>> },
>> };
>>
>> +#ifdef CONFIG_HW_PERF_EVENTS
>> +static struct resource pmu_resources[] = {
>> + [0] = {
>> + .start = 152,
>> + .end = 152,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + [1] = {
>> + .start = 153,
>> + .end = 153,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +};
>> +
>> +static struct platform_device pmu_device = {
>> + .name = "arm-pmu",
>> + .id = ARM_PMU_DEVICE_CPU,
>> + .num_resources = ARRAY_SIZE(pmu_resources),
>> + .resource = pmu_resources,
>> +};
>> +#endif
>> +
>> static struct platform_device *emev2_early_devices[] __initdata = {
>> &uart0_device,
>> &uart1_device,
>> @@ -370,6 +393,9 @@ static struct platform_device *emev2_late_devices[] __initdata = {
>> &gio2_device,
>> &gio3_device,
>> &gio4_device,
>> +#ifdef CONFIG_HW_PERF_EVENTS
>> + &pmu_device,
>> +#endif
>> };
>>
>> void __init emev2_add_standard_devices(void)
>> --
>> 1.7.9.5
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit)
2012-09-05 2:48 [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit) Tetsuyuki Kobayshi
2012-09-05 2:58 ` Simon Horman
2012-09-05 4:25 ` Tetsuyuki Kobayashi
@ 2012-09-05 8:20 ` Magnus Damm
2012-09-05 9:00 ` Tetsuyuki Kobayashi
3 siblings, 0 replies; 6+ messages in thread
From: Magnus Damm @ 2012-09-05 8:20 UTC (permalink / raw)
To: linux-sh
Hello Kobayashi-san,
Many thanks for your patches!
On Tue, Sep 4, 2012 at 9:48 PM, Tetsuyuki Kobayshi <koba@kmckk.co.jp> wrote:
> From: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>
> This patch enables PMU(Performance Monitoring Unit) for emev2 when compiled with CONFIG_HW_PERF_EVENTS.
>
> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> ---
> arch/arm/mach-shmobile/setup-emev2.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm/mach-shmobile/setup-emev2.c b/arch/arm/mach-shmobile/setup-emev2.c
> index dae9aa6..f3ff171 100644
> --- a/arch/arm/mach-shmobile/setup-emev2.c
> +++ b/arch/arm/mach-shmobile/setup-emev2.c
> @@ -36,6 +36,7 @@
> #include <asm/mach/map.h>
> #include <asm/mach/time.h>
> #include <asm/hardware/gic.h>
> +#include <asm/pmu.h>
>
> static struct map_desc emev2_io_desc[] __initdata = {
> #ifdef CONFIG_SMP
> @@ -356,6 +357,28 @@ static struct platform_device gio4_device = {
> },
> };
>
> +#ifdef CONFIG_HW_PERF_EVENTS
> +static struct resource pmu_resources[] = {
> + [0] = {
> + .start = 152,
> + .end = 152,
> + .flags = IORESOURCE_IRQ,
> + },
> + [1] = {
> + .start = 153,
> + .end = 153,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device pmu_device = {
> + .name = "arm-pmu",
> + .id = ARM_PMU_DEVICE_CPU,
> + .num_resources = ARRAY_SIZE(pmu_resources),
> + .resource = pmu_resources,
> +};
> +#endif
> +
> static struct platform_device *emev2_early_devices[] __initdata = {
> &uart0_device,
> &uart1_device,
> @@ -370,6 +393,9 @@ static struct platform_device *emev2_late_devices[] __initdata = {
> &gio2_device,
> &gio3_device,
> &gio4_device,
> +#ifdef CONFIG_HW_PERF_EVENTS
> + &pmu_device,
> +#endif
Is there any special reason why you want to wrap the code in #ifdefs?
As you can see, that's not so common for arch/arm/mach-shmobile to use
#ifdefs.
Usually we try to avoid using #ifdefs for regular platform data since
it makes the code more unreadable. The idea behind this is that the
platform device will exist regardless of the configuration and this
rather low potentially additional memory overhead has so far been seen
as acceptable.
Also, is there some way you can use the PMU with the DT (device tree)?
If so then the per-soc device tree files should be updated as well. We
have been requested this from the ARM soc maintainers. Such changes
should be CC:ed to the ARM mailing list so they can see that we are
following the standard way and working with the device tree.
Thank you!
/ magnus
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit)
2012-09-05 2:48 [PATCH 2/2] ARM: shmobile: emev2: enable PMU(Performance Monitoring Unit) Tetsuyuki Kobayshi
` (2 preceding siblings ...)
2012-09-05 8:20 ` Magnus Damm
@ 2012-09-05 9:00 ` Tetsuyuki Kobayashi
3 siblings, 0 replies; 6+ messages in thread
From: Tetsuyuki Kobayashi @ 2012-09-05 9:00 UTC (permalink / raw)
To: linux-sh
Hello Magnus and Paul,
(2012/09/05 17:20), Magnus Damm wrote:
> Hello Kobayashi-san,
>
> Many thanks for your patches!
>
> On Tue, Sep 4, 2012 at 9:48 PM, Tetsuyuki Kobayshi <koba@kmckk.co.jp> wrote:
>> From: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>>
>> This patch enables PMU(Performance Monitoring Unit) for emev2 when compiled with CONFIG_HW_PERF_EVENTS.
>>
>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
>> ---
>> arch/arm/mach-shmobile/setup-emev2.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/mach-shmobile/setup-emev2.c b/arch/arm/mach-shmobile/setup-emev2.c
>> index dae9aa6..f3ff171 100644
>> --- a/arch/arm/mach-shmobile/setup-emev2.c
>> +++ b/arch/arm/mach-shmobile/setup-emev2.c
>> @@ -36,6 +36,7 @@
>> #include <asm/mach/map.h>
>> #include <asm/mach/time.h>
>> #include <asm/hardware/gic.h>
>> +#include <asm/pmu.h>
>>
>> static struct map_desc emev2_io_desc[] __initdata = {
>> #ifdef CONFIG_SMP
>> @@ -356,6 +357,28 @@ static struct platform_device gio4_device = {
>> },
>> };
>>
>> +#ifdef CONFIG_HW_PERF_EVENTS
>> +static struct resource pmu_resources[] = {
>> + [0] = {
>> + .start = 152,
>> + .end = 152,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + [1] = {
>> + .start = 153,
>> + .end = 153,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +};
>> +
>> +static struct platform_device pmu_device = {
>> + .name = "arm-pmu",
>> + .id = ARM_PMU_DEVICE_CPU,
>> + .num_resources = ARRAY_SIZE(pmu_resources),
>> + .resource = pmu_resources,
>> +};
>> +#endif
>> +
>> static struct platform_device *emev2_early_devices[] __initdata = {
>> &uart0_device,
>> &uart1_device,
>> @@ -370,6 +393,9 @@ static struct platform_device *emev2_late_devices[] __initdata = {
>> &gio2_device,
>> &gio3_device,
>> &gio4_device,
>> +#ifdef CONFIG_HW_PERF_EVENTS
>> + &pmu_device,
>> +#endif
>
> Is there any special reason why you want to wrap the code in #ifdefs?
> As you can see, that's not so common for arch/arm/mach-shmobile to use
> #ifdefs.
>
I thought I didn't like increasing code size ..
> Usually we try to avoid using #ifdefs for regular platform data since
> it makes the code more unreadable. The idea behind this is that the
> platform device will exist regardless of the configuration and this
> rather low potentially additional memory overhead has so far been seen
> as acceptable.
I see. Paul said the same, too.
I will post revised patch without #ifdefs.
>
> Also, is there some way you can use the PMU with the DT (device tree)?
> If so then the per-soc device tree files should be updated as well. We
> have been requested this from the ARM soc maintainers. Such changes
> should be CC:ed to the ARM mailing list so they can see that we are
> following the standard way and working with the device tree.
I have to study about device tree.
The differece between these 2 patches are only IRQ number.
It is better to get PMU IRQ number from DT.. But how?
Is there any example?
^ permalink raw reply [flat|nested] 6+ messages in thread