| Message ID | 1341823542-15654-1-git-send-email-Varun.Sethi@freescale.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Kumar Gala |
| Headers | show |
On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: > Previously, these interrupts would be mapped, but the offset > calculation was broken, and only the first group was initialized. > > Signed-off-by: Scott Wood <scottwood@freescale.com> > --- > arch/powerpc/include/asm/mpic.h | 5 +++ > arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++----------- > 2 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h > index c9f698a..e14d35d 100644 > --- a/arch/powerpc/include/asm/mpic.h > +++ b/arch/powerpc/include/asm/mpic.h > @@ -63,6 +63,7 @@ > */ > #define MPIC_TIMER_BASE 0x01100 > #define MPIC_TIMER_STRIDE 0x40 > +#define MPIC_TIMER_GROUP_STRIDE 0x1000 > > #define MPIC_TIMER_CURRENT_CNT 0x00000 > #define MPIC_TIMER_BASE_CNT 0x00010 > @@ -110,6 +111,9 @@ > #define MPIC_VECPRI_SENSE_MASK 0x00400000 > #define MPIC_IRQ_DESTINATION 0x00010 > > +#define MPIC_FSL_BRR1 0x00000 > +#define MPIC_FSL_BRR1_VER 0x0000ffff > + > #define MPIC_MAX_IRQ_SOURCES 2048 > #define MPIC_MAX_CPUS 32 > #define MPIC_MAX_ISU 32 > @@ -296,6 +300,7 @@ struct mpic > phys_addr_t paddr; > > /* The various ioremap'ed bases */ > + struct mpic_reg_bank thiscpuregs; > struct mpic_reg_bank gregs; > struct mpic_reg_bank tmregs; > struct mpic_reg_bank cpuregs[MPIC_MAX_CPUS]; > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c > index 395af13..a98eb77 100644 > --- a/arch/powerpc/sysdev/mpic.c > +++ b/arch/powerpc/sysdev/mpic.c > @@ -6,7 +6,7 @@ > * with various broken implementations of this HW. > * > * Copyright (C) 2004 Benjamin Herrenschmidt, IBM Corp. > - * Copyright 2010-2011 Freescale Semiconductor, Inc. > + * Copyright 2010-2012 Freescale Semiconductor, Inc. > * > * This file is subject to the terms and conditions of the GNU General Public > * License. See the file COPYING in the main directory of this archive > @@ -221,24 +221,24 @@ static inline void _mpic_ipi_write(struct mpic *mpic, unsigned int ipi, u32 valu > _mpic_write(mpic->reg_type, &mpic->gregs, offset, value); > } > > -static inline u32 _mpic_tm_read(struct mpic *mpic, unsigned int tm) > +static inline unsigned int mpic_tm_offset(struct mpic *mpic, unsigned int tm) > { > - unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) + > - ((tm & 3) * MPIC_INFO(TIMER_STRIDE)); > + return (tm >> 2) * MPIC_TIMER_GROUP_STRIDE + > + (tm & 3) * MPIC_INFO(TIMER_STRIDE); > +} > > - if (tm >= 4) > - offset += 0x1000 / 4; > +static inline u32 _mpic_tm_read(struct mpic *mpic, unsigned int tm) > +{ > + unsigned int offset = mpic_tm_offset(mpic, tm) + > + MPIC_INFO(TIMER_VECTOR_PRI); > > return _mpic_read(mpic->reg_type, &mpic->tmregs, offset); > } > > static inline void _mpic_tm_write(struct mpic *mpic, unsigned int tm, u32 value) > { > - unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) + > - ((tm & 3) * MPIC_INFO(TIMER_STRIDE)); > - > - if (tm >= 4) > - offset += 0x1000 / 4; > + unsigned int offset = mpic_tm_offset(mpic, tm) + > + MPIC_INFO(TIMER_VECTOR_PRI); > > _mpic_write(mpic->reg_type, &mpic->tmregs, offset, value); > } > @@ -1301,6 +1301,16 @@ struct mpic * __init mpic_alloc(struct device_node *node, > mpic_map(mpic, mpic->paddr, &mpic->gregs, MPIC_INFO(GREG_BASE), 0x1000); > mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000); > > + if (mpic->flags & MPIC_FSL) { > + /* > + * Yes, Freescale really did put global registers in the > + * magic per-cpu area -- and they don't even show up in the > + * non-magic per-cpu copies that this driver normally uses. > + */ > + mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs, > + MPIC_CPU_THISBASE, 0x1000); > + } > + > /* Reset */ > > /* When using a device-node, reset requests are only honored if the MPIC > @@ -1440,6 +1450,7 @@ void __init mpic_assign_isu(struct mpic *mpic, unsigned int isu_num, > void __init mpic_init(struct mpic *mpic) > { > int i, cpu; > + int num_timers = 4; > > BUG_ON(mpic->num_sources == 0); > > @@ -1448,15 +1459,30 @@ void __init mpic_init(struct mpic *mpic) > /* Set current processor priority to max */ > mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf); > > + if (mpic->flags & MPIC_FSL) { > + u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, > + MPIC_FSL_BRR1); > + u32 version = brr1 & MPIC_FSL_BRR1_VER; > + > + /* > + * Timer group B is present at the latest in MPIC 3.1 (e.g. > + * mpc8536). It is not present in MPIC 2.0 (e.g. mpc8544). > + * I don't know about the status of intermediate versions (or > + * whether they even exist). > + */ > + if (version >= 0x0301) > + num_timers = 8; > + } > + Why don't we do this just via the device tree? > /* Initialize timers to our reserved vectors and mask them for now */ > - for (i = 0; i < 4; i++) { > + for (i = 0; i < num_timers; i++) { > + unsigned int offset = mpic_tm_offset(mpic, i); > + > mpic_write(mpic->tmregs, > - i * MPIC_INFO(TIMER_STRIDE) + > - MPIC_INFO(TIMER_DESTINATION), > + offset + MPIC_INFO(TIMER_DESTINATION), > 1 << hard_smp_processor_id()); > mpic_write(mpic->tmregs, > - i * MPIC_INFO(TIMER_STRIDE) + > - MPIC_INFO(TIMER_VECTOR_PRI), > + offset + MPIC_INFO(TIMER_VECTOR_PRI), > MPIC_VECPRI_MASK | > (9 << MPIC_VECPRI_PRIORITY_SHIFT) | > (mpic->timer_vecs[0] + i)); > -- > 1.7.2.2 >
On 07/09/2012 09:12 AM, Kumar Gala wrote: > > On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: > >> Previously, these interrupts would be mapped, but the offset >> calculation was broken, and only the first group was initialized. >> >> Signed-off-by: Scott Wood <scottwood@freescale.com> >> --- >> arch/powerpc/include/asm/mpic.h | 5 +++ >> arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++----------- >> 2 files changed, 47 insertions(+), 16 deletions(-) Varun, where's your signoff? >> + if (mpic->flags & MPIC_FSL) { >> + u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, >> + MPIC_FSL_BRR1); >> + u32 version = brr1 & MPIC_FSL_BRR1_VER; >> + >> + /* >> + * Timer group B is present at the latest in MPIC 3.1 (e.g. >> + * mpc8536). It is not present in MPIC 2.0 (e.g. mpc8544). >> + * I don't know about the status of intermediate versions (or >> + * whether they even exist). >> + */ >> + if (version >= 0x0301) >> + num_timers = 8; >> + } >> + > > Why don't we do this just via the device tree? Then we'd have to change existing device trees (again), and in general there's no reason to put it in the device tree if it's discoverable via hardware version registers. -Scott
On Jul 9, 2012, at 11:43 AM, Scott Wood wrote: > On 07/09/2012 09:12 AM, Kumar Gala wrote: >> >> On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: >> >>> Previously, these interrupts would be mapped, but the offset >>> calculation was broken, and only the first group was initialized. >>> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>> --- >>> arch/powerpc/include/asm/mpic.h | 5 +++ >>> arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++----------- >>> 2 files changed, 47 insertions(+), 16 deletions(-) > > Varun, where's your signoff? > >>> + if (mpic->flags & MPIC_FSL) { >>> + u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, >>> + MPIC_FSL_BRR1); >>> + u32 version = brr1 & MPIC_FSL_BRR1_VER; >>> + >>> + /* >>> + * Timer group B is present at the latest in MPIC 3.1 (e.g. >>> + * mpc8536). It is not present in MPIC 2.0 (e.g. mpc8544). >>> + * I don't know about the status of intermediate versions (or >>> + * whether they even exist). >>> + */ >>> + if (version >= 0x0301) >>> + num_timers = 8; >>> + } >>> + >> >> Why don't we do this just via the device tree? > > Then we'd have to change existing device trees (again), and in general > there's no reason to put it in the device tree if it's discoverable via > hardware version registers. Except for the whole AMP issue ;). One reason we did add the 2nd bank of timers was for AMP. Also, we have this in the .dts already: $ git grep pq3-mpic-timer-B.dtsi arch/powerpc/boot/dts/ arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/mpc8572si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p1010si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p1020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p1021si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p1022si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p1023si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" - k
On 07/09/2012 12:36 PM, Kumar Gala wrote: > > On Jul 9, 2012, at 11:43 AM, Scott Wood wrote: > >> On 07/09/2012 09:12 AM, Kumar Gala wrote: >>> >>> On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: >>> >>>> Previously, these interrupts would be mapped, but the offset >>>> calculation was broken, and only the first group was initialized. >>>> >>>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>>> --- >>>> arch/powerpc/include/asm/mpic.h | 5 +++ >>>> arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++----------- >>>> 2 files changed, 47 insertions(+), 16 deletions(-) >> >> Varun, where's your signoff? >> >>>> + if (mpic->flags & MPIC_FSL) { >>>> + u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, >>>> + MPIC_FSL_BRR1); >>>> + u32 version = brr1 & MPIC_FSL_BRR1_VER; >>>> + >>>> + /* >>>> + * Timer group B is present at the latest in MPIC 3.1 (e.g. >>>> + * mpc8536). It is not present in MPIC 2.0 (e.g. mpc8544). >>>> + * I don't know about the status of intermediate versions (or >>>> + * whether they even exist). >>>> + */ >>>> + if (version >= 0x0301) >>>> + num_timers = 8; >>>> + } >>>> + >>> >>> Why don't we do this just via the device tree? >> >> Then we'd have to change existing device trees (again), and in general >> there's no reason to put it in the device tree if it's discoverable via >> hardware version registers. > > Except for the whole AMP issue ;). One reason we did add the 2nd bank of timers was for AMP. What's the AMP issue? This is just for determining whether the timer IVPRs exist and can be requested. The interrupt should not be requested if the timer node isn't in the tree, unless the caller has other knowledge that it is valid (such as a KVM directly assigned interrupt). > Also, we have this in the .dts already: > > $ git grep pq3-mpic-timer-B.dtsi arch/powerpc/boot/dts/ > arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/mpc8572si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p1010si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p1020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p1021si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p1022si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p1023si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" > arch/powerpc/boot/dts/fsl/p2020si-post.dtsi:/include/ "pq3-mpic-timer-B.dtsi" Right, but what if we remove that node to keep the host from accessing those timers, and still want to assign it to a KVM guest? I thought you were asking for a property on the MPIC node. -Scott
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Monday, July 09, 2012 11:12 PM > To: Kumar Gala > Cc: Sethi Varun-B16395; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/3] powerpc/mpic: finish supporting timer group B on > Freescale chips > > On 07/09/2012 12:36 PM, Kumar Gala wrote: > > > > On Jul 9, 2012, at 11:43 AM, Scott Wood wrote: > > > >> On 07/09/2012 09:12 AM, Kumar Gala wrote: > >>> > >>> On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: > >>> > >>>> Previously, these interrupts would be mapped, but the offset > >>>> calculation was broken, and only the first group was initialized. > >>>> > >>>> Signed-off-by: Scott Wood <scottwood@freescale.com> > >>>> --- > >>>> arch/powerpc/include/asm/mpic.h | 5 +++ > >>>> arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++- > ---------- > >>>> 2 files changed, 47 insertions(+), 16 deletions(-) > >> Signed-off-by: Varun Sethi <varun.sethi@freescale.com>
On Jul 31, 2012, at 8:55 AM, Sethi Varun-B16395 wrote: > > >> -----Original Message----- >> From: Kumar Gala [mailto:galak@kernel.crashing.org] >> Sent: Tuesday, July 31, 2012 7:20 PM >> To: Sethi Varun-B16395 >> Cc: Wood Scott-B07421 >> Subject: Re: [PATCH 1/3] powerpc/mpic: finish supporting timer group B on >> Freescale chips >> >> >> On Jul 9, 2012, at 3:45 AM, Varun Sethi wrote: >> >>> Previously, these interrupts would be mapped, but the offset >>> calculation was broken, and only the first group was initialized. >>> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>> --- >>> arch/powerpc/include/asm/mpic.h | 5 +++ >>> arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++---- >> ------- >>> 2 files changed, 47 insertions(+), 16 deletions(-) >> >> Varun, >> >> Can you reply with a Signed-off-by to the thread. >> > Signed-off-by: Varun Sethi <varun.sethi@freescale.com> thanks applied to next - k
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index c9f698a..e14d35d 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -63,6 +63,7 @@ */ #define MPIC_TIMER_BASE 0x01100 #define MPIC_TIMER_STRIDE 0x40 +#define MPIC_TIMER_GROUP_STRIDE 0x1000 #define MPIC_TIMER_CURRENT_CNT 0x00000 #define MPIC_TIMER_BASE_CNT 0x00010 @@ -110,6 +111,9 @@ #define MPIC_VECPRI_SENSE_MASK 0x00400000 #define MPIC_IRQ_DESTINATION 0x00010 +#define MPIC_FSL_BRR1 0x00000 +#define MPIC_FSL_BRR1_VER 0x0000ffff + #define MPIC_MAX_IRQ_SOURCES 2048 #define MPIC_MAX_CPUS 32 #define MPIC_MAX_ISU 32 @@ -296,6 +300,7 @@ struct mpic phys_addr_t paddr; /* The various ioremap'ed bases */ + struct mpic_reg_bank thiscpuregs; struct mpic_reg_bank gregs; struct mpic_reg_bank tmregs; struct mpic_reg_bank cpuregs[MPIC_MAX_CPUS]; diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c index 395af13..a98eb77 100644 --- a/arch/powerpc/sysdev/mpic.c +++ b/arch/powerpc/sysdev/mpic.c @@ -6,7 +6,7 @@ * with various broken implementations of this HW. * * Copyright (C) 2004 Benjamin Herrenschmidt, IBM Corp. - * Copyright 2010-2011 Freescale Semiconductor, Inc. + * Copyright 2010-2012 Freescale Semiconductor, Inc. * * This file is subject to the terms and conditions of the GNU General Public * License. See the file COPYING in the main directory of this archive @@ -221,24 +221,24 @@ static inline void _mpic_ipi_write(struct mpic *mpic, unsigned int ipi, u32 valu _mpic_write(mpic->reg_type, &mpic->gregs, offset, value); } -static inline u32 _mpic_tm_read(struct mpic *mpic, unsigned int tm) +static inline unsigned int mpic_tm_offset(struct mpic *mpic, unsigned int tm) { - unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) + - ((tm & 3) * MPIC_INFO(TIMER_STRIDE)); + return (tm >> 2) * MPIC_TIMER_GROUP_STRIDE + + (tm & 3) * MPIC_INFO(TIMER_STRIDE); +} - if (tm >= 4) - offset += 0x1000 / 4; +static inline u32 _mpic_tm_read(struct mpic *mpic, unsigned int tm) +{ + unsigned int offset = mpic_tm_offset(mpic, tm) + + MPIC_INFO(TIMER_VECTOR_PRI); return _mpic_read(mpic->reg_type, &mpic->tmregs, offset); } static inline void _mpic_tm_write(struct mpic *mpic, unsigned int tm, u32 value) { - unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) + - ((tm & 3) * MPIC_INFO(TIMER_STRIDE)); - - if (tm >= 4) - offset += 0x1000 / 4; + unsigned int offset = mpic_tm_offset(mpic, tm) + + MPIC_INFO(TIMER_VECTOR_PRI); _mpic_write(mpic->reg_type, &mpic->tmregs, offset, value); } @@ -1301,6 +1301,16 @@ struct mpic * __init mpic_alloc(struct device_node *node, mpic_map(mpic, mpic->paddr, &mpic->gregs, MPIC_INFO(GREG_BASE), 0x1000); mpic_map(mpic, mpic->paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000); + if (mpic->flags & MPIC_FSL) { + /* + * Yes, Freescale really did put global registers in the + * magic per-cpu area -- and they don't even show up in the + * non-magic per-cpu copies that this driver normally uses. + */ + mpic_map(mpic, mpic->paddr, &mpic->thiscpuregs, + MPIC_CPU_THISBASE, 0x1000); + } + /* Reset */ /* When using a device-node, reset requests are only honored if the MPIC @@ -1440,6 +1450,7 @@ void __init mpic_assign_isu(struct mpic *mpic, unsigned int isu_num, void __init mpic_init(struct mpic *mpic) { int i, cpu; + int num_timers = 4; BUG_ON(mpic->num_sources == 0); @@ -1448,15 +1459,30 @@ void __init mpic_init(struct mpic *mpic) /* Set current processor priority to max */ mpic_cpu_write(MPIC_INFO(CPU_CURRENT_TASK_PRI), 0xf); + if (mpic->flags & MPIC_FSL) { + u32 brr1 = _mpic_read(mpic->reg_type, &mpic->thiscpuregs, + MPIC_FSL_BRR1); + u32 version = brr1 & MPIC_FSL_BRR1_VER; + + /* + * Timer group B is present at the latest in MPIC 3.1 (e.g. + * mpc8536). It is not present in MPIC 2.0 (e.g. mpc8544). + * I don't know about the status of intermediate versions (or + * whether they even exist). + */ + if (version >= 0x0301) + num_timers = 8; + } + /* Initialize timers to our reserved vectors and mask them for now */ - for (i = 0; i < 4; i++) { + for (i = 0; i < num_timers; i++) { + unsigned int offset = mpic_tm_offset(mpic, i); + mpic_write(mpic->tmregs, - i * MPIC_INFO(TIMER_STRIDE) + - MPIC_INFO(TIMER_DESTINATION), + offset + MPIC_INFO(TIMER_DESTINATION), 1 << hard_smp_processor_id()); mpic_write(mpic->tmregs, - i * MPIC_INFO(TIMER_STRIDE) + - MPIC_INFO(TIMER_VECTOR_PRI), + offset + MPIC_INFO(TIMER_VECTOR_PRI), MPIC_VECPRI_MASK | (9 << MPIC_VECPRI_PRIORITY_SHIFT) | (mpic->timer_vecs[0] + i));
Previously, these interrupts would be mapped, but the offset calculation was broken, and only the first group was initialized. Signed-off-by: Scott Wood <scottwood@freescale.com> --- arch/powerpc/include/asm/mpic.h | 5 +++ arch/powerpc/sysdev/mpic.c | 58 ++++++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 16 deletions(-)