| Message ID | 20210121225712.1118239-3-saravanak@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding | expand |
Hi Saravana, 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@kernel.org> > Cc: Kevin Hilman <khilman@baylibre.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reviewed-by: Rob Herring <robh@kernel.org> > Reviewed-by: Thierry Reding <treding@nvidia.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Saravana Kannan <saravanak@google.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@50000 device defined in arch/arm64/boot/dts/arm/juno-motherboard.dtsi). I didn't check further what's wrong there as without MMC mounting rootfs fails in my test system. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Saravana, > > 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@kernel.org> > > Cc: Kevin Hilman <khilman@baylibre.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Rob Herring <robh@kernel.org> > > Reviewed-by: Thierry Reding <treding@nvidia.com> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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 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@2c010000 { 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? -Saravana
Hi Saravana, On 04.02.2021 22:31, Saravana Kannan wrote: > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > <m.szyprowski@samsung.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@kernel.org> >>> Cc: Kevin Hilman <khilman@baylibre.com> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Reviewed-by: Rob Herring <robh@kernel.org> >>> Reviewed-by: Thierry Reding <treding@nvidia.com> >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > 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@8000000:motherboard-bus not ready 1c050000.mmci amba: probe deferral - supplier bus@8000000:motherboard-bus not ready 1c1d0000.gpio amba: probe deferral - supplier bus@8000000: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@8000000:motherboard-bus not ready 1c070000.kmi amba: probe deferral - supplier bus@8000000:motherboard-bus not ready 1c170000.rtc amba: probe deferral - supplier bus@8000000:motherboard-bus not ready 1c0f0000.wdt amba: probe deferral - supplier bus@8000000: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@8000000:motherboard-bus' on the deferred devices list, so it looks that device core added a link to something that is not a platform device... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Hi Marek, On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > On 04.02.2021 22:31, Saravana Kannan wrote: > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > <m.szyprowski@samsung.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@kernel.org> > >>> Cc: Kevin Hilman <khilman@baylibre.com> > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>> Reviewed-by: Rob Herring <robh@kernel.org> > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > 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@8000000:motherboard-bus not ready > 1c050000.mmci amba: probe deferral - supplier > bus@8000000:motherboard-bus not ready > 1c1d0000.gpio amba: probe deferral - supplier > bus@8000000: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@8000000:motherboard-bus not ready > 1c070000.kmi amba: probe deferral - supplier > bus@8000000:motherboard-bus not ready > 1c170000.rtc amba: probe deferral - supplier > bus@8000000:motherboard-bus not ready > 1c0f0000.wdt amba: probe deferral - supplier > bus@8000000: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@8000000:motherboard-bus' on the deferred devices > list, so it looks that device core added a link to something that is not > a platform device... Lemme guess: bus@8000000 is a simple bus, but it has an interrupt-map, and the devlink code doesn't follow the mapping? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Marek, > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > <m.szyprowski@samsung.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@kernel.org> > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > 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@8000000:motherboard-bus not ready > > 1c050000.mmci amba: probe deferral - supplier > > bus@8000000:motherboard-bus not ready > > 1c1d0000.gpio amba: probe deferral - supplier > > bus@8000000: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@8000000:motherboard-bus not ready > > 1c070000.kmi amba: probe deferral - supplier > > bus@8000000:motherboard-bus not ready > > 1c170000.rtc amba: probe deferral - supplier > > bus@8000000:motherboard-bus not ready > > 1c0f0000.wdt amba: probe deferral - supplier > > bus@8000000: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@8000000: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@8000000 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. -Saravana
Hi Saravana, On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > <m.szyprowski@samsung.com> wrote: > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > <m.szyprowski@samsung.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@kernel.org> > > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > > 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@8000000:motherboard-bus not ready > > > 1c050000.mmci amba: probe deferral - supplier > > > bus@8000000:motherboard-bus not ready > > > 1c1d0000.gpio amba: probe deferral - supplier > > > bus@8000000: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@8000000:motherboard-bus not ready > > > 1c070000.kmi amba: probe deferral - supplier > > > bus@8000000:motherboard-bus not ready > > > 1c170000.rtc amba: probe deferral - supplier > > > bus@8000000:motherboard-bus not ready > > > 1c0f0000.wdt amba: probe deferral - supplier > > > bus@8000000: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@8000000: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@8000000 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 ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > <m.szyprowski@samsung.com> wrote: > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > <m.szyprowski@samsung.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@kernel.org> > > > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > > > 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@8000000:motherboard-bus not ready > > > > 1c050000.mmci amba: probe deferral - supplier > > > > bus@8000000:motherboard-bus not ready > > > > 1c1d0000.gpio amba: probe deferral - supplier > > > > bus@8000000: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@8000000:motherboard-bus not ready > > > > 1c070000.kmi amba: probe deferral - supplier > > > > bus@8000000:motherboard-bus not ready > > > > 1c170000.rtc amba: probe deferral - supplier > > > > bus@8000000:motherboard-bus not ready > > > > 1c0f0000.wdt amba: probe deferral - supplier > > > > bus@8000000: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@8000000: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@8000000 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? -Saravana
Hi Saravana, On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@google.com> wrote: > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > <m.szyprowski@samsung.com> wrote: > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > <m.szyprowski@samsung.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@kernel.org> > > > > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > > > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > > > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > > > > 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@8000000:motherboard-bus not ready > > > > > 1c050000.mmci amba: probe deferral - supplier > > > > > bus@8000000:motherboard-bus not ready > > > > > 1c1d0000.gpio amba: probe deferral - supplier > > > > > bus@8000000: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@8000000:motherboard-bus not ready > > > > > 1c070000.kmi amba: probe deferral - supplier > > > > > bus@8000000:motherboard-bus not ready > > > > > 1c170000.rtc amba: probe deferral - supplier > > > > > bus@8000000:motherboard-bus not ready > > > > > 1c0f0000.wdt amba: probe deferral - supplier > > > > > bus@8000000: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@8000000: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@8000000 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@linux-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
On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Saravana, > > On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@google.com> wrote: > > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > > <m.szyprowski@samsung.com> wrote: > > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > > <m.szyprowski@samsung.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@kernel.org> > > > > > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > > > > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > > > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > > > > > 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@8000000:motherboard-bus not ready > > > > > > 1c050000.mmci amba: probe deferral - supplier > > > > > > bus@8000000:motherboard-bus not ready > > > > > > 1c1d0000.gpio amba: probe deferral - supplier > > > > > > bus@8000000: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@8000000:motherboard-bus not ready > > > > > > 1c070000.kmi amba: probe deferral - supplier > > > > > > bus@8000000:motherboard-bus not ready > > > > > > 1c170000.rtc amba: probe deferral - supplier > > > > > > bus@8000000:motherboard-bus not ready > > > > > > 1c0f0000.wdt amba: probe deferral - supplier > > > > > > bus@8000000: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@8000000: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@8000000 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/). > Yeah, I need to add interrupt-map support. Sigh. Only so many things I can fix at a time. Let me know if you want to help. -Saravana
On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan <saravanak@google.com> wrote: > > On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Saravana, > > > > On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@google.com> wrote: > > > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > > > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > > > <m.szyprowski@samsung.com> wrote: > > > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > > > <m.szyprowski@samsung.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@kernel.org> > > > > > > > >>> Cc: Kevin Hilman <khilman@baylibre.com> > > > > > > > >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > > > > >>> Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > >>> Reviewed-by: Thierry Reding <treding@nvidia.com> > > > > > > > >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > > > > >>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > > > > > > > > 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@8000000:motherboard-bus not ready > > > > > > > 1c050000.mmci amba: probe deferral - supplier > > > > > > > bus@8000000:motherboard-bus not ready > > > > > > > 1c1d0000.gpio amba: probe deferral - supplier > > > > > > > bus@8000000: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@8000000:motherboard-bus not ready > > > > > > > 1c070000.kmi amba: probe deferral - supplier > > > > > > > bus@8000000:motherboard-bus not ready > > > > > > > 1c170000.rtc amba: probe deferral - supplier > > > > > > > bus@8000000:motherboard-bus not ready > > > > > > > 1c0f0000.wdt amba: probe deferral - supplier > > > > > > > bus@8000000: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@8000000: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@8000000 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/). > > > > Yeah, I need to add interrupt-map support. Sigh. Only so many things I > can fix at a time. Let me know if you want to help. > Marek, After reading the DT spec and poking at the code, I THINK this code is correct. Can you give it a shot? If it works, then I can clean it up, roll in interrupts-extended and send a patch. -Saravana --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1300,10 +1300,12 @@ static struct device_node *parse_gpio_compat(struct device_node *np, static struct device_node *parse_interrupts(struct device_node *np, const char *prop_name, int index) { - if (strcmp(prop_name, "interrupts") || index) + struct of_phandle_args sup_args; + if (strcmp(prop_name, "interrupts")) return NULL; - return of_irq_find_parent(np); + return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; }
Hi Saravana, On 06.02.2021 05:32, Saravana Kannan wrote: > On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan <saravanak@google.com> wrote: >> On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>> On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@google.com> wrote: >>>> On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>>>> On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: >>>>>> On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >>>>>>> On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski >>>>>>> <m.szyprowski@samsung.com> wrote: >>>>>>>> On 04.02.2021 22:31, Saravana Kannan wrote: >>>>>>>>> On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski >>>>>>>>> <m.szyprowski@samsung.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@kernel.org> >>>>>>>>>>> Cc: Kevin Hilman <khilman@baylibre.com> >>>>>>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org> >>>>>>>>>>> Reviewed-by: Thierry Reding <treding@nvidia.com> >>>>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>>>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { >>>>>>>>> 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@8000000:motherboard-bus not ready >>>>>>>> 1c050000.mmci amba: probe deferral - supplier >>>>>>>> bus@8000000:motherboard-bus not ready >>>>>>>> 1c1d0000.gpio amba: probe deferral - supplier >>>>>>>> bus@8000000: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@8000000:motherboard-bus not ready >>>>>>>> 1c070000.kmi amba: probe deferral - supplier >>>>>>>> bus@8000000:motherboard-bus not ready >>>>>>>> 1c170000.rtc amba: probe deferral - supplier >>>>>>>> bus@8000000:motherboard-bus not ready >>>>>>>> 1c0f0000.wdt amba: probe deferral - supplier >>>>>>>> bus@8000000: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@8000000: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@8000000 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://protect2.fireeye.com/v1/url?k=72fff987-2d64c09f-72fe72c8-0cc47a314e9a-fd7dac11a78508f3&q=1&e=c0dbf5ca-130b-4aac-a011-447e82ca1914&u=https%3A%2F%2Fwww.devicetree.org%2F). >>> >> Yeah, I need to add interrupt-map support. Sigh. Only so many things I >> can fix at a time. Let me know if you want to help. >> > Marek, > > After reading the DT spec and poking at the code, I THINK this code is > correct. Can you give it a shot? If it works, then I can clean it up, > roll in interrupts-extended and send a patch. Yep, this fixes this issue. Fell free to add: Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > -Saravana > > --- a/drivers/of/property.c > +++ b/drivers/of/property.c > @@ -1300,10 +1300,12 @@ static struct device_node > *parse_gpio_compat(struct device_node *np, > static struct device_node *parse_interrupts(struct device_node *np, > const char *prop_name, int index) > { > - if (strcmp(prop_name, "interrupts") || index) > + struct of_phandle_args sup_args; > + if (strcmp(prop_name, "interrupts")) > return NULL; > > - return of_irq_find_parent(np); > + return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; > } > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Mon, Feb 8, 2021 at 12:14 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Saravana, > > On 06.02.2021 05:32, Saravana Kannan wrote: > > On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan <saravanak@google.com> wrote: > >> On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >>> On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan <saravanak@google.com> wrote: > >>>> On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >>>>> On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan <saravanak@google.com> wrote: > >>>>>> On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >>>>>>> On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > >>>>>>> <m.szyprowski@samsung.com> wrote: > >>>>>>>> On 04.02.2021 22:31, Saravana Kannan wrote: > >>>>>>>>> On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > >>>>>>>>> <m.szyprowski@samsung.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@kernel.org> > >>>>>>>>>>> Cc: Kevin Hilman <khilman@baylibre.com> > >>>>>>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>>>>>>>>>> Reviewed-by: Rob Herring <robh@kernel.org> > >>>>>>>>>>> Reviewed-by: Thierry Reding <treding@nvidia.com> > >>>>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>>>>>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.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@50000 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@2c010000 { > >>>>>>>>> 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@8000000:motherboard-bus not ready > >>>>>>>> 1c050000.mmci amba: probe deferral - supplier > >>>>>>>> bus@8000000:motherboard-bus not ready > >>>>>>>> 1c1d0000.gpio amba: probe deferral - supplier > >>>>>>>> bus@8000000: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@8000000:motherboard-bus not ready > >>>>>>>> 1c070000.kmi amba: probe deferral - supplier > >>>>>>>> bus@8000000:motherboard-bus not ready > >>>>>>>> 1c170000.rtc amba: probe deferral - supplier > >>>>>>>> bus@8000000:motherboard-bus not ready > >>>>>>>> 1c0f0000.wdt amba: probe deferral - supplier > >>>>>>>> bus@8000000: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@8000000: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@8000000 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://protect2.fireeye.com/v1/url?k=72fff987-2d64c09f-72fe72c8-0cc47a314e9a-fd7dac11a78508f3&q=1&e=c0dbf5ca-130b-4aac-a011-447e82ca1914&u=https%3A%2F%2Fwww.devicetree.org%2F). > >>> > >> Yeah, I need to add interrupt-map support. Sigh. Only so many things I > >> can fix at a time. Let me know if you want to help. > >> > > Marek, > > > > After reading the DT spec and poking at the code, I THINK this code is > > correct. Can you give it a shot? If it works, then I can clean it up, > > roll in interrupts-extended and send a patch. > > Yep, this fixes this issue. Fell free to add: > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > Thanks! I'll send out the proper patch. -Saravana
Hi, On Thu, Jan 21, 2021 at 02:57:12PM -0800, 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@kernel.org> > Cc: Kevin Hilman <khilman@baylibre.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reviewed-by: Rob Herring <robh@kernel.org> > Reviewed-by: Thierry Reding <treding@nvidia.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Saravana Kannan <saravanak@google.com> This patch causes all ppc64:powernv qemu emulations to fail. The problem is always the same: The root file system can not be mounted. Example: [ 14.245672][ T1] VFS: Cannot open root device "sda" or unknown-block(0,0): error -6 [ 14.246063][ T1] Please append a correct "root=" boot option; here are the available partitions: [ 14.246609][ T1] 1f00 131072 mtdblock0 [ 14.246648][ T1] (driver?) [ 14.247137][ T1] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) [ 14.247631][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.11.0-rc7-next-20210212 #1 [ 14.248166][ T1] Call Trace: [ 14.248344][ T1] [c000000002c07a70] [c0000000008f052c] dump_stack+0x100/0x174 (unreliable) [ 14.248780][ T1] [c000000002c07ab0] [c00000000010d0e0] panic+0x190/0x450 [ 14.249097][ T1] [c000000002c07b50] [c0000000014d1af8] mount_block_root+0x320/0x430 [ 14.249442][ T1] [c000000002c07c50] [c0000000014d1e64] prepare_namespace+0x1b0/0x204 [ 14.249798][ T1] [c000000002c07cc0] [c0000000014d1544] kernel_init_freeable+0x3dc/0x438 [ 14.250145][ T1] [c000000002c07da0] [c000000000012b7c] kernel_init+0x2c/0x170 [ 14.250466][ T1] [c000000002c07e10] [c00000000000d56c] ret_from_kernel_thread+0x5c/0x70 [ 28.068945385,5] OPAL: Reboot request... Another: [ 14.273398][ T1] md: Autodetecting RAID arrays. [ 14.273665][ T1] md: autorun ... [ 14.273860][ T1] md: ... autorun DONE. [ 14.275078][ T1] Waiting for root device /dev/mmcblk0... [ waits until terminated ] Key difference seems to be that PCI devices are no longer instantiated with this patch applied. Specifically, I see [ 1.153780][ T1] pci 0005:01 : [PE# fd] Setting up window#0 0..7fffffff pg=10000^M [ 1.154475][ T1] pci 0005:01 : [PE# fd] Enabling 64-bit DMA bypass^M [ 1.155749][ T1] pci 0005:01:00.0: Adding to iommu group 0^M [ 1.160543][ T1] pci 0005:00:00.0: enabling device (0105 -> 0107)^M in both cases, but (exmple nvme) I don't see [ 13.520561][ T11] nvme nvme0: pci function 0005:01:00.0^M [ 13.521747][ T45] nvme 0005:01:00.0: enabling device (0100 -> 0102)^M after this patch has been applied. Reverting th patch plus its fix resolves the problem. Bisect log attached. Guenter --- # bad: [07f7e57c63aaa2afb4ea31edef05e08699a63a00] Add linux-next specific files for 20210212 # good: [92bf22614b21a2706f4993b278017e437f7785b3] Linux 5.11-rc7 git bisect start 'HEAD' 'v5.11-rc7' # good: [987d576a592082b8e0e499990236f49ad655f5dc] Merge remote-tracking branch 'crypto/master' git bisect good 987d576a592082b8e0e499990236f49ad655f5dc # good: [e254726a51e0440c05eb4606215772c34b77a53f] Merge remote-tracking branch 'spi/for-next' git bisect good e254726a51e0440c05eb4606215772c34b77a53f # bad: [309b7dce497b1ce820b2afb13a19b0ad39b23a4a] Merge remote-tracking branch 'char-misc/char-misc-next' git bisect bad 309b7dce497b1ce820b2afb13a19b0ad39b23a4a # good: [5c45f9ce2ad5c4abf79baf114e42620da77c84fe] Merge remote-tracking branch 'chrome-platform/for-next' git bisect good 5c45f9ce2ad5c4abf79baf114e42620da77c84fe # bad: [b45d849cd57bf11d1824f68d92404ceea1293886] Merge remote-tracking branch 'usb/usb-next' git bisect bad b45d849cd57bf11d1824f68d92404ceea1293886 # good: [89451aabea5f91a6c1b6dc4c52cac4caffecbc8a] Merge tag 'tag-ib-usb-typec-chrome-platform-cros-ec-typec-clear-pd-discovery-events-for-5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux into usb-next git bisect good 89451aabea5f91a6c1b6dc4c52cac4caffecbc8a # good: [43861d29c0810a70792bf69d37482efb7bb6677d] USB: quirks: sort quirk entries git bisect good 43861d29c0810a70792bf69d37482efb7bb6677d # good: [08f4a6b903369ee0147b557931b7075c17e015f6] dt-bindings: usb: dwc3: add description for rk3328 git bisect good 08f4a6b903369ee0147b557931b7075c17e015f6 # bad: [1753c4d1edbcf1b35e585ff76777d09434344c8f] of: property: Don't add links to absent suppliers git bisect bad 1753c4d1edbcf1b35e585ff76777d09434344c8f # good: [072a51be8ecfb84e15b27b7f80a601560f386788] Merge 5.11-rc5 into driver-core-next git bisect good 072a51be8ecfb84e15b27b7f80a601560f386788 # bad: [4731210c09f5977300f439b6c56ba220c65b2348] gpiolib: Bind gpio_device to a driver to enable fw_devlink=on by default git bisect bad 4731210c09f5977300f439b6c56ba220c65b2348 # bad: [4044b2fcfb2048a256529ecbd869b43713982006] drivers: base: change 'driver_create_groups' to 'driver_add_groups' in printk git bisect bad 4044b2fcfb2048a256529ecbd869b43713982006 # bad: [4104ca776ba38d81bd6610256d3b0d7e6a058067] of: property: Add fw_devlink support for interrupts git bisect bad 4104ca776ba38d81bd6610256d3b0d7e6a058067 # good: [e13f5b7a130f7b6d4d34be27a87393890b5ee2ba] of: property: Add fw_devlink support for "gpio" and "gpios" binding git bisect good e13f5b7a130f7b6d4d34be27a87393890b5ee2ba # first bad commit: [4104ca776ba38d81bd6610256d3b0d7e6a058067] of: property: Add fw_devlink support for interrupts
On Sat, Feb 13, 2021 at 10:54 AM Guenter Roeck <linux@roeck-us.net> wrote: > > Hi, > > On Thu, Jan 21, 2021 at 02:57:12PM -0800, 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@kernel.org> > > Cc: Kevin Hilman <khilman@baylibre.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Rob Herring <robh@kernel.org> > > Reviewed-by: Thierry Reding <treding@nvidia.com> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Saravana Kannan <saravanak@google.com> > > This patch causes all ppc64:powernv qemu emulations to fail. > The problem is always the same: The root file system can not be mounted. > > Example: > > [ 14.245672][ T1] VFS: Cannot open root device "sda" or unknown-block(0,0): error -6 > [ 14.246063][ T1] Please append a correct "root=" boot option; here are the available partitions: > [ 14.246609][ T1] 1f00 131072 mtdblock0 > [ 14.246648][ T1] (driver?) > [ 14.247137][ T1] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) > [ 14.247631][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.11.0-rc7-next-20210212 #1 > [ 14.248166][ T1] Call Trace: > [ 14.248344][ T1] [c000000002c07a70] [c0000000008f052c] dump_stack+0x100/0x174 (unreliable) > [ 14.248780][ T1] [c000000002c07ab0] [c00000000010d0e0] panic+0x190/0x450 > [ 14.249097][ T1] [c000000002c07b50] [c0000000014d1af8] mount_block_root+0x320/0x430 > [ 14.249442][ T1] [c000000002c07c50] [c0000000014d1e64] prepare_namespace+0x1b0/0x204 > [ 14.249798][ T1] [c000000002c07cc0] [c0000000014d1544] kernel_init_freeable+0x3dc/0x438 > [ 14.250145][ T1] [c000000002c07da0] [c000000000012b7c] kernel_init+0x2c/0x170 > [ 14.250466][ T1] [c000000002c07e10] [c00000000000d56c] ret_from_kernel_thread+0x5c/0x70 > [ 28.068945385,5] OPAL: Reboot request... > > Another: > > [ 14.273398][ T1] md: Autodetecting RAID arrays. > [ 14.273665][ T1] md: autorun ... > [ 14.273860][ T1] md: ... autorun DONE. > [ 14.275078][ T1] Waiting for root device /dev/mmcblk0... > > [ waits until terminated ] > > Key difference seems to be that PCI devices are no longer instantiated > with this patch applied. Specifically, I see > > [ 1.153780][ T1] pci 0005:01 : [PE# fd] Setting up window#0 0..7fffffff pg=10000^M > [ 1.154475][ T1] pci 0005:01 : [PE# fd] Enabling 64-bit DMA bypass^M > [ 1.155749][ T1] pci 0005:01:00.0: Adding to iommu group 0^M > [ 1.160543][ T1] pci 0005:00:00.0: enabling device (0105 -> 0107)^M > > in both cases, but (exmple nvme) I don't see > > [ 13.520561][ T11] nvme nvme0: pci function 0005:01:00.0^M > [ 13.521747][ T45] nvme 0005:01:00.0: enabling device (0100 -> 0102)^M > > after this patch has been applied. > > Reverting th patch plus its fix resolves the problem. > > Bisect log attached. Hi Guenter, Thanks for the report. Can you please give me the following details: * The DTS file for the board (not the SoC). * A boot log with the logs enabled in device_links_check_suppliers() and device_link_add() That should help me debug this. Rob, Looks like Guenter has this patch[1] too. What PPC specific IRQ hack am I missing? Any ideas? [1] - https://lore.kernel.org/lkml/20210209010439.3529036-1-saravanak@google.com/ Thanks, Saravana
On 2/14/21 1:12 PM, Saravana Kannan wrote: [ ... ] > Can you please give me the following details: > * The DTS file for the board (not the SoC). I don't have it, sorry. The devicetree file is generated by qemu. Is there a way to extract it from a running system ? > * A boot log with the logs enabled in device_links_check_suppliers() > and device_link_add() > There is not much to be seen, except this: [ 14.084606][ T11] pci 0005:01:00.0: probe deferral - wait for supplier interrupt-controller@0 repeated three times. Guenter
On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 2/14/21 1:12 PM, Saravana Kannan wrote: > [ ... ] > > > > Can you please give me the following details: > > * The DTS file for the board (not the SoC). > > The devicetree file extracted from the running system is attached. > Hope it helps. Hi Guenter, Thanks for the DTS file and logs. That helps a lot. Looking at the attachment and this line from the earlier email: [ 14.084606][ T11] pci 0005:01:00.0: probe deferral - wait for supplier interrupt-controller@0 It's clear the PCI node is waiting on: interrupt-controller@0 { #address-cells = <0x00>; device_type = "PowerPC-Interrupt-Source-Controller"; compatible = "ibm,opal-xive-vc\0IBM,opal-xics"; #interrupt-cells = <0x02>; reg = <0x00 0x00 0x00 0x00>; phandle = <0x804b>; interrupt-controller; }; If I grep for "ibm,opal-xive-vc", I see only one instance of it in the code. And that eventually ends up getting called like this: irq_find_matching_fwspec() -> xive_irq_domain_match() -> xive_native_match() static bool xive_native_match(struct device_node *node) { return of_device_is_compatible(node, "ibm,opal-xive-vc"); } However, when the IRQ domain are first registered, in xive_init_host() the "np" passed in is NOT the same node that xive_native_match() would match. static void __init xive_init_host(struct device_node *np) { xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, &xive_irq_domain_ops, NULL); if (WARN_ON(xive_irq_domain == NULL)) return; irq_set_default_host(xive_irq_domain); } Instead, the "np" here is: interrupt-controller@6030203180000 { ibm,xive-provision-page-size = <0x10000>; ibm,xive-eq-sizes = <0x0c 0x10 0x15 0x18>; single-escalation-support; ibm,xive-provision-chips = <0x00>; ibm,xive-#priorities = <0x08>; compatible = "ibm,opal-xive-pe\0ibm,opal-intc"; reg = <0x60302 0x3180000 0x00 0x10000 0x60302 0x3190000 0x00 0x10000 0x60302 0x31a0000 0x00 0x10000 0x60302 0x31b0000 0x00 0x10000>; phandle = <0x8051>; }; There are many ways to fix this, but I first want to make sure this is a valid way to register irqdomains before trying to fix it. I just find it weird that the node that's registered is unrelated (not a parent/child) of the node that matches. Marc, Is this a valid way to register irqdomains? Just registering interrupt-controller@6030203180000 DT node where there are multiple interrupt controllers? Thanks, Saravana
Hi Saravana, On Mon, 15 Feb 2021 08:29:53 +0000, Saravana Kannan <saravanak@google.com> wrote: > > On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 2/14/21 1:12 PM, Saravana Kannan wrote: > > [ ... ] > > > > > > Can you please give me the following details: > > > * The DTS file for the board (not the SoC). > > > > The devicetree file extracted from the running system is attached. > > Hope it helps. > > Hi Guenter, > > Thanks for the DTS file and logs. That helps a lot. > > Looking at the attachment and this line from the earlier email: > [ 14.084606][ T11] pci 0005:01:00.0: probe deferral - wait for > supplier interrupt-controller@0 > > It's clear the PCI node is waiting on: > interrupt-controller@0 { > #address-cells = <0x00>; > device_type = "PowerPC-Interrupt-Source-Controller"; > compatible = "ibm,opal-xive-vc\0IBM,opal-xics"; > #interrupt-cells = <0x02>; > reg = <0x00 0x00 0x00 0x00>; > phandle = <0x804b>; > interrupt-controller; > }; > > If I grep for "ibm,opal-xive-vc", I see only one instance of it in the > code. And that eventually ends up getting called like this: > irq_find_matching_fwspec() -> xive_irq_domain_match() -> xive_native_match() > > static bool xive_native_match(struct device_node *node) > { > return of_device_is_compatible(node, "ibm,opal-xive-vc"); > } > > However, when the IRQ domain are first registered, in xive_init_host() > the "np" passed in is NOT the same node that xive_native_match() would > match. > static void __init xive_init_host(struct device_node *np) > { > xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, > &xive_irq_domain_ops, NULL); > if (WARN_ON(xive_irq_domain == NULL)) > return; > irq_set_default_host(xive_irq_domain); > } > > Instead, the "np" here is: > interrupt-controller@6030203180000 { > ibm,xive-provision-page-size = <0x10000>; > ibm,xive-eq-sizes = <0x0c 0x10 0x15 0x18>; > single-escalation-support; > ibm,xive-provision-chips = <0x00>; > ibm,xive-#priorities = <0x08>; > compatible = "ibm,opal-xive-pe\0ibm,opal-intc"; > reg = <0x60302 0x3180000 0x00 0x10000 0x60302 > 0x3190000 0x00 0x10000 0x60302 0x31a0000 0x00 0x10000 0x60302 > 0x31b0000 0x00 0x10000>; > phandle = <0x8051>; > }; > > There are many ways to fix this, but I first want to make sure this is > a valid way to register irqdomains before trying to fix it. I just > find it weird that the node that's registered is unrelated (not a > parent/child) of the node that matches. > > Marc, > > Is this a valid way to register irqdomains? Just registering > interrupt-controller@6030203180000 DT node where there are multiple > interrupt controllers? Absolutely. The node is only one of the many possible ways to retrieve a domain. In general, what you pass as the of_node/fwnode_handle can be anything you want. It doesn't have to represent anything in the system (we even create then ex-nihilo in some cases), and the match/select callbacks are authoritative when they exist. There is also the use of a default domain, which is used as a fallback when no domain is found via the normal matching procedure. PPC has established a way of dealing with domains long before ARM did, closer to the board files of old than what we would do today (code driven rather than data structure driven). Strictly mapping domains onto HW blocks is a desirable property, but that is all it is. That doesn't affect the very purpose of the IRQ domains, which is to translate numbers from one context into another. I'd be all for rationalising this, but it is pretty hard to introduce semantic where there is none. Thanks, M. -- Without deviation from the norm, progress is not possible.
On Mon, Feb 15, 2021 at 1:09 AM Marc Zyngier <maz@kernel.org> wrote: > > Hi Saravana, > > On Mon, 15 Feb 2021 08:29:53 +0000, > Saravana Kannan <saravanak@google.com> wrote: > > > > On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > On 2/14/21 1:12 PM, Saravana Kannan wrote: > > > [ ... ] > > > > > > > > Can you please give me the following details: > > > > * The DTS file for the board (not the SoC). > > > > > > The devicetree file extracted from the running system is attached. > > > Hope it helps. > > > > Hi Guenter, > > > > Thanks for the DTS file and logs. That helps a lot. > > > > Looking at the attachment and this line from the earlier email: > > [ 14.084606][ T11] pci 0005:01:00.0: probe deferral - wait for > > supplier interrupt-controller@0 > > > > It's clear the PCI node is waiting on: > > interrupt-controller@0 { > > #address-cells = <0x00>; > > device_type = "PowerPC-Interrupt-Source-Controller"; > > compatible = "ibm,opal-xive-vc\0IBM,opal-xics"; > > #interrupt-cells = <0x02>; > > reg = <0x00 0x00 0x00 0x00>; > > phandle = <0x804b>; > > interrupt-controller; > > }; > > > > If I grep for "ibm,opal-xive-vc", I see only one instance of it in the > > code. And that eventually ends up getting called like this: > > irq_find_matching_fwspec() -> xive_irq_domain_match() -> xive_native_match() > > > > static bool xive_native_match(struct device_node *node) > > { > > return of_device_is_compatible(node, "ibm,opal-xive-vc"); > > } > > > > However, when the IRQ domain are first registered, in xive_init_host() > > the "np" passed in is NOT the same node that xive_native_match() would > > match. > > static void __init xive_init_host(struct device_node *np) > > { > > xive_irq_domain = irq_domain_add_nomap(np, XIVE_MAX_IRQ, > > &xive_irq_domain_ops, NULL); > > if (WARN_ON(xive_irq_domain == NULL)) > > return; > > irq_set_default_host(xive_irq_domain); > > } > > > > Instead, the "np" here is: > > interrupt-controller@6030203180000 { > > ibm,xive-provision-page-size = <0x10000>; > > ibm,xive-eq-sizes = <0x0c 0x10 0x15 0x18>; > > single-escalation-support; > > ibm,xive-provision-chips = <0x00>; > > ibm,xive-#priorities = <0x08>; > > compatible = "ibm,opal-xive-pe\0ibm,opal-intc"; > > reg = <0x60302 0x3180000 0x00 0x10000 0x60302 > > 0x3190000 0x00 0x10000 0x60302 0x31a0000 0x00 0x10000 0x60302 > > 0x31b0000 0x00 0x10000>; > > phandle = <0x8051>; > > }; > > > > There are many ways to fix this, but I first want to make sure this is > > a valid way to register irqdomains before trying to fix it. I just > > find it weird that the node that's registered is unrelated (not a > > parent/child) of the node that matches. > > > > Marc, > > > > Is this a valid way to register irqdomains? Just registering > > interrupt-controller@6030203180000 DT node where there are multiple > > interrupt controllers? > > Absolutely. > > The node is only one of the many possible ways to retrieve a > domain. In general, what you pass as the of_node/fwnode_handle can be > anything you want. It doesn't have to represent anything in the system > (we even create then ex-nihilo in some cases), and the match/select > callbacks are authoritative when they exist. > > There is also the use of a default domain, which is used as a fallback > when no domain is found via the normal matching procedure. > > PPC has established a way of dealing with domains long before ARM did, > closer to the board files of old than what we would do today (code > driven rather than data structure driven). > > Strictly mapping domains onto HW blocks is a desirable property, but > that is all it is. That doesn't affect the very purpose of the IRQ > domains, which is to translate numbers from one context into another. > > I'd be all for rationalising this, but it is pretty hard to introduce > semantic where there is none. Ok, I'm going to disable parsing "interrupts" for PPC. It doesn't look like any of the irq drivers are even remotely ready to be converted to a proper device driver anyway. And if this continues for other properties, I'll just disable fw_devlink for PPC entirely. -Saravana
diff --git a/drivers/of/property.c b/drivers/of/property.c index b2ea1951d937..6287c6d60bb7 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -24,6 +24,7 @@ #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_graph.h> +#include <linux/of_irq.h> #include <linux/string.h> #include <linux/moduleparam.h> @@ -1293,6 +1294,15 @@ static struct device_node *parse_gpio_compat(struct device_node *np, return sup_args.np; } +static struct device_node *parse_interrupts(struct device_node *np, + const char *prop_name, int index) +{ + if (strcmp(prop_name, "interrupts") || index) + return NULL; + + return of_irq_find_parent(np); +} + static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_clocks, }, { .parse_prop = parse_interconnects, }, @@ -1319,6 +1329,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_pinctrl7, }, { .parse_prop = parse_pinctrl8, }, { .parse_prop = parse_gpio_compat, }, + { .parse_prop = parse_interrupts, }, { .parse_prop = parse_regulators, }, { .parse_prop = parse_gpio, }, { .parse_prop = parse_gpios, },