diff mbox series

[1/2] iommu/arm-smmu-v3: Don't display an error when IRQ lines are missing

Message ID 20191111111721.4145919-1-jean-philippe@linaro.org (mailing list archive)
State Mainlined
Commit f7aff1a93f52047739af31072de0ad8d149641f3
Headers show
Series [1/2] iommu/arm-smmu-v3: Don't display an error when IRQ lines are missing | expand

Commit Message

Jean-Philippe Brucker Nov. 11, 2019, 11:17 a.m. UTC
Since commit 7723f4c5ecdb ("driver core: platform: Add an error message
to platform_get_irq*()"), platform_get_irq_byname() displays an error
when the IRQ isn't found. Since the SMMUv3 driver uses that function to
query which interrupt method is available, the message is now displayed
during boot for any SMMUv3 that doesn't implement the combined
interrupt, or that implements MSIs.

[   20.700337] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ combined not found
[   20.706508] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ eventq not found
[   20.712503] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ priq not found
[   20.718325] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ gerror not found

Use platform_get_irq_byname_optional() to avoid displaying a spurious
error.

Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()")
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 drivers/iommu/arm-smmu-v3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

John Garry Nov. 11, 2019, 11:52 a.m. UTC | #1
On 11/11/2019 11:17, Jean-Philippe Brucker wrote:
> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message
> to platform_get_irq*()"), platform_get_irq_byname() displays an error
> when the IRQ isn't found. Since the SMMUv3 driver uses that function to
> query which interrupt method is available, the message is now displayed
> during boot for any SMMUv3 that doesn't implement the combined
> interrupt, or that implements MSIs.
> 
> [   20.700337] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ combined not found
> [   20.706508] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ eventq not found
> [   20.712503] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ priq not found
> [   20.718325] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ gerror not found
> 
> Use platform_get_irq_byname_optional() to avoid displaying a spurious
> error.
> 
> Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

That stops a nuisance:
Tested-by: John Garry <john.garry@huawei.com>

However, I will say though that the combined irq seems necessary for 
TX2, which is not warned about being missing now.

Finally, A cover letter would have been handy to mention that the new 
API was only introduced after rc1

thanks

> ---
>   drivers/iommu/arm-smmu-v3.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index bfa4a0f39ed0..a89797f346a4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -5207,19 +5207,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>   
>   	/* Interrupt lines */
>   
> -	irq = platform_get_irq_byname(pdev, "combined");
> +	irq = platform_get_irq_byname_optional(pdev, "combined");
>   	if (irq > 0)
>   		smmu->combined_irq = irq;
>   	else {
> -		irq = platform_get_irq_byname(pdev, "eventq");
> +		irq = platform_get_irq_byname_optional(pdev, "eventq");
>   		if (irq > 0)
>   			smmu->evtq.q.irq = irq;
>   
> -		irq = platform_get_irq_byname(pdev, "priq");
> +		irq = platform_get_irq_byname_optional(pdev, "priq");
>   		if (irq > 0)
>   			smmu->priq.q.irq = irq;
>   
> -		irq = platform_get_irq_byname(pdev, "gerror");
> +		irq = platform_get_irq_byname_optional(pdev, "gerror");
>   		if (irq > 0)
>   			smmu->gerr_irq = irq;
>   	}
>
Jean-Philippe Brucker Nov. 11, 2019, 2:23 p.m. UTC | #2
Hi John,

On Mon, Nov 11, 2019 at 11:52:38AM +0000, John Garry wrote:
> On 11/11/2019 11:17, Jean-Philippe Brucker wrote:
> > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message
> > to platform_get_irq*()"), platform_get_irq_byname() displays an error
> > when the IRQ isn't found. Since the SMMUv3 driver uses that function to
> > query which interrupt method is available, the message is now displayed
> > during boot for any SMMUv3 that doesn't implement the combined
> > interrupt, or that implements MSIs.
> > 
> > [   20.700337] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ combined not found
> > [   20.706508] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ eventq not found
> > [   20.712503] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ priq not found
> > [   20.718325] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ gerror not found
> > 
> > Use platform_get_irq_byname_optional() to avoid displaying a spurious
> > error.
> > 
> > Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()")
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> That stops a nuisance:
> Tested-by: John Garry <john.garry@huawei.com>

Thanks

> However, I will say though that the combined irq seems necessary for TX2,
> which is not warned about being missing now.

I don't think we warned about this before commit 7723f4c5ecdb either.
There are some warnings later in arm_smmu_setup_irqs() if the firmware
didn't describe any wiring at all, but we don't check whether the TX2 does
have the combined interrupt. Personally I wouldn't tie this to one SMMU
implementation because it would prevent from supporting a (hypothetical)
platform that integrates the same SMMU but "fixes" the IRQ lines.

> Finally, A cover letter would have been handy to mention that the new API
> was only introduced after rc1

Oh, I didn't even notice that, I thought it was introduced by rc1. I'll
add a cover if I send a v2.

Thanks,
Jean
Will Deacon Nov. 11, 2019, 2:36 p.m. UTC | #3
On Mon, Nov 11, 2019 at 12:17:20PM +0100, Jean-Philippe Brucker wrote:
> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message
> to platform_get_irq*()"), platform_get_irq_byname() displays an error
> when the IRQ isn't found. Since the SMMUv3 driver uses that function to
> query which interrupt method is available, the message is now displayed
> during boot for any SMMUv3 that doesn't implement the combined
> interrupt, or that implements MSIs.
> 
> [   20.700337] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ combined not found
> [   20.706508] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ eventq not found
> [   20.712503] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ priq not found
> [   20.718325] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ gerror not found
> 
> Use platform_get_irq_byname_optional() to avoid displaying a spurious
> error.
> 
> Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index bfa4a0f39ed0..a89797f346a4 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -5207,19 +5207,19 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  
>  	/* Interrupt lines */
>  
> -	irq = platform_get_irq_byname(pdev, "combined");
> +	irq = platform_get_irq_byname_optional(pdev, "combined");
>  	if (irq > 0)
>  		smmu->combined_irq = irq;
>  	else {
> -		irq = platform_get_irq_byname(pdev, "eventq");
> +		irq = platform_get_irq_byname_optional(pdev, "eventq");
>  		if (irq > 0)
>  			smmu->evtq.q.irq = irq;
>  
> -		irq = platform_get_irq_byname(pdev, "priq");
> +		irq = platform_get_irq_byname_optional(pdev, "priq");
>  		if (irq > 0)
>  			smmu->priq.q.irq = irq;
>  
> -		irq = platform_get_irq_byname(pdev, "gerror");
> +		irq = platform_get_irq_byname_optional(pdev, "gerror");
>  		if (irq > 0)
>  			smmu->gerr_irq = irq;
>  	}

Thanks, looks mechanical enough for me:

Acked-by: Will Deacon <will@kernel.org>

Will
Joerg Roedel Nov. 11, 2019, 2:44 p.m. UTC | #4
On Mon, Nov 11, 2019 at 12:17:20PM +0100, Jean-Philippe Brucker wrote:
> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message
> to platform_get_irq*()"), platform_get_irq_byname() displays an error
> when the IRQ isn't found. Since the SMMUv3 driver uses that function to
> query which interrupt method is available, the message is now displayed
> during boot for any SMMUv3 that doesn't implement the combined
> interrupt, or that implements MSIs.
> 
> [   20.700337] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ combined not found
> [   20.706508] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ eventq not found
> [   20.712503] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ priq not found
> [   20.718325] arm-smmu-v3 arm-smmu-v3.7.auto: IRQ gerror not found
> 
> Use platform_get_irq_byname_optional() to avoid displaying a spurious
> error.
> 
> Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()")
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Applied both, thanks.
diff mbox series

Patch

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index bfa4a0f39ed0..a89797f346a4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -5207,19 +5207,19 @@  static int arm_smmu_device_probe(struct platform_device *pdev)
 
 	/* Interrupt lines */
 
-	irq = platform_get_irq_byname(pdev, "combined");
+	irq = platform_get_irq_byname_optional(pdev, "combined");
 	if (irq > 0)
 		smmu->combined_irq = irq;
 	else {
-		irq = platform_get_irq_byname(pdev, "eventq");
+		irq = platform_get_irq_byname_optional(pdev, "eventq");
 		if (irq > 0)
 			smmu->evtq.q.irq = irq;
 
-		irq = platform_get_irq_byname(pdev, "priq");
+		irq = platform_get_irq_byname_optional(pdev, "priq");
 		if (irq > 0)
 			smmu->priq.q.irq = irq;
 
-		irq = platform_get_irq_byname(pdev, "gerror");
+		irq = platform_get_irq_byname_optional(pdev, "gerror");
 		if (irq > 0)
 			smmu->gerr_irq = irq;
 	}