| Message ID | 1346813317-4030-3-git-send-email-koba@kmckk.co.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)