Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 12 Feb 2021 12:34:01 +0100
From:   Maxime Ripard <maxime@cerno.tech>
To:     Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Cc:     Florian Fainelli <f.fainelli@gmail.com>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Marc Zyngier <maz@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Eric Anholt <eric@anholt.net>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Daniel Vetter <daniel.vetter@intel.com>,
        David Airlie <airlied@linux.ie>,
        linux-rpi-kernel@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        LKML <linux-kernel@vger.kernel.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        bcm-kernel-feedback-list@broadcom.com,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt
 controller
Message-ID: <20210212113401.rqm3373c7i3gvfaa@gilmour>
References: <20210111142309.193441-1-maxime@cerno.tech>
 <20210111142309.193441-15-maxime@cerno.tech>
 <CAPY8ntC7U1BAVT8xe0emX19p4mCu-BQOeBeC-CDgKjE+asZKRQ@mail.gmail.com>
 <20210210144043.s4plyc7ekwnnu7k4@gilmour>
 <9d868bf76072fee7838b6f2ff73a575c@kernel.org>
 <CAPY8ntCG3vLy1NWNF09DEZWE-t_xc+q5m89Jv8GB8GofBLVp3Q@mail.gmail.com>
 <d4f8c89b-9bab-3dce-1f05-aeef39f64211@gmail.com>
 <97eef5ad872b08b174dfd4ed903508371b1baaa5.camel@suse.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="moz7vodkmjhn6f2x"
Content-Disposition: inline
In-Reply-To: <97eef5ad872b08b174dfd4ed903508371b1baaa5.camel@suse.de>
Precedence: bulk


--moz7vodkmjhn6f2x
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 12, 2021 at 12:20:14PM +0100, Nicolas Saenz Julienne wrote:
> On Wed, 2021-02-10 at 10:49 -0800, Florian Fainelli wrote:
> > On 2/10/21 7:49 AM, Dave Stevenson wrote:
> > > Hi Marc.
> > >=20
> > > On Wed, 10 Feb 2021 at 15:30, Marc Zyngier <maz@kernel.org> wrote:
> > > >=20
> > > > Hi Maxime,
> > > >=20
> > > > On 2021-02-10 14:40, Maxime Ripard wrote:
> > > > > Hi Dave,
> > > > >=20
> > > > > On Tue, Feb 09, 2021 at 09:49:05AM +0000, Dave Stevenson wrote:
> > > > > > On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <maxime@cerno.tech>=
 wrote:
> > > > > > >=20
> > > > > > > The BSC controllers used for the HDMI DDC have an interrupt c=
ontroller
> > > > > > > shared between both instances. Let's add it to avoid polling.
> > > > > >=20
> > > > > > This seems to have unintended side effects.
> > > > > > GIC interrupt 117 is shared between the standard I2C controllers
> > > > > > (i2c-bcm2835) and the l2-intc block handling the HDMI I2C inter=
rupts.
> > > > > >=20
> > > > > > Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
> > > > > > doesn't appear to be an option for l2-intc registering as an in=
terrupt
> > > > > > controller. i2c-bcm2835 therefore loses out and fails to regist=
er for
> > > > > > the interrupt.
> > > > > >=20
> > > > > > Is there an equivalent flag that an interrupt controller can ad=
d to
> > > > > > say that the parent interrupt is shared? Is that even supported?
> > > > >=20
> > > > > Indeed, it looks like setting an equivalent to IRQF_SHARED would =
be the
> > > > > solution, but I couldn't find anything that would allow us to in =
the
> > > > > irqchip code.
> > > > >=20
> > > > > Marc, Thomas, is it something that is allowed?
> > > >=20
> > > > No, not really. That's because the chained handler is actually an
> > > > interrupt flow, and not a normal handler. IRQF_SHARED acts at the w=
rong
> > > > level for that.
> > > >=20
> > > > I can see two possibilities:
> > > >=20
> > > > - the l2-intc gets turned into a normal handler, and does the demux
> > > > =A0=A0=A0from there. Horrible stuff.
> > > >=20
> > > > - the i2c controller gets parented to the l2c-int as a fake interru=
pt,
> > > > =A0=A0=A0and gets called from there. Horrible stuff.
> > > >=20
> > > > Pick your poison... :-/
> > >=20
> > > Thanks for the info.
> > >=20
> > > Option 3 - remove l2-intc and drop back to polling the i2c-brcmstb
> > > blocks (which the driver supports anyway).
> > > HDMI I2C generally isn't heavily used once displays are connected, so
> > > I'd be OK with that.
> > >=20
> > > (We can keep the l2-intc that handles CEC and HPD as that is on a
> > > unique GIC interrupt).
> >=20
> > Agreed, Maxime or Nicolas do you want me to send a revert of this patch?
>=20
> Reverting seems the safe move, but I'll defer to whatever Maxime says.

Yes, reverting it seems like the easiest way forward. If you can send it
Florian that would be great :)

Maxime

--moz7vodkmjhn6f2x
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYCZnnwAKCRDj7w1vZxhR
xdTQAQC85VJJ8wGUhPixlwCo17bQ6yPOc9ss6XeEenGXEOsgkwEA73taLwLEdjTp
Vm9W4RMnUydzJnx0LRGV68eTY4SMNwE=
=X4Yr
-----END PGP SIGNATURE-----

--moz7vodkmjhn6f2x--