Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Oct 2022 12:21:15 +0100
Message-ID: <864jvyg2qc.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Huacai Chen <chenhuacai@loongson.cn>
Cc:     Thomas Gleixner <tglx@linutronix.de>, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org, Xuefeng Li <lixuefeng@loongson.cn>,
        Huacai Chen <chenhuacai@gmail.com>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>
Subject: Re: [PATCH] irqchip/loongarch: Adjust acpi_cascade_irqdomain_init() and sub-routines
In-Reply-To: <20221020073346.536494-1-chenhuacai@loongson.cn>
References: <20221020073346.536494-1-chenhuacai@loongson.cn>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Thu, 20 Oct 2022 08:33:46 +0100,
Huacai Chen <chenhuacai@loongson.cn> wrote:
> 
> 1, Adjust prototype of acpi_cascade_irqdomain_init() because we don't
>    need its return value.
> 2, Combine unnecessary short lines to one long line.
> 
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
>  drivers/irqchip/irq-loongarch-cpu.c    | 19 +++++++------------
>  drivers/irqchip/irq-loongson-eiointc.c | 19 +++++++------------
>  drivers/irqchip/irq-loongson-pch-pic.c | 11 ++++-------
>  3 files changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-loongarch-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
> index 741612ba6a52..093609c8eaa7 100644
> --- a/drivers/irqchip/irq-loongarch-cpu.c
> +++ b/drivers/irqchip/irq-loongarch-cpu.c
> @@ -92,31 +92,26 @@ static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
>  	.xlate = irq_domain_xlate_onecell,
>  };
>  
> -static int __init
> -liointc_parse_madt(union acpi_subtable_headers *header,
> -		       const unsigned long end)
> +static int __init liointc_parse_madt(union acpi_subtable_headers *header,
> +					const unsigned long end)
>  {
>  	struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
>  
>  	return liointc_acpi_init(irq_domain, liointc_entry);
>  }
>  
> -static int __init
> -eiointc_parse_madt(union acpi_subtable_headers *header,
> -		       const unsigned long end)
> +static int __init eiointc_parse_madt(union acpi_subtable_headers *header,
> +					const unsigned long end)
>  {
>  	struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
>  
>  	return eiointc_acpi_init(irq_domain, eiointc_entry);
>  }
>  
> -static int __init acpi_cascade_irqdomain_init(void)
> +static void __init acpi_cascade_irqdomain_init(void)
>  {
> -	acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
> -			      liointc_parse_madt, 0);
> -	acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
> -			      eiointc_parse_madt, 0);
> -	return 0;
> +	acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC, liointc_parse_madt, 0);
> +	acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC, eiointc_parse_madt, 0);

This definitely looks like it is moving in the wrong direction. The
parsing can return an error, and you're not handling it anywhere.
Given how often firmware tables are broken, this seems pretty bad.

	M.

-- 
Without deviation from the norm, progress is not possible.