[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.