[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Ys28HemkGTnsmdYX@rosenzweig.io>
Date: Tue, 12 Jul 2022 18:23:25 +0000
From: Alyssa Rosenzweig <alyssa@...enzweig.io>
To: Konrad Dybcio <konrad.dybcio@...ainline.org>
Cc: ~postmarketos/upstreaming@...ts.sr.ht, martin.botka@...ainline.org,
angelogioacchino.delregno@...ainline.org,
marijn.suijten@...ainline.org, jamipkettunen@...ainline.org,
Hector Martin <marcan@...can.st>,
Sven Peter <sven@...npeter.dev>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] irqchip/apple-aic: Add support for A7-A11 SoCs
I don't see where you actually set the new features to false, or indeed
check the compatible at all? Am I missing a patch?
> Add support for A7-A11 SoCs by if-ing out some features only present on
> A12 & newer (UNCORE2 registers) or M1 & newer (EL2 registers - the
> older SoCs don't implement EL2).
>
> Also, annotate IPI regs support (A11 and newer*) so that the driver can
> tell whether the SoC supports these (they are written to even if fast
> IPI is disabled, when the registers are there of course).
>
> *A11 is supposed to use this feature, but it is currently not working.
> That said, it is not yet necessary, especially with only one core up,
> and it works a-ok using the same featureset as earlier SoCs.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@...ainline.org>
> ---
> drivers/irqchip/irq-apple-aic.c | 54 +++++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 12dd48727a15..36f4b52addc2 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -245,7 +245,10 @@ struct aic_info {
> u32 die_stride;
>
> /* Features */
> + bool el2_regs;
> bool fast_ipi;
> + bool ipi_regs;
> + bool uncore2_regs;
> };
>
> static const struct aic_info aic1_info = {
> @@ -261,7 +264,10 @@ static const struct aic_info aic1_fipi_info = {
> .event = AIC_EVENT,
> .target_cpu = AIC_TARGET_CPU,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct aic_info aic2_info = {
> @@ -269,7 +275,10 @@ static const struct aic_info aic2_info = {
>
> .irq_cfg = AIC2_IRQ_CFG,
>
> + .el2_regs = true,
> .fast_ipi = true,
> + .ipi_regs = true,
> + .uncore2_regs = true,
> };
>
> static const struct of_device_id aic_info_match[] = {
> @@ -452,6 +461,9 @@ static unsigned long aic_fiq_get_idx(struct irq_data *d)
>
> static void aic_fiq_set_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> /* Only the guest timers have real mask bits, unfortunately. */
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> @@ -469,6 +481,9 @@ static void aic_fiq_set_mask(struct irq_data *d)
>
> static void aic_fiq_clear_mask(struct irq_data *d)
> {
> + if (!aic_irqc->info.el2_regs)
> + return;
> +
> switch (aic_fiq_get_idx(d)) {
> case AIC_TMR_EL02_PHYS:
> sysreg_clear_set_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2, 0, VM_TMR_FIQ_ENABLE_P);
> @@ -524,12 +539,14 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
> * we check for everything here, even things we don't support yet.
> */
>
> - if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) {
> - if (static_branch_likely(&use_fast_ipi)) {
> - aic_handle_ipi(regs);
> - } else {
> - pr_err_ratelimited("Fast IPI fired. Acking.\n");
> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
> + if (aic_irqc->info.ipi_regs) {
> + if (read_sysreg_s(SYS_IMP_APL_IPI_SR_EL1) & IPI_SR_PENDING) {
> + if (static_branch_likely(&use_fast_ipi)) {
> + aic_handle_ipi(regs);
> + } else {
> + pr_err_ratelimited("Fast IPI fired. Acking.\n");
> + write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
> + }
> }
> }
>
> @@ -566,12 +583,14 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs)
> AIC_FIQ_HWIRQ(irq));
> }
>
> - if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ &&
> - (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
> - /* Same story with uncore PMCs */
> - pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n");
> - sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + if (aic_irqc->info.uncore2_regs) {
> + if (FIELD_GET(UPMCR0_IMODE, read_sysreg_s(SYS_IMP_APL_UPMCR0_EL1)) == UPMCR0_IMODE_FIQ &&
> + (read_sysreg_s(SYS_IMP_APL_UPMSR_EL1) & UPMSR_IACT)) {
> + /* Same story with uncore PMCs */
> + pr_err_ratelimited("Uncore PMC FIQ fired. Masking.\n");
> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + }
> }
> }
>
> @@ -676,7 +695,8 @@ static int aic_irq_domain_translate(struct irq_domain *id,
> break;
> case AIC_TMR_HV_PHYS:
> case AIC_TMR_HV_VIRT:
> - return -ENOENT;
> + if (aic_irqc->info.el2_regs)
> + return -ENOENT;
> default:
> break;
> }
> @@ -944,7 +964,8 @@ static int aic_init_cpu(unsigned int cpu)
> /* Mask all hard-wired per-CPU IRQ/FIQ sources */
>
> /* Pending Fast IPI FIQs */
> - write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
> + if (aic_irqc->info.ipi_regs)
> + write_sysreg_s(IPI_SR_PENDING, SYS_IMP_APL_IPI_SR_EL1);
>
> /* Timer FIQs */
> sysreg_clear_set(cntp_ctl_el0, 0, ARCH_TIMER_CTRL_IT_MASK);
> @@ -965,8 +986,9 @@ static int aic_init_cpu(unsigned int cpu)
> FIELD_PREP(PMCR0_IMODE, PMCR0_IMODE_OFF));
>
> /* Uncore PMC FIQ */
> - sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> - FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
> + if (aic_irqc->info.uncore2_regs)
> + sysreg_clear_set_s(SYS_IMP_APL_UPMCR0_EL1, UPMCR0_IMODE,
> + FIELD_PREP(UPMCR0_IMODE, UPMCR0_IMODE_OFF));
>
> /* Commit all of the above */
> isb();
> --
> 2.37.0
>
Powered by blists - more mailing lists