Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Mon, Feb 15, 2021 at 1:09 AM Marc Zyngier wrote: > > Hi Saravana, > > On Mon, 15 Feb 2021 08:29:53 +, > Saravana Kannan wrote: > > > > On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck 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@603020318 { > > ibm,xive-provision-page-size = <0x1>; > > 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 0x318 0x00 0x1 0x60302 > > 0x319 0x00 0x1 0x60302 0x31a 0x00 0x1 0x60302 > > 0x31b 0x00 0x1>; > > 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@603020318 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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On Mon, 15 Feb 2021 08:29:53 +, Saravana Kannan wrote: > > On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck 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@603020318 { > ibm,xive-provision-page-size = <0x1>; > 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 0x318 0x00 0x1 0x60302 > 0x319 0x00 0x1 0x60302 0x31a 0x00 0x1 0x60302 > 0x31b 0x00 0x1>; > 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@603020318 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.
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Sun, Feb 14, 2021 at 7:58 PM Guenter Roeck 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@603020318 { ibm,xive-provision-page-size = <0x1>; 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 0x318 0x00 0x1 0x60302 0x319 0x00 0x1 0x60302 0x31a 0x00 0x1 0x60302 0x31b 0x00 0x1>; 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@603020318 DT node where there are multiple interrupt controllers? Thanks, Saravana
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Sat, Feb 13, 2021 at 10:54 AM Guenter Roeck 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 > > Cc: Kevin Hilman > > Cc: Greg Kroah-Hartman > > Reviewed-by: Rob Herring > > Reviewed-by: Thierry Reding > > Reviewed-by: Linus Walleij > > Signed-off-by: Saravana Kannan > > 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] [c2c07a70] [c08f052c] > dump_stack+0x100/0x174 (unreliable) > [ 14.248780][T1] [c2c07ab0] [c010d0e0] panic+0x190/0x450 > [ 14.249097][T1] [c2c07b50] [c14d1af8] > mount_block_root+0x320/0x430 > [ 14.249442][T1] [c2c07c50] [c14d1e64] > prepare_namespace+0x1b0/0x204 > [ 14.249798][T1] [c2c07cc0] [c14d1544] > kernel_init_freeable+0x3dc/0x438 > [ 14.250145][T1] [c2c07da0] [c0012b7c] > kernel_init+0x2c/0x170 > [ 14.250466][T1] [c2c07e10] [c000d56c] > 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..7fff pg=1^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-sarava...@google.com/ Thanks, Saravana
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
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 > Cc: Kevin Hilman > Cc: Greg Kroah-Hartman > Reviewed-by: Rob Herring > Reviewed-by: Thierry Reding > Reviewed-by: Linus Walleij > Signed-off-by: Saravana Kannan 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] [c2c07a70] [c08f052c] dump_stack+0x100/0x174 (unreliable) [ 14.248780][T1] [c2c07ab0] [c010d0e0] panic+0x190/0x450 [ 14.249097][T1] [c2c07b50] [c14d1af8] mount_block_root+0x320/0x430 [ 14.249442][T1] [c2c07c50] [c14d1e64] prepare_namespace+0x1b0/0x204 [ 14.249798][T1] [c2c07cc0] [c14d1544] kernel_init_freeable+0x3dc/0x438 [ 14.250145][T1] [c2c07da0] [c0012b7c] kernel_init+0x2c/0x170 [ 14.250466][T1] [c2c07e10] [c000d56c] 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..7fff pg=1^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: [987d576a592082b8e0e40236f49ad655f5dc] Merge remote-tracking branch 'crypto/master' git bisect good 987d576a592082b8e0e40236f49ad655f5dc # 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:
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Mon, Feb 8, 2021 at 12:14 AM Marek Szyprowski wrote: > > Hi Saravana, > > On 06.02.2021 05:32, Saravana Kannan wrote: > > On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan wrote: > >> On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven > >> wrote: > >>> On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan > >>> wrote: > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven > wrote: > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan > > wrote: > >> On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > >> wrote: > >>> On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > >>> wrote: > On 04.02.2021 22:31, Saravana Kannan wrote: > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > 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 > >>> Cc: Kevin Hilman > >>> Cc: Greg Kroah-Hartman > >>> Reviewed-by: Rob Herring > >>> Reviewed-by: Thierry Reding > >>> Reviewed-by: Linus Walleij > >>> Signed-off-by: Saravana Kannan > >> 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@5 > >> 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@2c01 { > > 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: > 1800.ethernetplatform: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c05.mmciamba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c1d.gpioamba: probe deferral - supplier > bus@800:motherboard-bus not ready > 2b60.iommu platform: probe deferral - wait for supplier > scpi-power-domains > 7ff5.hdlcd platform: probe deferral - wait for supplier > scpi-clk > 7ff6.hdlcd platform: probe deferral - wait for supplier > scpi-clk > 1c06.kmi amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c07.kmi amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c17.rtc amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c0f.wdt amba: probe deferral - supplier > bus@800: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@800: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@800 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 inte
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On 06.02.2021 05:32, Saravana Kannan wrote: > On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan wrote: >> On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven >> wrote: >>> On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan wrote: On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven wrote: > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan > wrote: >> On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven >> wrote: >>> On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski >>> wrote: On 04.02.2021 22:31, Saravana Kannan wrote: > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > 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 >>> Cc: Kevin Hilman >>> Cc: Greg Kroah-Hartman >>> Reviewed-by: Rob Herring >>> Reviewed-by: Thierry Reding >>> Reviewed-by: Linus Walleij >>> Signed-off-by: Saravana Kannan >> 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@5 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@2c01 { > 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: 1800.ethernetplatform: probe deferral - supplier bus@800:motherboard-bus not ready 1c05.mmciamba: probe deferral - supplier bus@800:motherboard-bus not ready 1c1d.gpioamba: probe deferral - supplier bus@800:motherboard-bus not ready 2b60.iommu platform: probe deferral - wait for supplier scpi-power-domains 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk 1c06.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c07.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c17.rtc amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c0f.wdt amba: probe deferral - supplier bus@800: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@800: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@800 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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan wrote: > > On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven > wrote: > > > > Hi Saravana, > > > > On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan wrote: > > > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven > > > wrote: > > > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan > > > > wrote: > > > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > > > > > wrote: > > > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > > > wrote: > > > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > > > 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 > > > > > > > >>> Cc: Kevin Hilman > > > > > > > >>> Cc: Greg Kroah-Hartman > > > > > > > >>> Reviewed-by: Rob Herring > > > > > > > >>> Reviewed-by: Thierry Reding > > > > > > > >>> Reviewed-by: Linus Walleij > > > > > > > >>> Signed-off-by: Saravana Kannan > > > > > > > >> 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@5 > > > > > > > >> 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@2c01 { > > > > > > > > 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: > > > > > > > 1800.ethernetplatform: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 1c05.mmciamba: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 1c1d.gpioamba: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 2b60.iommu platform: probe deferral - wait for supplier > > > > > > > scpi-power-domains > > > > > > > 7ff5.hdlcd platform: probe deferral - wait for supplier > > > > > > > scpi-clk > > > > > > > 7ff6.hdlcd platform: probe deferral - wait for supplier > > > > > > > scpi-clk > > > > > > > 1c06.kmi amba: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 1c07.kmi amba: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 1c17.rtc amba: probe deferral - supplier > > > > > > > bus@800:motherboard-bus not ready > > > > > > > 1c0f.wdt amba: probe deferral - supplier > > > > > > > bus@800: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@800: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@800 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)
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven wrote: > > Hi Marek, > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > wrote: > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > 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 > > >>> Cc: Kevin Hilman > > >>> Cc: Greg Kroah-Hartman > > >>> Reviewed-by: Rob Herring > > >>> Reviewed-by: Thierry Reding > > >>> Reviewed-by: Linus Walleij > > >>> Signed-off-by: Saravana Kannan > > >> 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@5 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@2c01 { > > > 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: > > 1800.ethernetplatform: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 1c05.mmciamba: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 1c1d.gpioamba: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 2b60.iommu platform: probe deferral - wait for supplier > > scpi-power-domains > > 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk > > 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk > > 1c06.kmi amba: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 1c07.kmi amba: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 1c17.rtc amba: probe deferral - supplier > > bus@800:motherboard-bus not ready > > 1c0f.wdt amba: probe deferral - supplier > > bus@800: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@800: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@800 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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven wrote: > > Hi Saravana, > > On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan wrote: > > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven > > wrote: > > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan > > > wrote: > > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > > > > wrote: > > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > > wrote: > > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > > 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 > > > > > > >>> Cc: Kevin Hilman > > > > > > >>> Cc: Greg Kroah-Hartman > > > > > > >>> Reviewed-by: Rob Herring > > > > > > >>> Reviewed-by: Thierry Reding > > > > > > >>> Reviewed-by: Linus Walleij > > > > > > >>> Signed-off-by: Saravana Kannan > > > > > > >> 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@5 > > > > > > >> 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@2c01 { > > > > > > > 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: > > > > > > 1800.ethernetplatform: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 1c05.mmciamba: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 1c1d.gpioamba: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 2b60.iommu platform: probe deferral - wait for supplier > > > > > > scpi-power-domains > > > > > > 7ff5.hdlcd platform: probe deferral - wait for supplier > > > > > > scpi-clk > > > > > > 7ff6.hdlcd platform: probe deferral - wait for supplier > > > > > > scpi-clk > > > > > > 1c06.kmi amba: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 1c07.kmi amba: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 1c17.rtc amba: probe deferral - supplier > > > > > > bus@800:motherboard-bus not ready > > > > > > 1c0f.wdt amba: probe deferral - supplier > > > > > > bus@800: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@800: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@800 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
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 wrote: > On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven > wrote: > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan > > wrote: > > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > > > wrote: > > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > > wrote: > > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > > 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 > > > > > >>> Cc: Kevin Hilman > > > > > >>> Cc: Greg Kroah-Hartman > > > > > >>> Reviewed-by: Rob Herring > > > > > >>> Reviewed-by: Thierry Reding > > > > > >>> Reviewed-by: Linus Walleij > > > > > >>> Signed-off-by: Saravana Kannan > > > > > >> 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@5 > > > > > >> 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@2c01 { > > > > > > 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: > > > > > 1800.ethernetplatform: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 1c05.mmciamba: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 1c1d.gpioamba: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 2b60.iommu platform: probe deferral - wait for supplier > > > > > scpi-power-domains > > > > > 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > > > 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > > > 1c06.kmi amba: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 1c07.kmi amba: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 1c17.rtc amba: probe deferral - supplier > > > > > bus@800:motherboard-bus not ready > > > > > 1c0f.wdt amba: probe deferral - supplier > > > > > bus@800: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@800: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@800 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 >
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven wrote: > > Hi Saravana, > > On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan wrote: > > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > > wrote: > > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > > wrote: > > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > > 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 > > > > >>> Cc: Kevin Hilman > > > > >>> Cc: Greg Kroah-Hartman > > > > >>> Reviewed-by: Rob Herring > > > > >>> Reviewed-by: Thierry Reding > > > > >>> Reviewed-by: Linus Walleij > > > > >>> Signed-off-by: Saravana Kannan > > > > >> 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@5 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@2c01 { > > > > > 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: > > > > 1800.ethernetplatform: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 1c05.mmciamba: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 1c1d.gpioamba: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 2b60.iommu platform: probe deferral - wait for supplier > > > > scpi-power-domains > > > > 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > > 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > > 1c06.kmi amba: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 1c07.kmi amba: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 1c17.rtc amba: probe deferral - supplier > > > > bus@800:motherboard-bus not ready > > > > 1c0f.wdt amba: probe deferral - supplier > > > > bus@800: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@800: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@800 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 interru
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan wrote: > On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven > wrote: > > On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski > > wrote: > > > On 04.02.2021 22:31, Saravana Kannan wrote: > > > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > > > 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 > > > >>> Cc: Kevin Hilman > > > >>> Cc: Greg Kroah-Hartman > > > >>> Reviewed-by: Rob Herring > > > >>> Reviewed-by: Thierry Reding > > > >>> Reviewed-by: Linus Walleij > > > >>> Signed-off-by: Saravana Kannan > > > >> 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@5 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@2c01 { > > > > 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: > > > 1800.ethernetplatform: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 1c05.mmciamba: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 1c1d.gpioamba: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 2b60.iommu platform: probe deferral - wait for supplier > > > scpi-power-domains > > > 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk > > > 1c06.kmi amba: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 1c07.kmi amba: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 1c17.rtc amba: probe deferral - supplier > > > bus@800:motherboard-bus not ready > > > 1c0f.wdt amba: probe deferral - supplier > > > bus@800: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@800: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@800 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 -- ge...@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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Marek, On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski wrote: > On 04.02.2021 22:31, Saravana Kannan wrote: > > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > > 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 > >>> Cc: Kevin Hilman > >>> Cc: Greg Kroah-Hartman > >>> Reviewed-by: Rob Herring > >>> Reviewed-by: Thierry Reding > >>> Reviewed-by: Linus Walleij > >>> Signed-off-by: Saravana Kannan > >> 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@5 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@2c01 { > > 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: > 1800.ethernetplatform: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c05.mmciamba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c1d.gpioamba: probe deferral - supplier > bus@800:motherboard-bus not ready > 2b60.iommu platform: probe deferral - wait for supplier > scpi-power-domains > 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk > 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk > 1c06.kmi amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c07.kmi amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c17.rtc amba: probe deferral - supplier > bus@800:motherboard-bus not ready > 1c0f.wdt amba: probe deferral - supplier > bus@800: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@800: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@800 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 -- ge...@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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On 04.02.2021 22:31, Saravana Kannan wrote: > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > 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 >>> Cc: Kevin Hilman >>> Cc: Greg Kroah-Hartman >>> Reviewed-by: Rob Herring >>> Reviewed-by: Thierry Reding >>> Reviewed-by: Linus Walleij >>> Signed-off-by: Saravana Kannan >> 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@5 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@2c01 { > 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: 1800.ethernet platform: probe deferral - supplier bus@800:motherboard-bus not ready 1c05.mmci amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c1d.gpio amba: probe deferral - supplier bus@800:motherboard-bus not ready 2b60.iommu platform: probe deferral - wait for supplier scpi-power-domains 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk 1c06.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c07.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c17.rtc amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c0f.wdt amba: probe deferral - supplier bus@800: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@800: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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski 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 > > Cc: Kevin Hilman > > Cc: Greg Kroah-Hartman > > Reviewed-by: Rob Herring > > Reviewed-by: Thierry Reding > > Reviewed-by: Linus Walleij > > Signed-off-by: Saravana Kannan > > 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@5 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@2c01 { 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
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
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 > Cc: Kevin Hilman > Cc: Greg Kroah-Hartman > Reviewed-by: Rob Herring > Reviewed-by: Thierry Reding > Reviewed-by: Linus Walleij > Signed-off-by: Saravana Kannan 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@5 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
[PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
This allows fw_devlink to create device links between consumers of an interrupt and the supplier of the interrupt. Cc: Marc Zyngier Cc: Kevin Hilman Cc: Greg Kroah-Hartman Reviewed-by: Rob Herring Reviewed-by: Thierry Reding Reviewed-by: Linus Walleij Signed-off-by: Saravana Kannan --- drivers/of/property.c | 11 +++ 1 file changed, 11 insertions(+) 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 #include #include +#include #include #include @@ -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, }, -- 2.30.0.296.g2bfb1c46d8-goog
[PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
This allows fw_devlink to create device links between consumers of an interrupt and the supplier of the interrupt. Cc: Marc Zyngier Cc: Kevin Hilman Cc: Greg Kroah-Hartman Reviewed-by: Rob Herring Reviewed-by: Thierry Reding Reviewed-by: Linus Walleij Signed-off-by: Saravana Kannan --- drivers/of/property.c | 11 +++ 1 file changed, 11 insertions(+) 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 #include #include +#include #include #include @@ -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, }, -- 2.30.0.296.g2bfb1c46d8-goog