On Wednesday 10 July 2013 07:56 PM, Tony Lindgren wrote:
> * Kevin Hilman <khilman@xxxxxxxxxx> [130710 01:29]:
>> Felipe Balbi <balbi@xxxxxx> writes:
>>>>
>>>> Right, but calling serial_omap_restore_context() even when the context
>>>> is not lost, should not ideally cause an issue.
>>>
>>> it does in one condition. If context hasn't been saved before. And that
>>> can happen in the case of wrong pm runtime status for that device.
>>>
>>> Imagine the device is marked as suspended even though it's fully enabled
>>> (it hasn't been suspended by hwmod due to NO_IDLE flag). In that case
>>> your context structure is all zeroes (context has never been saved
>>> before) then when you call pm_runtime_get_sync() on probe() your
>>> ->runtime_resume() will get called, which will restore context,
>>> essentially undoing anything which was configured by u-boot.
>>>
>>> Am I missing something ?
>>
>> You're right, the _set_active() is crucial in the case when we prevent
>> the console UART from idling during boot (though that shouldn't be
>> happening in mainline unless the fix for "Issue 1" is done.)
>
> Felipe is right, looks like all we need is to check if context is
> initialized or not. So no need for mach-omap2/serial.c or hwmod tinkering.
>
> After that having DEBUG_LL and cmdline with earlyprintk console=ttyO..
> works for me.
On what platform? Like I said there are flags set statically in hmwod
data for OMAP4 and OMAP5, which make it work without any hwmod tinkering
in mach-omap2/serial.c. But it won't work for am33xx.
We could also check for some combination of the context
> save registers being NULL if somebody has a good idea which ones
> should never be 0.
>
> Regards,
>
> Tony
>
>
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -161,6 +161,7 @@ struct uart_omap_port {
> u32 calc_latency;
> struct work_struct qos_work;
> struct pinctrl *pins;
> + bool initialized;
> bool is_suspending;
> };
>
> @@ -1523,6 +1524,8 @@ static int serial_omap_probe(struct platform_device *pdev)
>
> pm_runtime_mark_last_busy(up->dev);
> pm_runtime_put_autosuspend(up->dev);
> + up->initialized = true;
This won't help as the context save for serial happens as part of set_termios() and not
when the device is probed.
> +
> return 0;
>
> err_add_port:
> @@ -1584,6 +1587,9 @@ static void serial_omap_mdr1_errataset(struct uart_omap_port *up, u8 mdr1)
> #ifdef CONFIG_PM_RUNTIME
> static void serial_omap_restore_context(struct uart_omap_port *up)
> {
> + if (!up->initialized)
> + return;
> +
> if (up->errata & UART_ERRATA_i202_MDR1_ACCESS)
> serial_omap_mdr1_errataset(up, UART_OMAP_MDR1_DISABLE);
> else
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
