* Russell King - ARM Linux <[email protected]> [120205 09:27]:
> On Sun, Feb 05, 2012 at 09:29:25AM -0800, Tony Lindgren wrote:
> > * Russell King - ARM Linux <[email protected]> [120205 04:28]:
> > > In any case, here's my current (tested) patch unbreaking OMAP as a whole,
> > > not only for all these section mismatches but the more fundamental issues
> > > like the broken serial ports on OMAP3 and the irq domain buggeration too.
> > >
> > > This leaves one section mismatch for me in the OMAP hotplug code.
> >
> > OK great all the section mismatch warning fixes look correct to me
> > except one. The ones that make things __init should be a separate
> > clean-up patch for the next merge window.
>
> Err. This stuff _really_ isn't merge window stuff. It's -rc stuff. Why?
>
> If there's the possibility that stuff in the .init sections could be
> called after it has been discarded (which is basically what the
> section mismatch warnings are telling you) there is the potential for
> OOPSing the kernel.
>
> They are _bug_ fixes.
Of course if that's the case.
> So, we have a non-__init function calling an __init function which will
> be discarded at runtime and the memory associated with omap2_hsmmc_init()
> poisoned.
>
> Now, the question is, can this function be called at runtime? Well,
> this is platform data for the TWL4030 GPIO platform device, and the
> TWL4030 GPIO platform driver is a loadable module:
>
> config GPIO_TWL4030
> tristate "TWL4030, TWL5030, and TPS659x0 GPIOs"
>
> So, it can be built as a loadable module, and then loaded into the
> kernel _after_ the __init code has been discarded. When that happens
> on the 3430SDP, the .setup function will be called, and therefore the
> discarded omap2_hsmmc_init() will also be called.
>
> Therefore omap2_hsmmc_init() and its called functions _must_ _not_ be
> marked __init - or 3430SDP needs to be fixed so that HSMMC is not
> dependent on TWL4030.
>
> But, as long as the code is structured in this way, the HSMMC code
> _must_ lose its __init attributes.
>
> What I suggest is that these changes get applied as is for -rc, fixing
> the OOPS potential of the current situation. Then, for the merge window,
> a proper solution to the 'omap2_hsmmc_init() might be called after init
> time' problem gets merged and then these functions can go back to being
> __init marked.
>
> > Does making sdp3430_twl_gpio_setup() into __init fix those warnings
> > for you? That should be safe as omap3430_i2c_init() is __init.
>
> See above why that's a very very wrong solution.
Argh. Yes you're right, the card change detect GPIOs on I2C cause the
nasty issue here if twl is a module. How horrible.
> > All the omap_mux_init_* functions should also __init. Again, there's
> > something wrong with the calling function if the caller is not __init.
>
> I disagree.
>
> Unfortunately, you have code which is not __init only which calls them.
> As I've already proven above, for example, hsmmc stuff must not be marked
> __init given the current structure of OMAP code. Because hsmmc calls
> into the OMAP mux stuff (specifically omap_mux_init_signal()) it too
> can't be marked __init.
>
> So, for now the __init stuff must go, until the bigger problem of
> why omap2_hsmmc_init() can get called from non-init contexts.
Argh. Yes right you are. We need to fix this properly too though, this
is only a short term solution.
> > > --- a/arch/arm/mach-omap2/pm34xx.c
> > > +++ b/arch/arm/mach-omap2/pm34xx.c
> > > @@ -420,7 +420,7 @@ static void omap3_pm_idle(void)
> > > {
> > > local_fiq_disable();
> > >
> > > - if (omap_irq_pending())
> > > + if (omap_irq_pending() || 1)
> > > goto out;
> > >
> > > trace_power_start(POWER_CSTATE, 1, smp_processor_id());
> >
> > This does not look right to me. I thought reverting of the serial
> > patches should have already solved the issue you're seeing with
> > slow serial port?
> >
> > Those are the reverting commits drivers/tty/serial/serial-omap.c:
> >
> > 8a74e9ffd97dc9de063de8c02ae32db79dd60436 (Revert "tty: serial: OMAP: ensure
> > FIFO levels are set correctly in non-DMA mode")
> >
> > af681cad3f79ad8f7bd6cb170b70990aeef74233 (Revert "tty: serial: OMAP:
> > transmit
> > FIFO threshold interrupts don't wake the chip")
>
> These commits have absolutely nothing to do with it. I pointed out the
> bad commit in one of my emails:
>
> commit 2fd149645eb46d26130d7070c6de037dddf34880
> Author: Govindraj.R <[email protected]>
> Date: Wed Nov 9 17:41:21 2011 +0530
>
> ARM: OMAP2+: UART: Remove omap_uart_can_sleep and add pm_qos
>
> Omap_uart_can_sleep function blocks system wide low power state until
> uart is active remove this func and add qos requests to prevent
> MPU from transitioning.
>
> Keep qos request to default value which will allow MPU to transition
> and while uart baud rate is available calculate the latency value
> from the baudrate and use the same to hold constraint while uart clocks
> are enabled, and if uart is auto-idled the constraint is updated with
> default constraint value allowing MPU to transition.
>
> Qos requests are blocking notifier calls so put these requests to
> work queue, also the driver uses irq_safe version of runtime API's
> and callbacks can be called in interrupt disabled context.
> So to avoid warn on slow path warning while using qos update
> API's from runtime callbacks use the qos_work_queue.
>
> During bootup the runtime_resume call backs might not be called and
> runtime
> callback gets called only after uart is idled by setting the autosuspend
> timeout. So qos_request from runtime resume callback might not activated
> during
> boot if uart baudrate is calculated during bootup for console uart, so
> schedule
> the qos_work queue once we calc_latency while configuring the uart port.
>
> Flush and complete any pending qos jobs in work queue while suspending.
>
> Signed-off-by: Govindraj.R <[email protected]>
> Acked-by: Greg Kroah-Hartman <[email protected]> (for drivers/tty changes)
> Signed-off-by: Kevin Hilman <[email protected]>
>
> Basically, it looks like the OMAP 3 UART is not delivering transmit IRQs
> while in some of the deeper low power modes.
>
> I tried reverting the rest of the patches between this one and HEAD for
> omap-serial.c, but they have no effect what so ever on this bug. As I
> said in one of my emails in this thread, the above commit can't be
> trivially reverted because some other stuff that the code relied upon
> has vanished.
>
> So, the above along with the other part in arch/arm/mach-omap2/cpuidle34xx.c
> is the smallest 'fix' I could find of resolving the regression.
OK, thanks, that should be enough info for let Kevin take a look at this.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
