| Message ID | 20231006151547.3.Ie582d33cfe46f9ec2248e7f2dabdd6bbd66486a6@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] arm64: Disable GiC priorities on Mediatek devices w/ firmware issues | expand |
On Fri, Oct 06, 2023 at 03:15:53PM -0700, Douglas Anderson wrote: > This is a partial revert of commit 44bd78dd2b88 ("irqchip/gic-v3: > Disable pseudo NMIs on Mediatek devices w/ firmware issues"). In the > patch ("arm64: Disable GiC priorities on Mediatek devices w/ firmware > issues") we've moved the quirk handling to another place and so it's > not needed in the GiC driver. > > NOTE: this isn't a full revert because it leaves some of the changes > to the "quirks" structure around in case future code needs it. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- I think it might make sense to fold this into the patch adding the cpucap detection. Otherwise, if you apply my suggestions to the first patch, there's a 2-commit window where we'll have two places that log that NMI is being disabled due to the FW issue. That's not a functional issue, so doesn't matter that much. Either way: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > > drivers/irqchip/irq-gic-v3.c | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 787ccc880b22..9ff776709ae6 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -39,8 +39,7 @@ > > #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0) > #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1) > -#define FLAGS_WORKAROUND_MTK_GICR_SAVE (1ULL << 2) > -#define FLAGS_WORKAROUND_ASR_ERRATUM_8601001 (1ULL << 3) > +#define FLAGS_WORKAROUND_ASR_ERRATUM_8601001 (1ULL << 2) > > #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) > > @@ -1790,15 +1789,6 @@ static bool gic_enable_quirk_msm8996(void *data) > return true; > } > > -static bool gic_enable_quirk_mtk_gicr(void *data) > -{ > - struct gic_chip_data *d = data; > - > - d->flags |= FLAGS_WORKAROUND_MTK_GICR_SAVE; > - > - return true; > -} > - > static bool gic_enable_quirk_cavium_38539(void *data) > { > struct gic_chip_data *d = data; > @@ -1891,11 +1881,6 @@ static const struct gic_quirk gic_quirks[] = { > .compatible = "asr,asr8601-gic-v3", > .init = gic_enable_quirk_asr8601, > }, > - { > - .desc = "GICv3: Mediatek Chromebook GICR save problem", > - .property = "mediatek,broken-save-restore-fw", > - .init = gic_enable_quirk_mtk_gicr, > - }, > { > .desc = "GICv3: HIP06 erratum 161010803", > .iidr = 0x0204043b, > @@ -1957,11 +1942,6 @@ static void gic_enable_nmi_support(void) > if (!gic_prio_masking_enabled()) > return; > > - if (gic_data.flags & FLAGS_WORKAROUND_MTK_GICR_SAVE) { > - pr_warn("Skipping NMI enable due to firmware issues\n"); > - return; > - } > - > rdist_nmi_refs = kcalloc(gic_data.ppi_nr + SGI_NR, > sizeof(*rdist_nmi_refs), GFP_KERNEL); > if (!rdist_nmi_refs) > -- > 2.42.0.609.gbb76f46606-goog >
Hi, On Wed, Oct 18, 2023 at 4:08 AM Mark Rutland <mark.rutland@arm.com> wrote: > > On Fri, Oct 06, 2023 at 03:15:53PM -0700, Douglas Anderson wrote: > > This is a partial revert of commit 44bd78dd2b88 ("irqchip/gic-v3: > > Disable pseudo NMIs on Mediatek devices w/ firmware issues"). In the > > patch ("arm64: Disable GiC priorities on Mediatek devices w/ firmware > > issues") we've moved the quirk handling to another place and so it's > > not needed in the GiC driver. > > > > NOTE: this isn't a full revert because it leaves some of the changes > > to the "quirks" structure around in case future code needs it. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > I think it might make sense to fold this into the patch adding the cpucap > detection. Otherwise, if you apply my suggestions to the first patch, there's a > 2-commit window where we'll have two places that log that NMI is being disabled > due to the FW issue. That's not a functional issue, so doesn't matter that > much. > > Either way: > > Acked-by: Mark Rutland <mark.rutland@arm.com> I'm happy to go either way so I'd love some advice from maintainers (Marc Zyngier, Catalin Marinas, Will Deacon) about what you'd prefer. -Doug
On 2023-10-30 23:01, Doug Anderson wrote: > Hi, > > On Wed, Oct 18, 2023 at 4:08 AM Mark Rutland <mark.rutland@arm.com> > wrote: >> >> On Fri, Oct 06, 2023 at 03:15:53PM -0700, Douglas Anderson wrote: >> > This is a partial revert of commit 44bd78dd2b88 ("irqchip/gic-v3: >> > Disable pseudo NMIs on Mediatek devices w/ firmware issues"). In the >> > patch ("arm64: Disable GiC priorities on Mediatek devices w/ firmware >> > issues") we've moved the quirk handling to another place and so it's >> > not needed in the GiC driver. >> > >> > NOTE: this isn't a full revert because it leaves some of the changes >> > to the "quirks" structure around in case future code needs it. >> > >> > Signed-off-by: Douglas Anderson <dianders@chromium.org> >> > --- >> >> I think it might make sense to fold this into the patch adding the >> cpucap >> detection. Otherwise, if you apply my suggestions to the first patch, >> there's a >> 2-commit window where we'll have two places that log that NMI is being >> disabled >> due to the FW issue. That's not a functional issue, so doesn't matter >> that >> much. >> >> Either way: >> >> Acked-by: Mark Rutland <mark.rutland@arm.com> > > I'm happy to go either way so I'd love some advice from maintainers > (Marc Zyngier, Catalin Marinas, Will Deacon) about what you'd prefer. I honestly don't mind either way. The sooner we have these fixes upstream, the better, so my only advise would be to respin it shortly. Thanks, M.
On Tue, Nov 07, 2023 at 11:37:18AM +0000, Marc Zyngier wrote: > On 2023-10-30 23:01, Doug Anderson wrote: > > On Wed, Oct 18, 2023 at 4:08 AM Mark Rutland <mark.rutland@arm.com> > > wrote: > > > On Fri, Oct 06, 2023 at 03:15:53PM -0700, Douglas Anderson wrote: > > > > This is a partial revert of commit 44bd78dd2b88 ("irqchip/gic-v3: > > > > Disable pseudo NMIs on Mediatek devices w/ firmware issues"). In the > > > > patch ("arm64: Disable GiC priorities on Mediatek devices w/ firmware > > > > issues") we've moved the quirk handling to another place and so it's > > > > not needed in the GiC driver. > > > > > > > > NOTE: this isn't a full revert because it leaves some of the changes > > > > to the "quirks" structure around in case future code needs it. > > > > > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > > --- > > > > > > I think it might make sense to fold this into the patch adding the > > > cpucap > > > detection. Otherwise, if you apply my suggestions to the first > > > patch, there's a > > > 2-commit window where we'll have two places that log that NMI is > > > being disabled > > > due to the FW issue. That's not a functional issue, so doesn't > > > matter that > > > much. > > > > > > Either way: > > > > > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > > > I'm happy to go either way so I'd love some advice from maintainers > > (Marc Zyngier, Catalin Marinas, Will Deacon) about what you'd prefer. > > I honestly don't mind either way. The sooner we have these fixes > upstream, the better, so my only advise would be to respin it > shortly. I agree. I can queue them for rc1 if they turn up in time.
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 787ccc880b22..9ff776709ae6 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -39,8 +39,7 @@ #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0) #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1) -#define FLAGS_WORKAROUND_MTK_GICR_SAVE (1ULL << 2) -#define FLAGS_WORKAROUND_ASR_ERRATUM_8601001 (1ULL << 3) +#define FLAGS_WORKAROUND_ASR_ERRATUM_8601001 (1ULL << 2) #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) @@ -1790,15 +1789,6 @@ static bool gic_enable_quirk_msm8996(void *data) return true; } -static bool gic_enable_quirk_mtk_gicr(void *data) -{ - struct gic_chip_data *d = data; - - d->flags |= FLAGS_WORKAROUND_MTK_GICR_SAVE; - - return true; -} - static bool gic_enable_quirk_cavium_38539(void *data) { struct gic_chip_data *d = data; @@ -1891,11 +1881,6 @@ static const struct gic_quirk gic_quirks[] = { .compatible = "asr,asr8601-gic-v3", .init = gic_enable_quirk_asr8601, }, - { - .desc = "GICv3: Mediatek Chromebook GICR save problem", - .property = "mediatek,broken-save-restore-fw", - .init = gic_enable_quirk_mtk_gicr, - }, { .desc = "GICv3: HIP06 erratum 161010803", .iidr = 0x0204043b, @@ -1957,11 +1942,6 @@ static void gic_enable_nmi_support(void) if (!gic_prio_masking_enabled()) return; - if (gic_data.flags & FLAGS_WORKAROUND_MTK_GICR_SAVE) { - pr_warn("Skipping NMI enable due to firmware issues\n"); - return; - } - rdist_nmi_refs = kcalloc(gic_data.ppi_nr + SGI_NR, sizeof(*rdist_nmi_refs), GFP_KERNEL); if (!rdist_nmi_refs)
This is a partial revert of commit 44bd78dd2b88 ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues"). In the patch ("arm64: Disable GiC priorities on Mediatek devices w/ firmware issues") we've moved the quirk handling to another place and so it's not needed in the GiC driver. NOTE: this isn't a full revert because it leaves some of the changes to the "quirks" structure around in case future code needs it. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/irqchip/irq-gic-v3.c | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)