All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: linux-kernel@vger.kernel.org, frowand.list@gmail.com,
	linux@armlinux.org.uk, linux-arm-kernel@lists.infradead.org,
	saravanak@google.com, linus.walleij@linaro.org,
	devicetree@vger.kernel.org, Ruizhe Lin <linruizhe@huawei.com>
Subject: Re: [PATCH v2 4/4] amba: Properly handle device probe without IRQ domain
Date: Tue, 31 Aug 2021 15:48:09 -0500	[thread overview]
Message-ID: <YS6ViZ98JGa/KJ+Z@robh.at.kernel.org> (raw)
In-Reply-To: <20210827150600.78811-5-wangkefeng.wang@huawei.com>

On Fri, Aug 27, 2021 at 11:06:00PM +0800, Kefeng Wang wrote:
> of_amba_device_create() uses irq_of_parse_and_map() to translate
> a DT interrupt specification into a Linux virtual interrupt number.
> 
> But it doesn't properly handle the case where the interrupt controller
> is not yet available, eg, when pl011 interrupt is connected to MBIGEN
> interrupt controller, because the mbigen initialization is too late,
> which will lead to no IRQ due to no IRQ domain found, log is shown below,
>   "irq: no irq domain found for uart0 !"
> 
> use of_irq_get() to return -EPROBE_DEFER as above, and in the driver
> deferred probe, it will properly handle in such case, also return 0
> in other fail cases to be consistent as before.
> 
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Reported-by: Ruizhe Lin <linruizhe@huawei.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/amba/bus.c    | 27 +++++++++++++++++++++++++++
>  drivers/of/platform.c |  6 +-----
>  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 4d3a565ca079..96e84ce66e9a 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -19,6 +19,7 @@
>  #include <linux/clk/clk-conf.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> +#include <linux/of_irq.h>
>  
>  #define to_amba_driver(d)	container_of(d, struct amba_driver, drv)
>  
> @@ -170,6 +171,28 @@ static int amba_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	return retval;
>  }
>  
> +static int of_amba_device_decode_irq(struct amba_device *dev)
> +{
> +	struct device_node *node = dev->dev.of_node;
> +	int i, irq;
> +
> +	if (IS_ENABLED(CONFIG_OF_IRQ) && node) {

I don't think this check is needed. If either is false, we should return 
an errno from of_irq_get().

> +		/* Decode the IRQs and address ranges */
> +		for (i = 0; i < AMBA_NR_IRQS; i++) {
> +			irq = of_irq_get(node, i);
> +			if (irq < 0) {
> +				if (irq == -EPROBE_DEFER)
> +					return irq;
> +				irq = 0;
> +			}
> +
> +			dev->irq[i] = irq;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * These are the device model conversion veneers; they convert the
>   * device model structures to our more specific structures.
> @@ -182,6 +205,10 @@ static int amba_probe(struct device *dev)
>  	int ret;
>  
>  	do {
> +		ret = of_amba_device_decode_irq(pcdev);
> +		if (ret)
> +			break;
> +
>  		ret = of_clk_set_defaults(dev->of_node, false);
>  		if (ret < 0)
>  			break;
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 74afbb7a4f5e..32d5ff8df747 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -222,7 +222,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  {
>  	struct amba_device *dev;
>  	const void *prop;
> -	int i, ret;
> +	int ret;
>  
>  	pr_debug("Creating amba device %pOF\n", node);
>  
> @@ -253,10 +253,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  	if (prop)
>  		dev->periphid = of_read_ulong(prop, 1);
>  
> -	/* Decode the IRQs and address ranges */
> -	for (i = 0; i < AMBA_NR_IRQS; i++)
> -		dev->irq[i] = irq_of_parse_and_map(node, i);
> -
>  	ret = of_address_to_resource(node, 0, &dev->res);
>  	if (ret) {
>  		pr_err("amba: of_address_to_resource() failed (%d) for %pOF\n",
> -- 
> 2.18.0.huawei.25
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Kefeng Wang <wangkefeng.wang@huawei.com>
Cc: linux-kernel@vger.kernel.org, frowand.list@gmail.com,
	linux@armlinux.org.uk, linux-arm-kernel@lists.infradead.org,
	saravanak@google.com, linus.walleij@linaro.org,
	devicetree@vger.kernel.org, Ruizhe Lin <linruizhe@huawei.com>
Subject: Re: [PATCH v2 4/4] amba: Properly handle device probe without IRQ domain
Date: Tue, 31 Aug 2021 15:48:09 -0500	[thread overview]
Message-ID: <YS6ViZ98JGa/KJ+Z@robh.at.kernel.org> (raw)
In-Reply-To: <20210827150600.78811-5-wangkefeng.wang@huawei.com>

On Fri, Aug 27, 2021 at 11:06:00PM +0800, Kefeng Wang wrote:
> of_amba_device_create() uses irq_of_parse_and_map() to translate
> a DT interrupt specification into a Linux virtual interrupt number.
> 
> But it doesn't properly handle the case where the interrupt controller
> is not yet available, eg, when pl011 interrupt is connected to MBIGEN
> interrupt controller, because the mbigen initialization is too late,
> which will lead to no IRQ due to no IRQ domain found, log is shown below,
>   "irq: no irq domain found for uart0 !"
> 
> use of_irq_get() to return -EPROBE_DEFER as above, and in the driver
> deferred probe, it will properly handle in such case, also return 0
> in other fail cases to be consistent as before.
> 
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Reported-by: Ruizhe Lin <linruizhe@huawei.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/amba/bus.c    | 27 +++++++++++++++++++++++++++
>  drivers/of/platform.c |  6 +-----
>  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 4d3a565ca079..96e84ce66e9a 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -19,6 +19,7 @@
>  #include <linux/clk/clk-conf.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
> +#include <linux/of_irq.h>
>  
>  #define to_amba_driver(d)	container_of(d, struct amba_driver, drv)
>  
> @@ -170,6 +171,28 @@ static int amba_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	return retval;
>  }
>  
> +static int of_amba_device_decode_irq(struct amba_device *dev)
> +{
> +	struct device_node *node = dev->dev.of_node;
> +	int i, irq;
> +
> +	if (IS_ENABLED(CONFIG_OF_IRQ) && node) {

I don't think this check is needed. If either is false, we should return 
an errno from of_irq_get().

> +		/* Decode the IRQs and address ranges */
> +		for (i = 0; i < AMBA_NR_IRQS; i++) {
> +			irq = of_irq_get(node, i);
> +			if (irq < 0) {
> +				if (irq == -EPROBE_DEFER)
> +					return irq;
> +				irq = 0;
> +			}
> +
> +			dev->irq[i] = irq;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * These are the device model conversion veneers; they convert the
>   * device model structures to our more specific structures.
> @@ -182,6 +205,10 @@ static int amba_probe(struct device *dev)
>  	int ret;
>  
>  	do {
> +		ret = of_amba_device_decode_irq(pcdev);
> +		if (ret)
> +			break;
> +
>  		ret = of_clk_set_defaults(dev->of_node, false);
>  		if (ret < 0)
>  			break;
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 74afbb7a4f5e..32d5ff8df747 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -222,7 +222,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  {
>  	struct amba_device *dev;
>  	const void *prop;
> -	int i, ret;
> +	int ret;
>  
>  	pr_debug("Creating amba device %pOF\n", node);
>  
> @@ -253,10 +253,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>  	if (prop)
>  		dev->periphid = of_read_ulong(prop, 1);
>  
> -	/* Decode the IRQs and address ranges */
> -	for (i = 0; i < AMBA_NR_IRQS; i++)
> -		dev->irq[i] = irq_of_parse_and_map(node, i);
> -
>  	ret = of_address_to_resource(node, 0, &dev->res);
>  	if (ret) {
>  		pr_err("amba: of_address_to_resource() failed (%d) for %pOF\n",
> -- 
> 2.18.0.huawei.25
> 
> 

  reply	other threads:[~2021-08-31 20:54 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-27 15:05 [PATCH v2 0/4] amba: Properly handle device probe without IRQ domain Kefeng Wang
2021-08-27 15:05 ` Kefeng Wang
2021-08-27 15:05 ` [PATCH v2 1/4] amba: Drop unused functions about APB/AHB devices add Kefeng Wang
2021-08-27 15:05   ` Kefeng Wang
2021-08-27 15:05 ` [PATCH v2 2/4] Revert "ARM: amba: make use of -1 IRQs warn" Kefeng Wang
2021-08-27 15:05   ` Kefeng Wang
2021-08-27 15:05 ` [PATCH v2 3/4] amba: Kill sysfs attribute file of irq Kefeng Wang
2021-08-27 15:05   ` Kefeng Wang
2021-08-31 20:44   ` Rob Herring
2021-08-31 20:44     ` Rob Herring
2021-08-27 15:06 ` [PATCH v2 4/4] amba: Properly handle device probe without IRQ domain Kefeng Wang
2021-08-27 15:06   ` Kefeng Wang
2021-08-31 20:48   ` Rob Herring [this message]
2021-08-31 20:48     ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YS6ViZ98JGa/KJ+Z@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=linruizhe@huawei.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=saravanak@google.com \
    --cc=wangkefeng.wang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.