Message-ID: <CAMuHMdVyTOp9PU0rO+YkpzE68VtGdy-bMOwmE_PJx2fiwwpMzQ@mail.gmail.com>
Date:   Fri, 5 Feb 2021 18:51:54 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Saravana Kannan <saravanak@...gle.com>
Cc:     Marek Szyprowski <m.szyprowski@...sung.com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Jon Hunter <jonathanh@...dia.com>,
        Marc Zyngier <maz@...nel.org>,
        Kevin Hilman <khilman@...libre.com>,
        Android Kernel Team <kernel-team@...roid.com>,
        Rob Herring <robh@...nel.org>,
        Thierry Reding <treding@...dia.com>
Subject: Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts

Hi Saravana,

On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@...gle.com> wrote:
> On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@...gle.com> wrote:
> > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski
> > > > <m.szyprowski@...sung.com> wrote:
> > > > > On 04.02.2021 22:31, Saravana Kannan wrote:
> > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski
> > > > > > <m.szyprowski@...sung.com> wrote:
> > > > > >> On 21.01.2021 23:57, Saravana Kannan wrote:
> > > > > >>> This allows fw_devlink to create device links between consumers of an
> > > > > >>> interrupt and the supplier of the interrupt.
> > > > > >>>
> > > > > >>> Cc: Marc Zyngier <maz@...nel.org>
> > > > > >>> Cc: Kevin Hilman <khilman@...libre.com>
> > > > > >>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > > > >>> Reviewed-by: Rob Herring <robh@...nel.org>
> > > > > >>> Reviewed-by: Thierry Reding <treding@...dia.com>
> > > > > >>> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> > > > > >>> Signed-off-by: Saravana Kannan <saravanak@...gle.com>
> > > > > >> This patch landed some time ago in linux-next as commit 4104ca776ba3
> > > > > >> ("of: property: Add fw_devlink support for interrupts"). It breaks MMC
> > > > > >> host controller operation on ARM Juno R1 board (the mmci@...00 device
> > > > > >> defined in arch/arm64/boot/dts/arm/juno-motherboard.dtsi). I didn't
> > > > > > I grepped around and it looks like the final board file is this or
> > > > > > whatever includes it?
> > > > > > arch/arm64/boot/dts/arm/juno-base.dtsi
> > > > > The final board file is arch/arm64/boot/dts/arm/juno-r1.dts
> > > > > > This patch just finds the interrupt-parent and then tries to use that
> > > > > > as a supplier if "interrupts" property is listed. But the only
> > > > > > interrupt parent I can see is:
> > > > > >          gic: interrupt-controller@...10000 {
> > > > > >                  compatible = "arm,gic-400", "arm,cortex-a15-gic";
> > > > > >
> > > > > > And the driver uses IRQCHIP_DECLARE() and hence should be pretty much
> > > > > > a NOP since those suppliers are never devices and are ignored.
> > > > > > $ git grep "arm,gic-400" -- drivers/
> > > > > > drivers/irqchip/irq-gic.c:IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
> > > > > >
> > > > > > This doesn't make any sense. Am I looking at the right files? Am I
> > > > > > missing something?
> > > > >
> > > > > Okay, I've added displaying a list of deferred devices when mounting
> > > > > rootfs fails and got following items:
> > > > >
> > > > > Deferred devices:
> > > > > 18000000.ethernet        platform: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 1c050000.mmci    amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 1c1d0000.gpio    amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 2b600000.iommu   platform: probe deferral - wait for supplier
> > > > > scpi-power-domains
> > > > > 7ff50000.hdlcd   platform: probe deferral - wait for supplier scpi-clk
> > > > > 7ff60000.hdlcd   platform: probe deferral - wait for supplier scpi-clk
> > > > > 1c060000.kmi     amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 1c070000.kmi     amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 1c170000.rtc     amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > 1c0f0000.wdt     amba: probe deferral - supplier
> > > > > bus@...0000:motherboard-bus not ready
> > > > > gpio-keys
> > > > > Kernel panic - not syncing: VFS: Unable to mount root fs on
> > > > > unknown-block(0,0)
> > > > >
> > > > > I don't see the 'bus@...0000:motherboard-bus' on the deferred devices
> > > > > list, so it looks that device core added a link to something that is not
> > > > > a platform device...
> > >
> > > Probe deferred devices (even platform devices) not showing up in that
> > > list is not unusual. That's because devices end up on that list only
> > > after a driver for them is matched and then it fails.
> > >
> > > > Lemme guess: bus@...0000 is a simple bus, but it has an
> > > > interrupt-map, and the devlink code doesn't follow the mapping?
> > > >
> > >
> > > No, what's happening is that (and this is something I just learned)
> > > that if a parent has an "#interrupt-cells" property, it becomes your
> > > interrupt parent. In this case, the motherboard-bus (still a platform
> > > device) is the parent, but it never probes (because it's simple-bus
> > > and "arm,vexpress,v2p-p1"). But it becomes the interrupt parent. And
> > > this mmci device is marked as a consumer of this bus (while still a
> > > grand-child). Yeah, I'm working on patches (multiple rewrites) to take
> > > care of cases like this.
> >
> > One more reason to scrap the different handling of "simple-bus" and
> > "simple-pm-bus", and use drivers/bus/simple-pm-bus.c, which is a
> > platform device driver, for both? (like I originally intended ;-)
>
> I'm not sure if this will cause more issues since people are used to
> simple-bus not needing a driver. I'm afraid to open that pandora's
> box. Maybe last resort if I don't have any other options.
>
> But keeping that aside, I'm confused how interrupts are even working
> if the parent is a DT node with no driver (let alone a device). Any
> ideas on what's going on or what I'm misunderstanding?

No driver is needed, as the interrupts are just translated by the map,
and passed to another interrupt controller, which does have a driver.

Cfr. Section 2.4.3 "Interrupt Nexus Properties" in the DeviceTree
Specification (https://www.devicetree.org/).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists