> On 13-Jul-2020, at 6:20 PM, Michael Ellerman <m...@ellerman.id.au> wrote: > > Athira Rajeev <atraj...@linux.vnet.ibm.com > <mailto:atraj...@linux.vnet.ibm.com>> writes: >>> On 08-Jul-2020, at 4:32 PM, Michael Ellerman <m...@ellerman.id.au> wrote: >>> >>> Athira Rajeev <atraj...@linux.vnet.ibm.com> writes: >>> ... >>>> diff --git a/arch/powerpc/perf/core-book3s.c >>>> b/arch/powerpc/perf/core-book3s.c >>>> index cd6a742..5c64bd3 100644 >>>> --- a/arch/powerpc/perf/core-book3s.c >>>> +++ b/arch/powerpc/perf/core-book3s.c >>>> @@ -39,10 +39,10 @@ struct cpu_hw_events { >>>> unsigned int flags[MAX_HWEVENTS]; >>>> /* >>>> * The order of the MMCR array is: >>>> - * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2 >>>> + * - 64-bit, MMCR0, MMCR1, MMCRA, MMCR2, MMCR3 >>>> * - 32-bit, MMCR0, MMCR1, MMCR2 >>>> */ >>>> - unsigned long mmcr[4]; >>>> + unsigned long mmcr[5]; >>>> struct perf_event *limited_counter[MAX_LIMITED_HWCOUNTERS]; >>>> u8 limited_hwidx[MAX_LIMITED_HWCOUNTERS]; >>>> u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES]; >>> ... >>>> @@ -1310,6 +1326,10 @@ static void power_pmu_enable(struct pmu *pmu) >>>> if (!cpuhw->n_added) { >>>> mtspr(SPRN_MMCRA, cpuhw->mmcr[2] & ~MMCRA_SAMPLE_ENABLE); >>>> mtspr(SPRN_MMCR1, cpuhw->mmcr[1]); >>>> +#ifdef CONFIG_PPC64 >>>> + if (ppmu->flags & PPMU_ARCH_310S) >>>> + mtspr(SPRN_MMCR3, cpuhw->mmcr[4]); >>>> +#endif /* CONFIG_PPC64 */ >>>> goto out_enable; >>>> } >>>> >>>> @@ -1353,6 +1373,11 @@ static void power_pmu_enable(struct pmu *pmu) >>>> if (ppmu->flags & PPMU_ARCH_207S) >>>> mtspr(SPRN_MMCR2, cpuhw->mmcr[3]); >>>> >>>> +#ifdef CONFIG_PPC64 >>>> + if (ppmu->flags & PPMU_ARCH_310S) >>>> + mtspr(SPRN_MMCR3, cpuhw->mmcr[4]); >>>> +#endif /* CONFIG_PPC64 */ >>> >>> I don't think you need the #ifdef CONFIG_PPC64? >> >> Hi Michael >> >> Thanks for reviewing this series. >> >> SPRN_MMCR3 is not defined for PPC32 and we hit build failure for >> pmac32_defconfig. >> The #ifdef CONFIG_PPC64 is to address this. > > We like to avoid #ifdefs in the body of the code like that. > > There's a bunch of existing #defines near the top of the file to make > 32-bit work, I think you should just add another for this, so eg: > > #ifdef CONFIG_PPC32 > ... > #define SPRN_MMCR3 0
Ok Ok. Found that currently we do same way as you mentioned for MMCRA which is not supported for 32-bit I will work on this change Thanks Athira > > cheers
