2022-10-20 07:42:15

by Huacai Chen

[permalink] [raw]
Subject: [PATCH] irqchip/loongarch: Adjust acpi_cascade_irqdomain_init() and sub-routines

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 <[email protected]>
---
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);
}

static int __init cpuintc_acpi_init(union acpi_subtable_headers *header,
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
index 16e9af8d8b1e..efead0bfb1ca 100644
--- a/drivers/irqchip/irq-loongson-eiointc.c
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -301,9 +301,8 @@ static struct irq_domain *acpi_get_vec_parent(int node, struct acpi_vector_group
return NULL;
}

-static int __init
-pch_pic_parse_madt(union acpi_subtable_headers *header,
- const unsigned long end)
+static int __init pch_pic_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
{
struct acpi_madt_bio_pic *pchpic_entry = (struct acpi_madt_bio_pic *)header;
unsigned int node = (pchpic_entry->address >> 44) & 0xf;
@@ -315,9 +314,8 @@ pch_pic_parse_madt(union acpi_subtable_headers *header,
return -EINVAL;
}

-static int __init
-pch_msi_parse_madt(union acpi_subtable_headers *header,
- const unsigned long end)
+static int __init pch_msi_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
{
struct acpi_madt_msi_pic *pchmsi_entry = (struct acpi_madt_msi_pic *)header;
struct irq_domain *parent = acpi_get_vec_parent(eiointc_priv[nr_pics - 1]->node, msi_group);
@@ -328,13 +326,10 @@ pch_msi_parse_madt(union acpi_subtable_headers *header,
return -EINVAL;
}

-static int __init acpi_cascade_irqdomain_init(void)
+static void __init acpi_cascade_irqdomain_init(void)
{
- acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
- pch_pic_parse_madt, 0);
- acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC,
- pch_msi_parse_madt, 1);
- return 0;
+ acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC, pch_pic_parse_madt, 0);
+ acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC, pch_msi_parse_madt, 1);
}

int __init eiointc_acpi_init(struct irq_domain *parent,
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index c01b9c257005..d2cf54a87bee 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -325,20 +325,17 @@ int find_pch_pic(u32 gsi)
return -1;
}

-static int __init
-pch_lpc_parse_madt(union acpi_subtable_headers *header,
- const unsigned long end)
+static int __init pch_lpc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
{
struct acpi_madt_lpc_pic *pchlpc_entry = (struct acpi_madt_lpc_pic *)header;

return pch_lpc_acpi_init(pch_pic_priv[0]->pic_domain, pchlpc_entry);
}

-static int __init acpi_cascade_irqdomain_init(void)
+static void __init acpi_cascade_irqdomain_init(void)
{
- acpi_table_parse_madt(ACPI_MADT_TYPE_LPC_PIC,
- pch_lpc_parse_madt, 0);
- return 0;
+ acpi_table_parse_madt(ACPI_MADT_TYPE_LPC_PIC, pch_lpc_parse_madt, 0);
}

int __init pch_pic_acpi_init(struct irq_domain *parent,
--
2.31.1


2022-10-20 12:12:20

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] irqchip/loongarch: Adjust acpi_cascade_irqdomain_init() and sub-routines

On Thu, 20 Oct 2022 08:33:46 +0100,
Huacai Chen <[email protected]> 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 <[email protected]>
> ---
> 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.

2022-10-20 13:41:06

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] irqchip/loongarch: Adjust acpi_cascade_irqdomain_init() and sub-routines

On Thu, Oct 20, 2022 at 7:21 PM Marc Zyngier <[email protected]> wrote:
>
> On Thu, 20 Oct 2022 08:33:46 +0100,
> Huacai Chen <[email protected]> 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 <[email protected]>
> > ---
> > 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.
Oh, I'm sorry, I have misunderstood your reply in another thread. :(

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