mbox series

[v5,00/17] RISC-V: ACPI: Add external interrupt controller support

Message ID 20240501121742.1215792-1-sunilvl@ventanamicro.com
Headers show
Series RISC-V: ACPI: Add external interrupt controller support | expand

Message

Sunil V L May 1, 2024, 12:17 p.m. UTC
This series adds support for the below ECR approved by ASWG.
1) MADT - https://drive.google.com/file/d/1oMGPyOD58JaPgMl1pKasT-VKsIKia7zR/view?usp=sharing

The series primarily enables irqchip drivers for RISC-V ACPI based
platforms.

The series can be broadly categorized like below. 

1) PCI ACPI related functions are migrated from arm64 to common file so
that we don't need to duplicate them for RISC-V.

2) Added support for re-ordering the probe of interrupt controllers when
IRQCHIP_ACPI_DECLARE is used.

3) To ensure probe order between interrupt controllers and devices,
implicit dependency is created similar to when _DEP is present.

4) Added 8250 serial driver for a generic 16550 UART which is enumerated
as ACPI platform device.

5) ACPI support added in RISC-V interrupt controller drivers.

Changes since RFC v4:
	1) Removed RFC tag as the RFCv4 design looked reasonable.
	2) Dropped PCI patch needed to avoid warning when there is no MSI
	   controller. This will be sent later separately after the
	   current series.
	3) Dropped PNP handling of _DEP since there is new ACPI ID for
	   generic 16550 UART. Added the serial driver patch instead.
	4) Rebased to latest linux-next.
	5) Reordered/squashed patches in the series

Changes since RFC v3:
	1) Moved to _DEP method instead of fw_devlink.
	2) PLIC/APLIC driver probe using namespace devices.
	3) Handling PNP devices as part of clearing dependency.
	4) Rebased to latest linux-next to get AIA DT drivers.

Changes since RFC v2:
	1) Introduced fw_devlink for ACPI nodes for IRQ dependency.
	2) Dropped patches in drivers which are not required due to
	   fw_devlink support.
	3) Dropped pci_set_msi() patch and added a patch in
	   pci_create_root_bus().
	4) Updated pnp_irq() patch so that none of the actual PNP
	   drivers need to change.

Changes since RFC v1:
	1) Abandoned swnode approach as per Marc's feedback.
	2) To cope up with AIA series changes which changed irqchip driver
	   probe from core_initcall() to platform_driver, added patches
	   to support deferred probing.
	3) Rebased on top of Anup's AIA v11 and added tags.

To test the series,

1) qemu should be built using the riscv_acpi_namespace_v2 branch at
https://github.com/vlsunil/qemu.git

2) EDK2 should be built using the instructions at:
https://github.com/tianocore/edk2/blob/master/OvmfPkg/RiscVVirt/README.md

NOTE: One should be able to use u-boot as well as per instructions from Björn.
https://lore.kernel.org/lkml/87a5lqsrvh.fsf@all.your.base.are.belong.to.us/

3) Build Linux using this series.

Run Qemu:
qemu-system-riscv64 \
 -M virt,pflash0=pflash0,pflash1=pflash1,aia=aplic-imsic \
 -m 2G -smp 8 \
 -serial mon:stdio \
 -device virtio-gpu-pci -full-screen \
 -device qemu-xhci \
 -device usb-kbd \
 -blockdev node-name=pflash0,driver=file,read-only=on,filename=RISCV_VIRT_CODE.fd \
 -blockdev node-name=pflash1,driver=file,filename=RISCV_VIRT_VARS.fd \
 -netdev user,id=net0 -device virtio-net-pci,netdev=net0 \
 -kernel arch/riscv/boot/Image \
 -initrd rootfs.cpio \
 -append "root=/dev/ram ro console=ttyS0 rootwait earlycon=uart8250,mmio,0x10000000"

To boot with APLIC only, use aia=aplic.
To boot with PLIC, remove aia= option.

This series is also available in acpi_b2_v5 branch at
https://github.com/vlsunil/linux.git

Based-on: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tag/?h=next-20240501

Sunil V L (17):
  arm64: PCI: Migrate ACPI related functions to pci-acpi.c
  ACPI: scan: Add a weak function to reorder the IRQCHIP probe
  ACPI: bus: Add acpi_riscv_init function
  ACPI: scan: Refactor dependency creation
  ACPI: scan: Add RISC-V interrupt controllers to honor list
  ACPI: scan: Define weak function to populate dependencies
  ACPI: bus: Add RINTC IRQ model for RISC-V
  ACPI: pci_link: Clear the dependencies after probe
  ACPI: RISC-V: Implement PCI related functionality
  ACPI: RISC-V: Implement function to reorder irqchip probe entries
  ACPI: RISC-V: Initialize GSI mapping structures
  ACPI: RISC-V: Implement function to add implicit dependencies
  irqchip/riscv-intc: Add ACPI support for AIA
  irqchip/riscv-imsic: Add ACPI support
  irqchip/riscv-aplic: Add ACPI support
  irqchip/sifive-plic: Add ACPI support
  serial: 8250: Add 8250_acpi driver

 arch/arm64/kernel/pci.c                    | 191 ------------
 arch/riscv/Kconfig                         |   3 +
 arch/riscv/configs/defconfig               |   1 +
 arch/riscv/include/asm/irq.h               |  57 ++++
 arch/riscv/kernel/acpi.c                   |  31 +-
 drivers/acpi/Kconfig                       |   3 +
 drivers/acpi/bus.c                         |   4 +
 drivers/acpi/pci_link.c                    |   3 +
 drivers/acpi/riscv/Makefile                |   2 +-
 drivers/acpi/riscv/init.c                  |  14 +
 drivers/acpi/riscv/init.h                  |   4 +
 drivers/acpi/riscv/irq.c                   | 329 +++++++++++++++++++++
 drivers/acpi/scan.c                        |  65 ++--
 drivers/irqchip/irq-riscv-aplic-direct.c   |  20 +-
 drivers/irqchip/irq-riscv-aplic-main.c     |  70 +++--
 drivers/irqchip/irq-riscv-aplic-main.h     |   1 +
 drivers/irqchip/irq-riscv-aplic-msi.c      |   9 +-
 drivers/irqchip/irq-riscv-imsic-early.c    |  52 +++-
 drivers/irqchip/irq-riscv-imsic-platform.c |  32 +-
 drivers/irqchip/irq-riscv-imsic-state.c    | 115 ++++---
 drivers/irqchip/irq-riscv-imsic-state.h    |   2 +-
 drivers/irqchip/irq-riscv-intc.c           | 102 ++++++-
 drivers/irqchip/irq-sifive-plic.c          |  89 ++++--
 drivers/pci/pci-acpi.c                     | 182 ++++++++++++
 drivers/tty/serial/8250/8250_acpi.c        |  96 ++++++
 drivers/tty/serial/8250/Kconfig            |   8 +
 drivers/tty/serial/8250/Makefile           |   1 +
 include/acpi/acpi_bus.h                    |   2 +
 include/linux/acpi.h                       |   9 +
 include/linux/irqchip/riscv-imsic.h        |  10 +
 30 files changed, 1155 insertions(+), 352 deletions(-)
 create mode 100644 drivers/acpi/riscv/init.c
 create mode 100644 drivers/acpi/riscv/init.h
 create mode 100644 drivers/acpi/riscv/irq.c
 create mode 100644 drivers/tty/serial/8250/8250_acpi.c

Comments

Will Deacon May 1, 2024, 12:59 p.m. UTC | #1
On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, move these functions
> to common location.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  arch/arm64/kernel/pci.c | 191 ----------------------------------------
>  drivers/pci/pci-acpi.c  | 182 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 182 insertions(+), 191 deletions(-)

Looks like a straight-forward move of the code, so:

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

Will
Andy Shevchenko May 2, 2024, 9:17 a.m. UTC | #2
On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> RISC-V has non-PNP generic 16550A compatible UART which needs to be
> enumerated as ACPI platform device. Add driver support for such devices
> similar to 8250_of.

...

> + * This driver is for generic 16550 compatible UART enumerated via ACPI
> + * platform bus instead of PNP bus like PNP0501. This is not a full

This has to be told in the commit message. Anyway, we don't need a duplication
code, please use 8250_pnp.

...

> +	{ "RSCV0003", 0 },

Does it have _CID to be PNP0501?
If not, add this ID to the 8250_pnp.

...

P.S.
The code you submitted has a lot of small style issues, I can comment on them
if you want, but as I said this code is not needed at all.
Andy Shevchenko May 2, 2024, 9:22 a.m. UTC | #3
On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> The functions defined in arm64 for ACPI support are required
> for RISC-V also. To avoid duplication, move these functions
> to common location.

...

There are -M -C parameters to git format-patch. Use them in the next version of
the series. (Note, you may add percentage numbers to each of those parameters
to get prettier result.)
Sunil V L May 2, 2024, 9:50 a.m. UTC | #4
On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> > RISC-V has non-PNP generic 16550A compatible UART which needs to be
> > enumerated as ACPI platform device. Add driver support for such devices
> > similar to 8250_of.
> 
> ...
> 
> > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > + * platform bus instead of PNP bus like PNP0501. This is not a full
> 
> This has to be told in the commit message. Anyway, we don't need a duplication
> code, please use 8250_pnp.
> 
Hi Andy,

Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
in a different way which causes issue to get _DEP to work.
pnpacpi_init() creates PNP data structures which gets skipped if the
UART puts _DEP on the GSI provider (interrupt controller). In that case,
we need to somehow reinitialize such PNP devices after interrupt
controller gets probed. I tried a solution [1] but it required several
functions to be moved out of __init. 

This driver is not a duplicate of 8250_pnp. It just relies on UART
enumerated as platform device instead of using PNP interfaces.
Isn't it better and simple to have an option to enumerate as platform
device instead of PNP? 

[1] - https://patchwork.kernel.org/project/linux-pci/patch/20240415170113.662318-14-sunilvl@ventanamicro.com/

Thanks,
Sunil
> ...
> 
> > +	{ "RSCV0003", 0 },
> 
> Does it have _CID to be PNP0501?
> If not, add this ID to the 8250_pnp.
> 
> ...
> 
> P.S.
> The code you submitted has a lot of small style issues, I can comment on them
> if you want, but as I said this code is not needed at all.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
Sunil V L May 2, 2024, 9:56 a.m. UTC | #5
On Thu, May 02, 2024 at 12:22:28PM +0300, Andy Shevchenko wrote:
> On Wed, May 01, 2024 at 05:47:26PM +0530, Sunil V L wrote:
> > The functions defined in arm64 for ACPI support are required
> > for RISC-V also. To avoid duplication, move these functions
> > to common location.
> 
> ...
> 
> There are -M -C parameters to git format-patch. Use them in the next version of
> the series. (Note, you may add percentage numbers to each of those parameters
> to get prettier result.)
> 
Yeah, I tried different combination of percentage after you told me last
time. I just didn't see any difference in the patch generated. Let me
try again while sending next version. Thanks!
Andy Shevchenko May 2, 2024, 10:09 a.m. UTC | #6
On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> > On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:

...

> > > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > > + * platform bus instead of PNP bus like PNP0501. This is not a full
> > 
> > This has to be told in the commit message. Anyway, we don't need a duplication
> > code, please use 8250_pnp.
> 
> Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
> in a different way which causes issue to get _DEP to work.
> pnpacpi_init() creates PNP data structures which gets skipped if the
> UART puts _DEP on the GSI provider (interrupt controller). In that case,
> we need to somehow reinitialize such PNP devices after interrupt
> controller gets probed.

Then fix that code, we don't want a hack driver on top of the existing one for
the same.

What I might think out of head is that used IRQ core for your case should
return a deferred probe error code when it's not ready, then 8250_pnp will
get reprobed.

> I tried a solution [1] but it required several
> functions to be moved out of __init. 

> This driver is not a duplicate of 8250_pnp. It just relies on UART
> enumerated as platform device instead of using PNP interfaces.
> Isn't it better and simple to have an option to enumerate as platform
> device instead of PNP? 

Ah, then extract platform driver first from 8250_core.c.

> [1] - https://patchwork.kernel.org/project/linux-pci/patch/20240415170113.662318-14-sunilvl@ventanamicro.com/
Andy Shevchenko May 2, 2024, 10:19 a.m. UTC | #7
On Thu, May 02, 2024 at 11:12:43AM +0100, Sudeep Holla wrote:
> On Thu, May 02, 2024 at 03:32:25PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> > > On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > > > Add a new function for RISC-V to do any architecture specific
> > > > initialization. This function will be used to create platform devices
> > > > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
> > >
> > > What is the special about this architecture that it requires a separate
> > > initialization that is _not_ going to be in other cases?
> > > Please, elaborate.
> > >
> > This init function will be used to create GSI mapping structures and in
> > future may be others like iommu. Like I mentioned, ARM already has
> > similar function acpi_arm_init(). So, it is not new right?
> 
> Just to add:
> 
> This is to initialise everything around all the arch specific tables
> which you will not have on any other architectures. We could execute
> on all architectures but the tables will never be found. The main point
> is why do we want to do that if we can optimise and skip on all other
> archs.

You need to carefully write the commit messages. Some kind of the above
paragraphs has to be in there.
Sunil V L May 2, 2024, 11 a.m. UTC | #8
On Thu, May 02, 2024 at 01:19:55PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 11:12:43AM +0100, Sudeep Holla wrote:
> > On Thu, May 02, 2024 at 03:32:25PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 12:24:14PM +0300, Andy Shevchenko wrote:
> > > > On Wed, May 01, 2024 at 05:47:28PM +0530, Sunil V L wrote:
> > > > > Add a new function for RISC-V to do any architecture specific
> > > > > initialization. This function will be used to create platform devices
> > > > > like APLIC, PLIC, RISC-V IOMMU etc. This is similar to acpi_arm_init().
> > > >
> > > > What is the special about this architecture that it requires a separate
> > > > initialization that is _not_ going to be in other cases?
> > > > Please, elaborate.
> > > >
> > > This init function will be used to create GSI mapping structures and in
> > > future may be others like iommu. Like I mentioned, ARM already has
> > > similar function acpi_arm_init(). So, it is not new right?
> > 
> > Just to add:
> > 
> > This is to initialise everything around all the arch specific tables
> > which you will not have on any other architectures. We could execute
> > on all architectures but the tables will never be found. The main point
> > is why do we want to do that if we can optimise and skip on all other
> > archs.
> 
> You need to carefully write the commit messages. Some kind of the above
> paragraphs has to be in there.
> 
Sure, let me update the commit message on similar lines.

Thanks,
Sunil
Sunil V L May 2, 2024, 11:20 a.m. UTC | #9
On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> > > On Wed, May 01, 2024 at 05:47:42PM +0530, Sunil V L wrote:
> 
> ...
> 
> > > > + * This driver is for generic 16550 compatible UART enumerated via ACPI
> > > > + * platform bus instead of PNP bus like PNP0501. This is not a full
> > > 
> > > This has to be told in the commit message. Anyway, we don't need a duplication
> > > code, please use 8250_pnp.
> > 
> > Thank you for the review!. Major issue with PNP0501 is, it gets enumerated
> > in a different way which causes issue to get _DEP to work.
> > pnpacpi_init() creates PNP data structures which gets skipped if the
> > UART puts _DEP on the GSI provider (interrupt controller). In that case,
> > we need to somehow reinitialize such PNP devices after interrupt
> > controller gets probed.
> 
> Then fix that code, we don't want a hack driver on top of the existing one for
> the same.
> 
> What I might think out of head is that used IRQ core for your case should
> return a deferred probe error code when it's not ready, then 8250_pnp will
> get reprobed.
> 
Deferred probe was ruled out in prior discussion. Also, deferred probe
will not work with _DEP approach. The reason is, PNP devices itself are
not getting created from the ACPI name space when they have _DEP. Hence,
serial_pnp_probe() will not be called at all.

> > I tried a solution [1] but it required several
> > functions to be moved out of __init. 
> 
> > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > enumerated as platform device instead of using PNP interfaces.
> > Isn't it better and simple to have an option to enumerate as platform
> > device instead of PNP? 
> 
> Ah, then extract platform driver first from 8250_core.c.
> 
Let me know if I understand your suggestion correctly. Do you mean call
something like serial8250_acpi_init() from serial8250_init() and
register the driver directly in serial8250_acpi_init()?

Thanks,
Sunil
Andy Shevchenko May 2, 2024, 3:35 p.m. UTC | #10
On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:

...

> > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > enumerated as platform device instead of using PNP interfaces.
> > > Isn't it better and simple to have an option to enumerate as platform
> > > device instead of PNP? 
> > 
> > Ah, then extract platform driver first from 8250_core.c.
> > 
> Let me know if I understand your suggestion correctly. Do you mean call
> something like serial8250_acpi_init() from serial8250_init() and
> register the driver directly in serial8250_acpi_init()?

Extract the code to be 8250_platform.c and update that file.
I have locally the extraction of RSA code, I will see if I can help you
with the rest.
Sunil V L May 3, 2024, 1:59 p.m. UTC | #11
On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > enumerated as platform device instead of using PNP interfaces.
> > > > Isn't it better and simple to have an option to enumerate as platform
> > > > device instead of PNP? 
> > > 
> > > Ah, then extract platform driver first from 8250_core.c.
> > > 
> > Let me know if I understand your suggestion correctly. Do you mean call
> > something like serial8250_acpi_init() from serial8250_init() and
> > register the driver directly in serial8250_acpi_init()?
> 
> Extract the code to be 8250_platform.c and update that file.
> I have locally the extraction of RSA code, I will see if I can help you
> with the rest.
> 
Thanks!. That will be helpful. TBH, I don't understand what to do for
extracting the platform driver code. There are already several vendor
specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
devices. 8250_core.c looks cleanly supporting such drivers which can
register themselves with the core. For generic UART, DT has 8250_of.c
and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
contract. So, the driver in this patch is similar to vendor specific
drivers to support generic uart devices which are enumerated as platform
device. I can rename 8250_acpi.c to 8250_platform.c if that is better.

Could you please help with a patch even if not compiled so that I can
understand your suggestion better? 

Thanks for your help!
Sunil
Andy Shevchenko May 3, 2024, 3:32 p.m. UTC | #12
On Fri, May 03, 2024 at 07:29:35PM +0530, Sunil V L wrote:
> On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> > On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:

...

> > > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > > enumerated as platform device instead of using PNP interfaces.
> > > > > Isn't it better and simple to have an option to enumerate as platform
> > > > > device instead of PNP? 
> > > > 
> > > > Ah, then extract platform driver first from 8250_core.c.
> > > > 
> > > Let me know if I understand your suggestion correctly. Do you mean call
> > > something like serial8250_acpi_init() from serial8250_init() and
> > > register the driver directly in serial8250_acpi_init()?
> > 
> > Extract the code to be 8250_platform.c and update that file.
> > I have locally the extraction of RSA code, I will see if I can help you
> > with the rest.
> > 
> Thanks!. That will be helpful. TBH, I don't understand what to do for
> extracting the platform driver code. There are already several vendor
> specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
> devices. 8250_core.c looks cleanly supporting such drivers which can
> register themselves with the core. For generic UART, DT has 8250_of.c
> and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
> contract. So, the driver in this patch is similar to vendor specific
> drivers to support generic uart devices which are enumerated as platform
> device.

> I can rename 8250_acpi.c to 8250_platform.c if that is better.

No, that's not what I meant. We _already_ have a generic platform driver,
it's just inline in the 8250_core.c and needs to be extracted. When it's done,
you may simply add an ACPI table to it.

> Could you please help with a patch even if not compiled so that I can
> understand your suggestion better?
Sunil V L May 6, 2024, 11:45 a.m. UTC | #13
On Fri, May 03, 2024 at 06:32:10PM +0300, Andy Shevchenko wrote:
> On Fri, May 03, 2024 at 07:29:35PM +0530, Sunil V L wrote:
> > On Thu, May 02, 2024 at 06:35:52PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 02, 2024 at 04:50:57PM +0530, Sunil V L wrote:
> > > > On Thu, May 02, 2024 at 01:09:57PM +0300, Andy Shevchenko wrote:
> > > > > On Thu, May 02, 2024 at 03:20:08PM +0530, Sunil V L wrote:
> > > > > > On Thu, May 02, 2024 at 12:17:59PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > > This driver is not a duplicate of 8250_pnp. It just relies on UART
> > > > > > enumerated as platform device instead of using PNP interfaces.
> > > > > > Isn't it better and simple to have an option to enumerate as platform
> > > > > > device instead of PNP? 
> > > > > 
> > > > > Ah, then extract platform driver first from 8250_core.c.
> > > > > 
> > > > Let me know if I understand your suggestion correctly. Do you mean call
> > > > something like serial8250_acpi_init() from serial8250_init() and
> > > > register the driver directly in serial8250_acpi_init()?
> > > 
> > > Extract the code to be 8250_platform.c and update that file.
> > > I have locally the extraction of RSA code, I will see if I can help you
> > > with the rest.
> > > 
> > Thanks!. That will be helpful. TBH, I don't understand what to do for
> > extracting the platform driver code. There are already several vendor
> > specific UART drivers (ex: 8250_fsl.c) which are enumerated as platform
> > devices. 8250_core.c looks cleanly supporting such drivers which can
> > register themselves with the core. For generic UART, DT has 8250_of.c
> > and ACPI has 8250_pnp.c. But 8250_pnp.c comes with baggage of PNP
> > contract. So, the driver in this patch is similar to vendor specific
> > drivers to support generic uart devices which are enumerated as platform
> > device.
> 
> > I can rename 8250_acpi.c to 8250_platform.c if that is better.
> 
> No, that's not what I meant. We _already_ have a generic platform driver,
> it's just inline in the 8250_core.c and needs to be extracted. When it's done,
> you may simply add an ACPI table to it.
> 
> > Could you please help with a patch even if not compiled so that I can
> > understand your suggestion better? 
> 
Okay, Thanks! Andy.

IIUC, you want to extract serial8250_isa_driver from 8250_core and
then add ACPI support. I was not sure about that since I thought it was
only for legacy ISA devices and they seem to be created by OS itself
instead of discovery. ISA driver has some ordering dependency on PNP
driver as well. Anyway, let me try if that is the acceptable way
forward.

Thanks,
Sunil