Date:   Mon, 13 Dec 2021 14:43:19 +0000
Message-ID: <87o85kk1ko.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Hector Martin <marcan@marcan.st>
Cc:     linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
        Will Deacon <will@kernel.org>, Sven Peter <sven@svenpeter.dev>,
        Alyssa Rosenzweig <alyssa@rosenzweig.io>,
        Rob Herring <robh+dt@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Dougall <dougallj@gmail.com>, kernel-team@android.com
Subject: Re: [PATCH v2 3/8] irqchip/apple-aic: Add cpumasks for E and P cores
In-Reply-To: <8bb14854-3377-4901-aaba-1a124c57cbec@marcan.st>
References: <20211201134909.390490-1-maz@kernel.org>
        <20211201134909.390490-4-maz@kernel.org>
        <8bb14854-3377-4901-aaba-1a124c57cbec@marcan.st>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Sun, 12 Dec 2021 07:30:20 +0000,
Hector Martin <marcan@marcan.st> wrote:
> 
> On 01/12/2021 22.49, Marc Zyngier wrote:
> > In order to be able to tell the core IRQ code about the affinity
> > of the PMU interrupt in later patches, compute the cpumasks of the
> > P and E cores at boot time.
> > 
> > This relies on the affinity scheme used by the vendor, which seems
> > to work for the couple of SoCs that are out in the wild.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >   drivers/irqchip/irq-apple-aic.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> > index 3759dc36cc8f..30ca80ccda8b 100644
> > --- a/drivers/irqchip/irq-apple-aic.c
> > +++ b/drivers/irqchip/irq-apple-aic.c
> > @@ -177,6 +177,8 @@ struct aic_irq_chip {
> >   	void __iomem *base;
> >   	struct irq_domain *hw_domain;
> >   	struct irq_domain *ipi_domain;
> > +	struct cpumask ecore_mask;
> > +	struct cpumask pcore_mask;
> >   	int nr_hw;
> >   	int ipi_hwirq;
> >   };
> > @@ -200,6 +202,11 @@ static void aic_ic_write(struct aic_irq_chip *ic, u32 reg, u32 val)
> >   	writel_relaxed(val, ic->base + reg);
> >   }
> >   +static bool __is_pcore(u64 mpidr)
> > +{
> > +	return MPIDR_AFFINITY_LEVEL(mpidr, 2) == 1;
> > +}
> > +
> >   /*
> >    * IRQ irqchip
> >    */
> > @@ -833,6 +840,13 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
> >   		return -ENODEV;
> >   	}
> >   +	for_each_possible_cpu(i) {
> > +		if (__is_pcore(cpu_logical_map(i)))
> > +			cpumask_set_cpu(i, &irqc->pcore_mask);
> > +		else
> > +			cpumask_set_cpu(i, &irqc->ecore_mask);
> > +	}
> > +
> >   	set_handle_irq(aic_handle_irq);
> >   	set_handle_fiq(aic_handle_fiq);
> >   
> 
> I'm okay with this approach, but if we want to be more explicit about
> the affinities, maybe something like apple,pmu-irq-index in the CPU
> nodes? Then we can either start at a higher FIQ offset for these (in
> case we need to add more FIQs in the future), or just make up a new
> AIC_PMU top level interrupt type and start at 0.

I'm actually worried that we'll need more of these "asymmetric FIQs"
in the future, and that the PMU-specific hack won't scale.

Do you know of any other per-CPU device that could differ between
small and big cores? This would certainly help guiding my
implementation between a device specific hack (the PMU irq index) or
something more generic (interrupt specifier containing the affinity
and following the AICv2 scheme).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.