Re: [Xen-devel] [PATCH RFC 17/35] pl011: Initialize serial from ACPI SPCR table

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2015-02-05 at 15:27 +0000, Stefano Stabellini wrote:
> On Wed, 4 Feb 2015, parth.dixit@xxxxxxxxxx wrote:
> > From: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> > 
> > Parse ACPI SPCR (Serial Port Console Redirection table) table and
> > initialize the serial port pl011.
> > 
> > Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> > Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/setup.c      |  6 +++++
> >  xen/drivers/char/pl011.c  | 69 
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  xen/include/acpi/actbl2.h |  6 +++++
> >  xen/include/xen/serial.h  |  1 +
> >  4 files changed, 82 insertions(+)
> > 
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index af9f429..317b985 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -762,7 +762,13 @@ void __init start_xen(unsigned long boot_phys_offset,
> >  
> >      init_IRQ();
> >  
> > +    /* If ACPI enabled and ARM64 arch then UART initialization from SPCR 
> > table */
> > +#if defined(CONFIG_ACPI) && defined(CONFIG_ARM_64)
> > +    acpi_uart_init();
> > +#else
> >      dt_uart_init();
> > +#endif
> 
> This is bad.  We should have a single uart_init function, that calls
> acpi_uart_init #ifdef(CONFIG_ACPI) and/or simply if(!acpi_disabled).

The latter, please.

And yes I agree: all these foo_init's should be refactored to have
generic and dt parts and then have the acpi stuff inserted in the
appropriate place (ideally in at least two patches, refactor then add
ACPI). It should not be done by making each caller have a conditional on
acpi support as is being done in this series. i.e smth like:

  uart_init()
  {
     [ early generic setup ]

     if (acpi_enabled)
         acpi_uart_init()
     else
         dt_uart_init()

     [ late generic setup]
  }

with start_xen calling uart_init.

The same is true of the timers, interrupt controllers, etc.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel