Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.

From: Rob Herring <robh@kernel.org>
To: Mohamed Mediouni <mohamed.mediouni@caramail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Arnd Bergmann <arnd@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Hector Martin <marcan@marcan.st>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Stan Skowronek <stan@corellium.com>
Subject: Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.
Date: Thu, 21 Jan 2021 11:37:46 -0600	[thread overview]
Message-ID: <CAL_Jsq+V74tMGJ==OVdsssfqTTa3TrFujfRjj0174uZNonpQSA@mail.gmail.com> (raw)
In-Reply-To: <FEDB12D6-2916-4430-A8DC-FBD9352F3730@caramail.com>

On Thu, Jan 21, 2021 at 10:43 AM Mohamed Mediouni
<mohamed.mediouni@caramail.com> wrote:
> > On 21 Jan 2021, at 17:40, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Jan 21, 2021 at 6:52 AM Mohamed Mediouni
> > <mohamed.mediouni@caramail.com> wrote:
> >>> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@kernel.org> wrote:
> >>> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> >>> <mohamed.mediouni@caramail.com> wrote:

[...]

> >>>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
> >>>>      if (WARN(!aic.base, "unable to map aic registers\n"))
> >>>>              return -EINVAL;
> >>>>
> >>>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> >>>
> >>> Where is this property documented, and what decides which one to use?
> >> It’s getting documented in the next patch set.
> >>
> >> This property is there to enable support for older iPhone processors
> >> later on, some of which do not have fast IPI support.
> >>
> >> On Apple M1, fast-ipi is always on.
> >
> > This should be implied by the compatible string which needs to be more
> > specific and include the SoC name.
> >
> > Rob
>
> Then we’ll eventually have two aic compatible strings, aic which is compatible
> with Apple A7 onwards and aicv2 which is a superset with fast IPI (introduced
> on the Apple A11, 3 years ago, with no further programmer-visible changes since
> then).
>
> Does that look right?

If we did this from the start, it would evolve like this:

A7: "AAPL,a7-aic"
A8: "AAPL,a8-aic", "AAPL,a7-aic"  # Read this as A8 AIC is backwards
compatible with A7 AIC
A9: "AAPL,a9-aic", "AAPL,a7-aic"

A11: "AAPL,a11-aic", "AAPL,a7-aic"

If the A11 version could work on an OS that only supported the
original model (sounds like this is the case) Or if it's not backwards
compatible:

A11: "AAPL,a11-aic"

If the A11 is different and not backwards compatible.

Then M1 could be:

M1: "AAPL,m1-aic", "AAPL,a11-aic"

Or to even support an OS with only v1 support:

M1: "AAPL,m1-aic", "AAPL,a11-aic", "AAPL,a7-aic"

You don't really need the fallback here because there isn't any
existing OS support and the baseline is the M1.

If you want to have generic fallback compatible strings with versions,
that's fine too. I'm not really a fan of version numbers that are just
made up by the binding author though. Most SoC vendors don't have
rigorous versioning of their IP and those that do seem to have a new
version on every SoC.

The important part is *always* having an SoC specific compatible so
you can deal with any quirk or feature without having to change the
DTB. Everyone says blocks are 'the same' until they aren't.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel