Re: [PATCH v2] ACPI: bus: Consolidate all arm specific initialisation into acpi_arm_init()

[Sudeep Holla <sudeep.holla@arm.com>]

On Mon, Oct 09, 2023 at 08:29:49PM +0800, Hanjun Guo wrote:
> On 2023/10/7 8:11, D Scott Phillips wrote:
> > Sudeep Holla <sudeep.holla@arm.com> writes:
> > 
> > > 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@kernel.org>
> > > Cc: Rafael J. Wysocki <rafael@kernel.org>
> > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> > > Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
> > > Link: https://lore.kernel.org/r/CAJZ5v0iBZRZmV_oU+VurqxnVMbFN_ttqrL=cLh0sUH+=u0PYsw@mail.gmail.com
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > >   drivers/acpi/arm64/Makefile |  2 +-
> > >   drivers/acpi/arm64/agdi.c   |  2 +-
> > >   drivers/acpi/arm64/apmt.c   |  2 +-
> > >   drivers/acpi/arm64/init.c   | 13 +++++++++++++
> > >   drivers/acpi/arm64/init.h   |  6 ++++++
> > >   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, 30 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

[...]

> > > @@ -1408,7 +1405,7 @@ static int __init acpi_init(void)
> > >   	acpi_init_ffh();
> > > 
> > >   	pci_mmcfg_late_init();
> > > -	acpi_iort_init();
> > > +	acpi_arm_init();
> > >   	acpi_viot_early_init();
> > >   	acpi_hest_init();
> > >   	acpi_ghes_init();
> > > @@ -1420,8 +1417,6 @@ static int __init acpi_init(void)
> > >   	acpi_debugger_init();
> > >   	acpi_setup_sb_notify_handler();
> > >   	acpi_viot_init();
> > > -	acpi_agdi_init();
> > > -	acpi_apmt_init();
> > 
> > Hi Sudeep, this moves acpi_agdi_init() before acpi_ghes_init().
> > sdei initialization currently happens from ghes_init, so agdi devices
> > using SDEI can no longer probe:
> > 
> > | [    0.515864] sdei: Failed to create event 1073741825: -5
> > | [    0.515866] agdi agdi.0: Failed to register for SDEI event 1073741825
> > | [    0.515867] agdi: probe of agdi.0 failed with error -5
> > | ...
> > | [    0.516022] sdei: SDEIv1.0 (0x0) detected in firmware.
>

Sorry for that.

> How about just move acpi_arm_init() to the place of after
> acpi_ghes_init()?
>

Yes that could work but  I am not comfortable with such play around with
the ordering. I don't have better alternative as well :( so I am OK with
the proposal.

We had all sorts of issues with initcalls in the past but I understand
this one is not as bad as that, I am just cautious.

> I checked the functions in acpi_arm_init(), there are no other
> dependencies except sdei_init().
>

Fair enough.

Anyways, for now Hanjun's suggestion should work.

Can either one of you write a patch and post ? Or you use want me to do
that ?

--
Regards,
Sudeep

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