Re: [PATCH] irqchip/gic-v3: Add Allwinner sunxi001 erratum workaround

[Marc Zyngier]

On Sun, 02 Jun 2024 14:23:43 +0100,
yu harry <harry.yu185@xxxxxxxxx> wrote:
> 
> On Sun, Jun 2, 2024 at 5:25 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
> >
> > On Sun, 02 Jun 2024 08:10:58 +0100,
> > "harry.yu185" <harry.yu185@xxxxxxxxx> wrote:
> > >
> > > Allwinner A523 GIC600 integration does not support the
> > > sharability feature. So assigned Erratum ID #sunxi001 for this
> > > issue.
> > >
> > > That the 0x0201643b ID is not Allwinner specific and thus
> > > there is an extra of_machine_is_compatible() check.
> > >
> > > Note, because more than one soc may have this problem, the 'sunxi'
> > > name is used instead of a fixed soc name like A523.
> > >
> > > Signed-off-by: harry.yu185 <harry.yu185@xxxxxxxxx>
> >
> > No, this is all already handled by the driver already (since 6.6).
> >
> > Please fix your DT to include the "dma-noncoherent" property in the
> > GIC and ITS nodes, which should paper over the integration bug.
> >
> > Thanks,
> >
> >         M.
> >
> > --
> > Without deviation from the norm, progress is not possible.
> 
> Thank you for your reply,
> the method you said may not be suitable, because this SOC
> also needs RDIST_FLAGS_FORCE_NON_SHAREABLE,
> just like RK3588, but it is different from the RK3588 version.

Who is talking of RK3588? Have you read what I wrote? Have you
actually looked at what these attributes do?

For context, here's what you're proposing:

+static bool __maybe_unused its_enable_sunxi001(void *data)
+{
+	struct its_node *its = data;
+
+	if (!of_machine_is_compatible("arm,sun55iw3p1"))
+		return false;
+
+	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
+	gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
+
+	return true;
+}
+

"dma-noncoherent" on the GIC node provides:

static bool rd_set_non_coherent(void *data)
{
	struct gic_chip_data *d = data;

	d->rdists.flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
	return true;
}

"dma-noncoherent" on the ITS node provides:

static bool its_set_non_coherent(void *data)
{
	struct its_node *its = data;

	its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
	return true;
}

So please do explain how the combination of the two isn't equivalent
to your patch. How does it fail to provide the required workaround?

	M.

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