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;
MIME-Version: 1.0
References: <20210111142309.193441-1-maxime@cerno.tech> <20210111142309.193441-15-maxime@cerno.tech>
In-Reply-To: <20210111142309.193441-15-maxime@cerno.tech>
From:   Dave Stevenson <dave.stevenson@raspberrypi.com>
Date:   Tue, 9 Feb 2021 09:49:05 +0000
Message-ID: <CAPY8ntC7U1BAVT8xe0emX19p4mCu-BQOeBeC-CDgKjE+asZKRQ@mail.gmail.com>
Subject: Re: [PATCH v2 14/15] ARM: dts: bcm2711: Add the BSC interrupt controller
To:     Maxime Ripard <maxime@cerno.tech>
Cc:     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>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Maxime

On Mon, 11 Jan 2021 at 14:23, Maxime Ripard <maxime@cerno.tech> wrote:
>
> The BSC controllers used for the HDMI DDC have an interrupt controller
> shared between both instances. Let's add it to avoid polling.

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 interrupts.

Whilst i2c-bcm2835 requests the interrupt with IRQF_SHARED, that
doesn't appear to be an option for l2-intc registering as an interrupt
controller. i2c-bcm2835 therefore loses out and fails to register for
the interrupt.

Is there an equivalent flag that an interrupt controller can add to
say that the parent interrupt is shared? Is that even supported?

Thanks
  Dave

> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  arch/arm/boot/dts/bcm2711.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 4847dd305317..8bb46ae76a92 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -308,6 +308,14 @@ dvp: clock@7ef00000 {
>                         #reset-cells = <1>;
>                 };
>
> +               bsc_intr: interrupt-controller@7ef00040 {
> +                       compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
> +                       reg = <0x7ef00040 0x30>;
> +                       interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <1>;
> +               };
> +
>                 hdmi0: hdmi@7ef00700 {
>                         compatible = "brcm,bcm2711-hdmi0";
>                         reg = <0x7ef00700 0x300>,
> @@ -341,6 +349,8 @@ ddc0: i2c@7ef04500 {
>                         reg = <0x7ef04500 0x100>, <0x7ef00b00 0x300>;
>                         reg-names = "bsc", "auto-i2c";
>                         clock-frequency = <97500>;
> +                       interrupt-parent = <&bsc_intr>;
> +                       interrupts = <0>;
>                         status = "disabled";
>                 };
>
> @@ -377,6 +387,8 @@ ddc1: i2c@7ef09500 {
>                         reg = <0x7ef09500 0x100>, <0x7ef05b00 0x300>;
>                         reg-names = "bsc", "auto-i2c";
>                         clock-frequency = <97500>;
> +                       interrupt-parent = <&bsc_intr>;
> +                       interrupts = <1>;
>                         status = "disabled";
>                 };
>         };
> --
> 2.29.2
>