* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT @ 2014-07-17 15:23 Tomasz Figa 2014-07-17 15:32 ` Jason Cooper 2014-07-18 13:17 ` Jason Cooper 0 siblings, 2 replies; 6+ messages in thread From: Tomasz Figa @ 2014-07-17 15:23 UTC (permalink / raw) To: linux-arm-kernel Certain GIC implementation, namely those found on earlier, single cluster, Exynos SoCs, have registers mapped without per-CPU banking, which means that the driver needs to use different offset for each CPU. Currently the driver calculates the offset by multiplying value returned by cpu_logical_map() by CPU offset parsed from DT. This is correct when CPU topology is not specified in DT and aforementioned function returns core ID alone. However when DT contains CPU topology, the function changes to return cluster ID as well, which is non-zero on mentioned SoCs and so breaks the calculation in GIC driver. This patch fixes this by masking out cluster ID in CPU offset calculation so that only core ID is considered. Multi-cluster Exynos SoCs already have banked GIC implementations, so this simple fix should be enough. Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Signed-off-by: Tomasz Figa <t.figa@samsung.com> --- drivers/irqchip/irq-gic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index ddee133..5c15a09 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -40,6 +40,7 @@ #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#include <asm/cputype.h> #include <asm/irq.h> #include <asm/exception.h> #include <asm/smp_plat.h> @@ -901,7 +902,9 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, } for_each_possible_cpu(cpu) { - unsigned long offset = percpu_offset * cpu_logical_map(cpu); + u32 mpidr = cpu_logical_map(cpu); + u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0); + unsigned long offset = percpu_offset * core_id; *per_cpu_ptr(gic->dist_base.percpu_base, cpu) = dist_base + offset; *per_cpu_ptr(gic->cpu_base.percpu_base, cpu) = cpu_base + offset; } -- 1.9.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT 2014-07-17 15:23 [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT Tomasz Figa @ 2014-07-17 15:32 ` Jason Cooper 2014-07-17 15:40 ` Tomasz Figa 2014-07-18 13:17 ` Jason Cooper 1 sibling, 1 reply; 6+ messages in thread From: Jason Cooper @ 2014-07-17 15:32 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 17, 2014 at 05:23:44PM +0200, Tomasz Figa wrote: > Certain GIC implementation, namely those found on earlier, single > cluster, Exynos SoCs, have registers mapped without per-CPU banking, > which means that the driver needs to use different offset for each CPU. > > Currently the driver calculates the offset by multiplying value returned > by cpu_logical_map() by CPU offset parsed from DT. This is correct when > CPU topology is not specified in DT and aforementioned function returns > core ID alone. However when DT contains CPU topology, the function > changes to return cluster ID as well, which is non-zero on mentioned > SoCs and so breaks the calculation in GIC driver. > > This patch fixes this by masking out cluster ID in CPU offset > calculation so that only core ID is considered. Multi-cluster Exynos > SoCs already have banked GIC implementations, so this simple fix should > be enough. > > Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > --- > drivers/irqchip/irq-gic.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) iiuc, this was introduced by: db0d4db22a78d ARM: gic: allow GIC to support non-banked setups and so should be for v3.3 and up, correct? thx, Jason. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT 2014-07-17 15:32 ` Jason Cooper @ 2014-07-17 15:40 ` Tomasz Figa 2014-07-17 15:51 ` Jason Cooper 0 siblings, 1 reply; 6+ messages in thread From: Tomasz Figa @ 2014-07-17 15:40 UTC (permalink / raw) To: linux-arm-kernel Hi Jason, On 17.07.2014 17:32, Jason Cooper wrote: > On Thu, Jul 17, 2014 at 05:23:44PM +0200, Tomasz Figa wrote: >> Certain GIC implementation, namely those found on earlier, single >> cluster, Exynos SoCs, have registers mapped without per-CPU banking, >> which means that the driver needs to use different offset for each CPU. >> >> Currently the driver calculates the offset by multiplying value returned >> by cpu_logical_map() by CPU offset parsed from DT. This is correct when >> CPU topology is not specified in DT and aforementioned function returns >> core ID alone. However when DT contains CPU topology, the function >> changes to return cluster ID as well, which is non-zero on mentioned >> SoCs and so breaks the calculation in GIC driver. >> >> This patch fixes this by masking out cluster ID in CPU offset >> calculation so that only core ID is considered. Multi-cluster Exynos >> SoCs already have banked GIC implementations, so this simple fix should >> be enough. >> >> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >> --- >> drivers/irqchip/irq-gic.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) > > iiuc, this was introduced by: > > db0d4db22a78d ARM: gic: allow GIC to support non-banked setups > > and so should be for v3.3 and up, correct? Could be, although there was and still is no topology data specified in DT for affected Exynos SoCs. The need for it showed up just recently, so I'm not sure this is a regression to fix in older kernels. Best regards, Tomasz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT 2014-07-17 15:40 ` Tomasz Figa @ 2014-07-17 15:51 ` Jason Cooper 2014-07-17 15:58 ` Tomasz Figa 0 siblings, 1 reply; 6+ messages in thread From: Jason Cooper @ 2014-07-17 15:51 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 17, 2014 at 05:40:50PM +0200, Tomasz Figa wrote: > Hi Jason, > > On 17.07.2014 17:32, Jason Cooper wrote: > > On Thu, Jul 17, 2014 at 05:23:44PM +0200, Tomasz Figa wrote: > >> Certain GIC implementation, namely those found on earlier, single > >> cluster, Exynos SoCs, have registers mapped without per-CPU banking, > >> which means that the driver needs to use different offset for each CPU. > >> > >> Currently the driver calculates the offset by multiplying value returned > >> by cpu_logical_map() by CPU offset parsed from DT. This is correct when > >> CPU topology is not specified in DT and aforementioned function returns > >> core ID alone. However when DT contains CPU topology, the function > >> changes to return cluster ID as well, which is non-zero on mentioned > >> SoCs and so breaks the calculation in GIC driver. > >> > >> This patch fixes this by masking out cluster ID in CPU offset > >> calculation so that only core ID is considered. Multi-cluster Exynos > >> SoCs already have banked GIC implementations, so this simple fix should > >> be enough. > >> > >> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > >> Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > >> Signed-off-by: Tomasz Figa <t.figa@samsung.com> > >> --- > >> drivers/irqchip/irq-gic.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > > > > iiuc, this was introduced by: > > > > db0d4db22a78d ARM: gic: allow GIC to support non-banked setups > > > > and so should be for v3.3 and up, correct? > > Could be, although there was and still is no topology data specified in > DT for affected Exynos SoCs. The need for it showed up just recently, so > I'm not sure this is a regression to fix in older kernels. In my "the kernel and the dtb aren't tied together" quest, these are the kinds of things I like to see fixed in stable kernels. If a user needs to update a dtb, say to fix a bug, it's reasonable to use the newest one for a given board. After all, any new nodes won't change anything, since the driver in the kernel won't match the node. However, in this case, without this fix, a user upgrading to the newest dtb would get a broken system. So, this fix should be backported to prevent the breakage. Or, have I missed something in my analysis? thx, Jason. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT 2014-07-17 15:51 ` Jason Cooper @ 2014-07-17 15:58 ` Tomasz Figa 0 siblings, 0 replies; 6+ messages in thread From: Tomasz Figa @ 2014-07-17 15:58 UTC (permalink / raw) To: linux-arm-kernel On 17.07.2014 17:51, Jason Cooper wrote: > On Thu, Jul 17, 2014 at 05:40:50PM +0200, Tomasz Figa wrote: >> Hi Jason, >> >> On 17.07.2014 17:32, Jason Cooper wrote: >>> On Thu, Jul 17, 2014 at 05:23:44PM +0200, Tomasz Figa wrote: >>>> Certain GIC implementation, namely those found on earlier, single >>>> cluster, Exynos SoCs, have registers mapped without per-CPU banking, >>>> which means that the driver needs to use different offset for each CPU. >>>> >>>> Currently the driver calculates the offset by multiplying value returned >>>> by cpu_logical_map() by CPU offset parsed from DT. This is correct when >>>> CPU topology is not specified in DT and aforementioned function returns >>>> core ID alone. However when DT contains CPU topology, the function >>>> changes to return cluster ID as well, which is non-zero on mentioned >>>> SoCs and so breaks the calculation in GIC driver. >>>> >>>> This patch fixes this by masking out cluster ID in CPU offset >>>> calculation so that only core ID is considered. Multi-cluster Exynos >>>> SoCs already have banked GIC implementations, so this simple fix should >>>> be enough. >>>> >>>> Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >>>> Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> >>>> Signed-off-by: Tomasz Figa <t.figa@samsung.com> >>>> --- >>>> drivers/irqchip/irq-gic.c | 5 ++++- >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> iiuc, this was introduced by: >>> >>> db0d4db22a78d ARM: gic: allow GIC to support non-banked setups >>> >>> and so should be for v3.3 and up, correct? >> >> Could be, although there was and still is no topology data specified in >> DT for affected Exynos SoCs. The need for it showed up just recently, so >> I'm not sure this is a regression to fix in older kernels. > > In my "the kernel and the dtb aren't tied together" quest, these are the > kinds of things I like to see fixed in stable kernels. > > If a user needs to update a dtb, say to fix a bug, it's reasonable to > use the newest one for a given board. After all, any new nodes won't > change anything, since the driver in the kernel won't match the node. > > However, in this case, without this fix, a user upgrading to the newest > dtb would get a broken system. So, this fix should be backported to > prevent the breakage. Or, have I missed something in my analysis? This is correct when looking only at this issue. However I suspect such move would cause a breakage anyway, because DT stuff isn't that stable on Exynos side. I don't mind if this patch hits stable, though. Best regards, Tomasz ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT 2014-07-17 15:23 [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT Tomasz Figa 2014-07-17 15:32 ` Jason Cooper @ 2014-07-18 13:17 ` Jason Cooper 1 sibling, 0 replies; 6+ messages in thread From: Jason Cooper @ 2014-07-18 13:17 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 17, 2014 at 05:23:44PM +0200, Tomasz Figa wrote: > Certain GIC implementation, namely those found on earlier, single > cluster, Exynos SoCs, have registers mapped without per-CPU banking, > which means that the driver needs to use different offset for each CPU. > > Currently the driver calculates the offset by multiplying value returned > by cpu_logical_map() by CPU offset parsed from DT. This is correct when > CPU topology is not specified in DT and aforementioned function returns > core ID alone. However when DT contains CPU topology, the function > changes to return cluster ID as well, which is non-zero on mentioned > SoCs and so breaks the calculation in GIC driver. > > This patch fixes this by masking out cluster ID in CPU offset > calculation so that only core ID is considered. Multi-cluster Exynos > SoCs already have banked GIC implementations, so this simple fix should > be enough. > > Reported-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > --- > drivers/irqchip/irq-gic.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index ddee133..5c15a09 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -40,6 +40,7 @@ > #include <linux/irqchip/chained_irq.h> > #include <linux/irqchip/arm-gic.h> Applied to irqchip/urgent and Cc'd to stable for v3.3+ thx, Jason. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-18 13:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-17 15:23 [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT Tomasz Figa 2014-07-17 15:32 ` Jason Cooper 2014-07-17 15:40 ` Tomasz Figa 2014-07-17 15:51 ` Jason Cooper 2014-07-17 15:58 ` Tomasz Figa 2014-07-18 13:17 ` Jason Cooper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox