[PATCH] Move serial_dev_init to device_initcall()

[PATCH] Move serial_dev_init to device_initcall()

[Olof Johansson]

With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
initcall is now called before I/O space is setup, but it's dependent on
it being available.

Since there's no way to make dependencies between initcalls, we'll just
have to move it to device_initcall(). Yes, it's suboptimal but I'm not
aware of any better solution at this time.


Signed-off-by: Olof Johansson <olof@lixom.net>

---

I think I might have one of the few hardware platforms with UART on PIO,
at least if noone else has already hit this. It was introduced by BenH's
rewrite of the I/O space allocation earlier, and the problem is that by
the time serial_dev_init() runs, the hose io_base_virt isn't available.

I still see the platform devices register before the PCI ones, so it
looks like init order is undisturbed by this change. Still, it'd be good
to hear test feedback from other platforms as needed. It's uncertain what
platforms the comments above serial_dev_init() about needing overrides
applies for, so I don't know which ones might be sensitive to call order
changes.


Index: mainline/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- mainline.orig/arch/powerpc/kernel/legacy_serial.c
+++ mainline/arch/powerpc/kernel/legacy_serial.c
@@ -493,7 +493,7 @@ static int __init serial_dev_init(void)
 
 	return platform_device_register(&serial_device);
 }
-arch_initcall(serial_dev_init);
+device_initcall(serial_dev_init);
 
 
 /*

Re: [PATCH] Move serial_dev_init to device_initcall()

[Guennadi Liakhovetski]

On Wed, 22 Aug 2007, Olof Johansson wrote:

> With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
> initcall is now called before I/O space is setup, but it's dependent on
> it being available.
> 
> Since there's no way to make dependencies between initcalls, we'll just
> have to move it to device_initcall(). Yes, it's suboptimal but I'm not
> aware of any better solution at this time.

Do I understand it right, that with this change all UARTs, controlled by 
legacy_serial will be initialized later, and that for example console 
output will be first possible later? Maybe, if there is really no other 
possibility for I/O space devices, we could have both calls

arch_initcall(serial_mem_dev_init);
device_initcall(serial_io_dev_init);

so, that at least MEMIO based UARTs could still initialize as before?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Move serial_dev_init to device_initcall()

[Olof Johansson]

On Fri, Aug 24, 2007 at 01:21:57AM +0200, Guennadi Liakhovetski wrote:
> On Wed, 22 Aug 2007, Olof Johansson wrote:
> 
> > With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
> > initcall is now called before I/O space is setup, but it's dependent on
> > it being available.
> > 
> > Since there's no way to make dependencies between initcalls, we'll just
> > have to move it to device_initcall(). Yes, it's suboptimal but I'm not
> > aware of any better solution at this time.
> 
> Do I understand it right, that with this change all UARTs, controlled by 
> legacy_serial will be initialized later, and that for example console 
> output will be first possible later?

Yes, unfortunately. Unless they've got a udbg driver, since that
would give console output during early boot anyway (even without using
EARLY_DEBUG).

> Maybe, if there is really no other 
> possibility for I/O space devices, we could have both calls
>
> arch_initcall(serial_mem_dev_init);
> device_initcall(serial_io_dev_init);
> 
> so, that at least MEMIO based UARTs could still initialize as before?

That's quite a hack, I hope we can avoid it. Maybe Ben has some suggestion
on how to get the IO setup earlier instead.


-Olof

Re: [PATCH] Move serial_dev_init to device_initcall()

[Benjamin Herrenschmidt]

On Thu, 2007-08-23 at 18:15 -0500, Olof Johansson wrote:
> On Fri, Aug 24, 2007 at 01:21:57AM +0200, Guennadi Liakhovetski wrote:
> > On Wed, 22 Aug 2007, Olof Johansson wrote:
> > 
> > > With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
> > > initcall is now called before I/O space is setup, but it's dependent on
> > > it being available.
> > > 
> > > Since there's no way to make dependencies between initcalls, we'll just
> > > have to move it to device_initcall(). Yes, it's suboptimal but I'm not
> > > aware of any better solution at this time.
> > 
> > Do I understand it right, that with this change all UARTs, controlled by 
> > legacy_serial will be initialized later, and that for example console 
> > output will be first possible later?
> 
> Yes, unfortunately. Unless they've got a udbg driver, since that
> would give console output during early boot anyway (even without using
> EARLY_DEBUG).

Legacy ports should get udbg first.

> > Maybe, if there is really no other 
> > possibility for I/O space devices, we could have both calls
> >
> > arch_initcall(serial_mem_dev_init);
> > device_initcall(serial_io_dev_init);
> > 
> > so, that at least MEMIO based UARTs could still initialize as before?
> 
> That's quite a hack, I hope we can avoid it. Maybe Ben has some suggestion
> on how to get the IO setup earlier instead.

IO space allocation now relies on get_vm_area() which can't be done too
early (not in setup_arch) which is why I allocate all PCI IO space at
PHB scanning time, which is a subsys initcall.

An option would be to do it from some other init call, but that's a bit
ugly. That means PCI would be split into setup_arch() setting up PHBs,
that initcall allocating the IO spaces, and later, the actual scan.

It would be tricky though as my current interface relies on pci_bus
structures. So you would also need to re-split that into a low-level
that works on the PHB and a high level version that works on the bus.

Is this really necessary ?

Ben.

Re: [PATCH] Move serial_dev_init to device_initcall()

[Olof Johansson]

On Fri, Aug 24, 2007 at 06:59:38AM +0200, Benjamin Herrenschmidt wrote:
> On Thu, 2007-08-23 at 18:15 -0500, Olof Johansson wrote:
> > On Fri, Aug 24, 2007 at 01:21:57AM +0200, Guennadi Liakhovetski wrote:
> > > On Wed, 22 Aug 2007, Olof Johansson wrote:
> > > 
> > > > With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
> > > > initcall is now called before I/O space is setup, but it's dependent on
> > > > it being available.
> > > > 
> > > > Since there's no way to make dependencies between initcalls, we'll just
> > > > have to move it to device_initcall(). Yes, it's suboptimal but I'm not
> > > > aware of any better solution at this time.
> > > 
> > > Do I understand it right, that with this change all UARTs, controlled by 
> > > legacy_serial will be initialized later, and that for example console 
> > > output will be first possible later?
> > 
> > Yes, unfortunately. Unless they've got a udbg driver, since that
> > would give console output during early boot anyway (even without using
> > EARLY_DEBUG).
> 
> Legacy ports should get udbg first.
> 
> > > Maybe, if there is really no other 
> > > possibility for I/O space devices, we could have both calls
> > >
> > > arch_initcall(serial_mem_dev_init);
> > > device_initcall(serial_io_dev_init);
> > > 
> > > so, that at least MEMIO based UARTs could still initialize as before?
> > 
> > That's quite a hack, I hope we can avoid it. Maybe Ben has some suggestion
> > on how to get the IO setup earlier instead.
> 
> IO space allocation now relies on get_vm_area() which can't be done too
> early (not in setup_arch) which is why I allocate all PCI IO space at
> PHB scanning time, which is a subsys initcall.
> 
> An option would be to do it from some other init call, but that's a bit
> ugly. That means PCI would be split into setup_arch() setting up PHBs,
> that initcall allocating the IO spaces, and later, the actual scan.
> 
> It would be tricky though as my current interface relies on pci_bus
> structures. So you would also need to re-split that into a low-level
> that works on the PHB and a high level version that works on the bus.
> 
> Is this really necessary ?

No, that's what I'm hoping won't be, i.e. that it'll be fine to just
init serial ports slightly later. If people care about early output they
should use udbg anyway.

It'd be useful if people could at least test the patch on their systems.
I have only a few 32-bit ones with proper uarts, and none of them are
currently in a state where I can just throw a new kernel on them for
testing. :(


-Olof

Re: [PATCH] Move serial_dev_init to device_initcall()

[Olof Johansson]

On Wed, Aug 22, 2007 at 07:26:37PM -0500, Olof Johansson wrote:
> With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
> initcall is now called before I/O space is setup, but it's dependent on
> it being available.
> 
> Since there's no way to make dependencies between initcalls, we'll just
> have to move it to device_initcall(). Yes, it's suboptimal but I'm not
> aware of any better solution at this time.

Paul, could we get this into 2.6.23? It's a regression from before.

Andy Fleming reported no problems on the MMIO-based UART board he tested
with, and Milton said the patch didn't break qemu/prep that has PIO UART
(he hasn't yet tested to see if it's really required there though).


-Olof