On Mon, Jun 05, 2023 at 04:24:10PM +0100, Robin Murphy wrote: > On 2023-06-05 11:35, Sudeep Holla wrote: > > Move all of the ARM-specific initialization into one function namely > > acpi_arm_init(), so it is not necessary to modify/update bus.c every > > time a new piece of it is added. > > > > Cc: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> > > Cc: Hanjun Guo <guohanjun@xxxxxxxxxx> > > Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx> > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > Link: https://lore.kernel.org/r/CAJZ5v0iBZRZmV_oU+VurqxnVMbFN_ttqrL=cLh0sUH+=u0PYsw@xxxxxxxxxxxxxx > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx> > > --- > > drivers/acpi/arm64/Makefile | 2 +- > > drivers/acpi/arm64/agdi.c | 2 +- > > drivers/acpi/arm64/apmt.c | 2 +- > > drivers/acpi/arm64/init.c | 10 ++++++++++ > > drivers/acpi/arm64/init.h | 20 ++++++++++++++++++++ > > drivers/acpi/arm64/iort.c | 1 + > > drivers/acpi/bus.c | 7 +------ > > include/linux/acpi.h | 6 ++++++ > > include/linux/acpi_agdi.h | 13 ------------- > > include/linux/acpi_apmt.h | 19 ------------------- > > include/linux/acpi_iort.h | 2 -- > > 11 files changed, 41 insertions(+), 43 deletions(-) > > create mode 100644 drivers/acpi/arm64/init.c > > create mode 100644 drivers/acpi/arm64/init.h > > delete mode 100644 include/linux/acpi_agdi.h > > delete mode 100644 include/linux/acpi_apmt.h > > > > diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile > > index e21a9e84e394..f81fe24894b2 100644 > > --- a/drivers/acpi/arm64/Makefile > > +++ b/drivers/acpi/arm64/Makefile > > @@ -3,4 +3,4 @@ obj-$(CONFIG_ACPI_AGDI) += agdi.o > > obj-$(CONFIG_ACPI_IORT) += iort.o > > obj-$(CONFIG_ACPI_GTDT) += gtdt.o > > obj-$(CONFIG_ACPI_APMT) += apmt.o > > -obj-y += dma.o > > +obj-y += dma.o init.o > > diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c > > index f605302395c3..8b3c7d42b41b 100644 > > --- a/drivers/acpi/arm64/agdi.c > > +++ b/drivers/acpi/arm64/agdi.c > > @@ -9,11 +9,11 @@ > > #define pr_fmt(fmt) "ACPI: AGDI: " fmt > > #include <linux/acpi.h> > > -#include <linux/acpi_agdi.h> > > #include <linux/arm_sdei.h> > > #include <linux/io.h> > > #include <linux/kernel.h> > > #include <linux/platform_device.h> > > +#include "init.h" > > struct agdi_data { > > int sdei_event; > > diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c > > index 8cab69fa5d59..e5c3bc99fc79 100644 > > --- a/drivers/acpi/arm64/apmt.c > > +++ b/drivers/acpi/arm64/apmt.c > > @@ -10,10 +10,10 @@ > > #define pr_fmt(fmt) "ACPI: APMT: " fmt > > #include <linux/acpi.h> > > -#include <linux/acpi_apmt.h> > > #include <linux/init.h> > > #include <linux/kernel.h> > > #include <linux/platform_device.h> > > +#include "init.h" > > #define DEV_NAME "arm-cs-arch-pmu" > > diff --git a/drivers/acpi/arm64/init.c b/drivers/acpi/arm64/init.c > > new file mode 100644 > > index 000000000000..b4f6ba1c8ef1 > > --- /dev/null > > +++ b/drivers/acpi/arm64/init.c > > @@ -0,0 +1,10 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +#include <linux/acpi.h> > > +#include "init.h" > > + > > +void __init acpi_arm_init(void) > > +{ > > + acpi_agdi_init(); > > + acpi_apmt_init(); > > + acpi_iort_init(); > > +} > > diff --git a/drivers/acpi/arm64/init.h b/drivers/acpi/arm64/init.h > > new file mode 100644 > > index 000000000000..85b0541ce3cc > > --- /dev/null > > +++ b/drivers/acpi/arm64/init.h > > @@ -0,0 +1,20 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +#include <linux/init.h> > > + > > +#ifdef CONFIG_ACPI_AGDI > > +void acpi_agdi_init(void); > > +#else > > +static inline void acpi_agdi_init(void) { } > > +#endif /* CONFIG_ACPI_AGDI */ > > Hmm, I wonder if it might be any nicer to make the declarations > unconditional and guard the calls in "if (IS_ENABLED(...))" instead. No > particular preference, just musing - either way this looks like a sensible > refactor, so FWIW, > Agreed, I was not so happy but the thought of using IS_ENABLED didn't come to me. Thanks for that, will update it. > Reviewed-by: Robin Murphy <robin.murphy@xxxxxxx> > Thanks. -- Regards, Sudeep
