linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] of: property: Add fw_devlink support for more props
@ 2021-01-21 22:57 Saravana Kannan
  2021-01-21 22:57 ` [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding Saravana Kannan
                   ` (3 more replies)
  0 siblings, 4 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Greg Kroah-Hartman
  Cc: Saravana Kannan, linux-tegra, devicetree, linux-kernel,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, kernel-team

Sending again because I messed up the To/Cc for the coverletter.

This series combines two patches [1] [2] that'd conflict.

Greg,

Can you please pull this into driver-core-next?

-Saravana

[1] - https://lore.kernel.org/lkml/20210115210159.3090203-1-saravanak@google.com/
[2] - https://lore.kernel.org/lkml/20201218210750.3455872-1-saravanak@google.com/

Individual -> Series:
Patch 1/2: Addressed Geert's gpio-hog problem with gpio[s] property
Patch 2/2: Switched to using of_irq_find_parent()

v1 -> v2:
Patch 1/2: Added the Reviewed-by tags
Patch 2/2: Updated commit text and comments. Added Reviewed-by tags.

Saravana Kannan (2):
  of: property: Add fw_devlink support for "gpio" and "gpios" binding
  of: property: Add fw_devlink support for interrupts

 drivers/of/property.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

-- 
2.30.0.296.g2bfb1c46d8-goog


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding
  2021-01-21 22:57 [PATCH v2 0/2] of: property: Add fw_devlink support for more props Saravana Kannan
@ 2021-01-21 22:57 ` Saravana Kannan
  2021-01-22 12:58   ` Linus Walleij
  2021-01-21 22:57 ` [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts Saravana Kannan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Greg Kroah-Hartman, Saravana Kannan
  Cc: linux-tegra, devicetree, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Geert Uytterhoeven, Jon Hunter,
	Marc Zyngier, Kevin Hilman, kernel-team, Rob Herring,
	Thierry Reding

To provide backward compatibility for boards that use deprecated DT
bindings, we need to add fw_devlink support for "gpio" and "gpios".

We also need to ignore these properties on nodes with "gpio-hog"
property because their gpio[s] are all supplied by the parent node.

Cc: linux-tegra <linux-tegra@vger.kernel.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 drivers/of/property.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 5f9eed79a8aa..b2ea1951d937 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1271,6 +1271,28 @@ static struct device_node *parse_iommu_maps(struct device_node *np,
 	return of_parse_phandle(np, prop_name, (index * 4) + 1);
 }
 
+static struct device_node *parse_gpio_compat(struct device_node *np,
+					     const char *prop_name, int index)
+{
+	struct of_phandle_args sup_args;
+
+	if (strcmp(prop_name, "gpio") && strcmp(prop_name, "gpios"))
+		return NULL;
+
+	/*
+	 * Ignore node with gpio-hog property since its gpios are all provided
+	 * by its parent.
+	 */
+	if (of_find_property(np, "gpio-hog", NULL))
+		return NULL;
+
+	if (of_parse_phandle_with_args(np, prop_name, "#gpio-cells", index,
+				       &sup_args))
+		return NULL;
+
+	return sup_args.np;
+}
+
 static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_clocks, },
 	{ .parse_prop = parse_interconnects, },
@@ -1296,6 +1318,7 @@ static const struct supplier_bindings of_supplier_bindings[] = {
 	{ .parse_prop = parse_pinctrl6, },
 	{ .parse_prop = parse_pinctrl7, },
 	{ .parse_prop = parse_pinctrl8, },
+	{ .parse_prop = parse_gpio_compat, },
 	{ .parse_prop = parse_regulators, },
 	{ .parse_prop = parse_gpio, },
 	{ .parse_prop = parse_gpios, },
-- 
2.30.0.296.g2bfb1c46d8-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-01-21 22:57 [PATCH v2 0/2] of: property: Add fw_devlink support for more props Saravana Kannan
  2021-01-21 22:57 ` [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding Saravana Kannan
@ 2021-01-21 22:57 ` Saravana Kannan
       [not found]   ` <CGME20210204115252eucas1p2d145686f7a5dc7e7a04dddd0b0f2286c@eucas1p2.samsung.com>
  2021-02-13 18:54   ` Guenter Roeck
  2021-01-26  8:22 ` [PATCH v2 0/2] of: property: Add fw_devlink support for more props Greg Kroah-Hartman
       [not found] ` <20210131163823.c4zb47pl4tukcl7c@viti.kaiser.cx>
  3 siblings, 2 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-01-21 22:57 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand, Greg Kroah-Hartman
  Cc: Saravana Kannan, linux-tegra, devicetree, linux-kernel,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, kernel-team, Rob Herring,
	Thierry Reding

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>
---
 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 <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, },
-- 
2.30.0.296.g2bfb1c46d8-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding
  2021-01-21 22:57 ` [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding Saravana Kannan
@ 2021-01-22 12:58   ` Linus Walleij
  0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2021-01-22 12:58 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

On Thu, Jan 21, 2021 at 11:57 PM Saravana Kannan <saravanak@google.com> wrote:

> To provide backward compatibility for boards that use deprecated DT
> bindings, we need to add fw_devlink support for "gpio" and "gpios".
>
> We also need to ignore these properties on nodes with "gpio-hog"
> property because their gpio[s] are all supplied by the parent node.
>
> Cc: linux-tegra <linux-tegra@vger.kernel.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Thierry Reding <treding@nvidia.com>
> Tested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Saravana Kannan <saravanak@google.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/2] of: property: Add fw_devlink support for more props
  2021-01-21 22:57 [PATCH v2 0/2] of: property: Add fw_devlink support for more props Saravana Kannan
  2021-01-21 22:57 ` [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding Saravana Kannan
  2021-01-21 22:57 ` [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts Saravana Kannan
@ 2021-01-26  8:22 ` Greg Kroah-Hartman
       [not found] ` <20210131163823.c4zb47pl4tukcl7c@viti.kaiser.cx>
  3 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-26  8:22 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, linux-tegra, devicetree, linux-kernel,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, kernel-team

On Thu, Jan 21, 2021 at 02:57:10PM -0800, Saravana Kannan wrote:
> Sending again because I messed up the To/Cc for the coverletter.
> 
> This series combines two patches [1] [2] that'd conflict.
> 
> Greg,
> 
> Can you please pull this into driver-core-next?

Now queued up, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/2] of: property: Add fw_devlink support for more props
       [not found] ` <20210131163823.c4zb47pl4tukcl7c@viti.kaiser.cx>
@ 2021-01-31 21:05   ` Saravana Kannan
  2021-02-01 10:52     ` Martin Kaiser
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-01-31 21:05 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team

On Sun, Jan 31, 2021 at 8:38 AM Martin Kaiser <martin@kaiser.cx> wrote:
>
> Dear all,
>
> Thus wrote Saravana Kannan (saravanak@google.com):
>
> > Sending again because I messed up the To/Cc for the coverletter.
>
> > This series combines two patches [1] [2] that'd conflict.
>
> > Greg,
>
> > Can you please pull this into driver-core-next?
>
> > -Saravana
>
> > [1] - https://lore.kernel.org/lkml/20210115210159.3090203-1-saravanak@google.com/
> > [2] - https://lore.kernel.org/lkml/20201218210750.3455872-1-saravanak@google.com/
>
> I'm running linux-next on my hardware which is based on the imx258
> chipset by Freescale/NXP.
>
> When those two patches appeared in linux-next, my system would not boot
> any more. It was stuck right after
>
> Uncompressing Linux... done, booting the kernel.
>
> Reverting the irq-patch made the system boot again. Still, a number of
> devices like usb or nand flash controller are not found any more.
> If I revert the gpio patch as well, all devices are available again.
>
> My system's device tree is based on arch/arm/boot/dts/imx25.dtsi with
> very few adaptations for my board.
>
> I tried to play around with the new parse_interrupts() function to
> figure out which device causes the boot failure. If I skip the following
> device, I can boot again:
>
> -       return of_irq_find_parent(np);
> +       np_ret = of_irq_find_parent(np);
> +       if (!strcmp(np->full_name, "serial@50008000")) {
> +           printk(KERN_ERR "skip serial@50008000\n");
> +           return NULL;
> +       }
> +      return np_ret;
>
> This is uart4 of the imx258 chip, which I use as my serial console. The
> imx25.dtsi device tree seems ok, we find an interrupt parent. The
> problem must be in the code that processes the result of
> parse_interrupts().
>
> I tried to boot the unmodified code with qemu, simulating the imx25-pdk
> device. This wouldn't boot either.
>
> Does this ring any bells with anyone?

This series [1] has a high chance of fixing it for you if
CONFIG_MODULES is disabled in your set up. Can you give it a shot?

The real problem is that arch/arm/mach-imx/avic.c doesn't set the
OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
information to know that this device node will never have a struct
device created for it. The proper way to do this for root IRQCHIP
nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
IMX [2], can you please give [2] a shot *without* [1] and with
CONFIG_MODULES enabled? Things should boot properly with this
combination too.

Btw, for future reference, you can try enabling the logs in
device_links_check_suppliers() to see what devices are being blocked
on what supplier nodes.

[1] - https://lore.kernel.org/lkml/20210130040344.2807439-1-saravanak@google.com/
[2] - https://lore.kernel.org/lkml/20210131205654.3379661-1-saravanak@google.com/T/#u

Thanks,
Saravana

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/2] of: property: Add fw_devlink support for more props
  2021-01-31 21:05   ` Saravana Kannan
@ 2021-02-01 10:52     ` Martin Kaiser
  2021-02-01 20:04       ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Martin Kaiser @ 2021-02-01 10:52 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team

Hi Saravana,

Thus wrote Saravana Kannan (saravanak@google.com):

> This series [1] has a high chance of fixing it for you if
> CONFIG_MODULES is disabled in your set up. Can you give it a shot?

sure. This fixes things for me if CONFIG_MODULES is disabled. Booting is
still stuck if modules are enabled.

> The real problem is that arch/arm/mach-imx/avic.c doesn't set the
> OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
> information to know that this device node will never have a struct
> device created for it. The proper way to do this for root IRQCHIP
> nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
> IMX [2], can you please give [2] a shot *without* [1] and with
> CONFIG_MODULES enabled? Things should boot properly with this
> combination too.

This works as well.

Thanks,
Martin

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/2] of: property: Add fw_devlink support for more props
  2021-02-01 10:52     ` Martin Kaiser
@ 2021-02-01 20:04       ` Saravana Kannan
  0 siblings, 0 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-02-01 20:04 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team

On Mon, Feb 1, 2021 at 2:52 AM Martin Kaiser <martin@kaiser.cx> wrote:
>
> Hi Saravana,
>
> Thus wrote Saravana Kannan (saravanak@google.com):
>
> > This series [1] has a high chance of fixing it for you if
> > CONFIG_MODULES is disabled in your set up. Can you give it a shot?
>
> sure. This fixes things for me if CONFIG_MODULES is disabled. Booting is
> still stuck if modules are enabled.
>
> > The real problem is that arch/arm/mach-imx/avic.c doesn't set the
> > OF_POPULATED flag for the "fsl,avic" node. fw_devlink uses this
> > information to know that this device node will never have a struct
> > device created for it. The proper way to do this for root IRQCHIP
> > nodes is to use IRQCHIP_DECLARE(). I Cc'ed you on a clean up patch for
> > IMX [2], can you please give [2] a shot *without* [1] and with
> > CONFIG_MODULES enabled? Things should boot properly with this
> > combination too.
>
> This works as well.

Thanks for testing both. Mind giving Tested-by for [1] too?

-Saravana

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
       [not found]   ` <CGME20210204115252eucas1p2d145686f7a5dc7e7a04dddd0b0f2286c@eucas1p2.samsung.com>
@ 2021-02-04 11:52     ` Marek Szyprowski
  2021-02-04 21:31       ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Marek Szyprowski @ 2021-02-04 11:52 UTC (permalink / raw)
  To: Saravana Kannan, Rob Herring, Frank Rowand, Greg Kroah-Hartman
  Cc: linux-tegra, devicetree, linux-kernel, Linus Walleij,
	Bartosz Golaszewski, Geert Uytterhoeven, Jon Hunter,
	Marc Zyngier, Kevin Hilman, kernel-team, Rob Herring,
	Thierry Reding

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-04 11:52     ` Marek Szyprowski
@ 2021-02-04 21:31       ` Saravana Kannan
  2021-02-05  7:38         ` Marek Szyprowski
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-04 21:31 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-04 21:31       ` Saravana Kannan
@ 2021-02-05  7:38         ` Marek Szyprowski
  2021-02-05  8:06           ` Geert Uytterhoeven
  0 siblings, 1 reply; 28+ messages in thread
From: Marek Szyprowski @ 2021-02-05  7:38 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05  7:38         ` Marek Szyprowski
@ 2021-02-05  8:06           ` Geert Uytterhoeven
  2021-02-05 10:05             ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2021-02-05  8:06 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Saravana Kannan, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05  8:06           ` Geert Uytterhoeven
@ 2021-02-05 10:05             ` Saravana Kannan
  2021-02-05 10:20               ` Geert Uytterhoeven
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-05 10:05 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05 10:05             ` Saravana Kannan
@ 2021-02-05 10:20               ` Geert Uytterhoeven
  2021-02-05 17:19                 ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2021-02-05 10:20 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05 10:20               ` Geert Uytterhoeven
@ 2021-02-05 17:19                 ` Saravana Kannan
  2021-02-05 17:51                   ` Geert Uytterhoeven
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-05 17:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05 17:19                 ` Saravana Kannan
@ 2021-02-05 17:51                   ` Geert Uytterhoeven
  2021-02-05 17:55                     ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2021-02-05 17:51 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05 17:51                   ` Geert Uytterhoeven
@ 2021-02-05 17:55                     ` Saravana Kannan
  2021-02-06  4:32                       ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-05 17:55 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-05 17:55                     ` Saravana Kannan
@ 2021-02-06  4:32                       ` Saravana Kannan
  2021-02-08  8:14                         ` Marek Szyprowski
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-06  4:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Marek Szyprowski, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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;
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-06  4:32                       ` Saravana Kannan
@ 2021-02-08  8:14                         ` Marek Szyprowski
  2021-02-08 23:57                           ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Marek Szyprowski @ 2021-02-08  8:14 UTC (permalink / raw)
  To: Saravana Kannan, Geert Uytterhoeven
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-08  8:14                         ` Marek Szyprowski
@ 2021-02-08 23:57                           ` Saravana Kannan
  0 siblings, 0 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-02-08 23:57 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Geert Uytterhoeven, Rob Herring, Frank Rowand,
	Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Jon Hunter, Marc Zyngier,
	Kevin Hilman, Android Kernel Team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-01-21 22:57 ` [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts Saravana Kannan
       [not found]   ` <CGME20210204115252eucas1p2d145686f7a5dc7e7a04dddd0b0f2286c@eucas1p2.samsung.com>
@ 2021-02-13 18:54   ` Guenter Roeck
  2021-02-14 21:12     ` Saravana Kannan
  1 sibling, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2021-02-13 18:54 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	devicetree, linux-kernel, Linus Walleij, Bartosz Golaszewski,
	Geert Uytterhoeven, Jon Hunter, Marc Zyngier, Kevin Hilman,
	kernel-team, Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-13 18:54   ` Guenter Roeck
@ 2021-02-14 21:12     ` Saravana Kannan
  2021-02-15  3:06       ` Guenter Roeck
  2021-02-15  3:58       ` Guenter Roeck
  0 siblings, 2 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-02-14 21:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-14 21:12     ` Saravana Kannan
@ 2021-02-15  3:06       ` Guenter Roeck
  2021-02-15  3:58       ` Guenter Roeck
  1 sibling, 0 replies; 28+ messages in thread
From: Guenter Roeck @ 2021-02-15  3:06 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-14 21:12     ` Saravana Kannan
  2021-02-15  3:06       ` Guenter Roeck
@ 2021-02-15  3:58       ` Guenter Roeck
  2021-02-15  8:29         ` Saravana Kannan
  1 sibling, 1 reply; 28+ messages in thread
From: Guenter Roeck @ 2021-02-15  3:58 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

[-- Attachment #1: Type: text/plain, Size: 240 bytes --]

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.

Guenter

[-- Attachment #2: powernv.dts --]
[-- Type: audio/vnd.dts, Size: 697342 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-15  3:58       ` Guenter Roeck
@ 2021-02-15  8:29         ` Saravana Kannan
  2021-02-15  9:08           ` Marc Zyngier
  0 siblings, 1 reply; 28+ messages in thread
From: Saravana Kannan @ 2021-02-15  8:29 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Frank Rowand, Greg Kroah-Hartman, linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Marc Zyngier, Kevin Hilman, Android Kernel Team,
	Rob Herring, Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-15  8:29         ` Saravana Kannan
@ 2021-02-15  9:08           ` Marc Zyngier
  2021-02-15 22:23             ` Saravana Kannan
  0 siblings, 1 reply; 28+ messages in thread
From: Marc Zyngier @ 2021-02-15  9:08 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Guenter Roeck, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Kevin Hilman, Android Kernel Team, Rob Herring,
	Thierry Reding

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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
  2021-02-15  9:08           ` Marc Zyngier
@ 2021-02-15 22:23             ` Saravana Kannan
  0 siblings, 0 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-02-15 22:23 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Guenter Roeck, Rob Herring, Frank Rowand, Greg Kroah-Hartman,
	linux-tegra,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Linus Walleij, Bartosz Golaszewski, Geert Uytterhoeven,
	Jon Hunter, Kevin Hilman, Android Kernel Team, Rob Herring,
	Thierry Reding

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
       [not found] <20210121191637.1067630-1-saravanak@google.com>
@ 2021-01-21 19:16 ` Saravana Kannan
  0 siblings, 0 replies; 28+ messages in thread
From: Saravana Kannan @ 2021-01-21 19:16 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: Saravana Kannan, kernel-team, Marc Zyngier, Kevin Hilman,
	Greg Kroah-Hartman, Rob Herring, Thierry Reding, Linus Walleij,
	devicetree, linux-kernel

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>
---
 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 <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, },
-- 
2.30.0.296.g2bfb1c46d8-goog


^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-02-15 22:26 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 22:57 [PATCH v2 0/2] of: property: Add fw_devlink support for more props Saravana Kannan
2021-01-21 22:57 ` [PATCH v2 1/2] of: property: Add fw_devlink support for "gpio" and "gpios" binding Saravana Kannan
2021-01-22 12:58   ` Linus Walleij
2021-01-21 22:57 ` [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts Saravana Kannan
     [not found]   ` <CGME20210204115252eucas1p2d145686f7a5dc7e7a04dddd0b0f2286c@eucas1p2.samsung.com>
2021-02-04 11:52     ` Marek Szyprowski
2021-02-04 21:31       ` Saravana Kannan
2021-02-05  7:38         ` Marek Szyprowski
2021-02-05  8:06           ` Geert Uytterhoeven
2021-02-05 10:05             ` Saravana Kannan
2021-02-05 10:20               ` Geert Uytterhoeven
2021-02-05 17:19                 ` Saravana Kannan
2021-02-05 17:51                   ` Geert Uytterhoeven
2021-02-05 17:55                     ` Saravana Kannan
2021-02-06  4:32                       ` Saravana Kannan
2021-02-08  8:14                         ` Marek Szyprowski
2021-02-08 23:57                           ` Saravana Kannan
2021-02-13 18:54   ` Guenter Roeck
2021-02-14 21:12     ` Saravana Kannan
2021-02-15  3:06       ` Guenter Roeck
2021-02-15  3:58       ` Guenter Roeck
2021-02-15  8:29         ` Saravana Kannan
2021-02-15  9:08           ` Marc Zyngier
2021-02-15 22:23             ` Saravana Kannan
2021-01-26  8:22 ` [PATCH v2 0/2] of: property: Add fw_devlink support for more props Greg Kroah-Hartman
     [not found] ` <20210131163823.c4zb47pl4tukcl7c@viti.kaiser.cx>
2021-01-31 21:05   ` Saravana Kannan
2021-02-01 10:52     ` Martin Kaiser
2021-02-01 20:04       ` Saravana Kannan
     [not found] <20210121191637.1067630-1-saravanak@google.com>
2021-01-21 19:16 ` [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts Saravana Kannan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).