[PATCH v2 05/12] OMAP: Serial: Hold console lock for console usage.
Govindraj
govindraj.ti at gmail.com
Thu May 5 06:25:17 EDT 2011
On Thu, May 5, 2011 at 2:13 AM, Kevin Hilman <khilman at ti.com> wrote:
> "Govindraj.R" <govindraj.raja at ti.com> writes:
>
>> Acquire console lock before enabling and writing to console-uart
>> to avoid any recursive prints from console write.
>> for ex:
>> --> printk
>> --> uart_console_write
>> --> get_sync
>> --> printk from omap_device enable
>> --> uart_console write
>
> By this time, since the device's runtime PM hooks have been called, the
> device's rutime PM state should be set to RPM_SUSPENDING (not yet
> RPM_SUSPENDED).
>
> In addition to the console lock, you should be able to use that flag to
> avoid console writes while the console is in the process of suspending.
>
Yes RPM_SUSPENDING check helps along with console lock
<< Changes as below >>
---------------
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 59f548f..71964c3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1040,6 +1040,18 @@ serial_omap_console_write(struct console *co,
const char *s,
if (console_trylock())
console_lock = 1;
+ /*
+ * If console_lock is not available and we are in suspending
+ * state then we can avoid the console usage scenario
+ * as this may introduce recursive prints.
+ * Basically this scenario occurs during boot while
+ * printing debug bootlogs.
+ */
+
+ if (!console_lock &&
+ up->pdev->dev.power.runtime_status == RPM_SUSPENDING)
+ return;
+
local_irq_save(flags);
if (up->port.sysrq)
locked = 0;
------------
Is it ok to check the RPM_SUSPENDING flag in driver ?
I can't find any API currently available under runtime.h to use.
>> Also during bootup console_lock is not available.
>> --> uart_add_one_port
>> --> console_register
>> --> console_lock
>> --> console_unlock
>> --> call_console_drivers (here yet console_lock is not released)
>> --> uart_console_write
>>
>> Hence convert prints from omap_device_enable/disable to pr_debug.
>
> And what if you add 'debug' to your kernel command line? IOW, you're
> only solving the problem if you debug printk's are not going to the
> console.
>
yes correct issue will re-surface during boot-up once we have debug enabled
and all pr_debugs with omap_device_enable trying to use console_write.
--
Thanks,
Govindraj.R
> You're also not solving the problem that will happen down the road when
> someone else adds a printk to low-level code in order to debug something
> deep in the idle sequence.
>
> The recursive locking needs to be solved, not avoided.
>
> Kevin
>
>> Signed-off-by: Govindraj.R <govindraj.raja at ti.com>
>> ---
>> arch/arm/plat-omap/omap_device.c | 8 ++++----
>> drivers/tty/serial/omap-serial.c | 8 +++++++-
>> 2 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
>> index 9bbda9a..f7c6dca 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -145,12 +145,12 @@ static int _omap_device_activate(struct omap_device *od, u8 ignore_lat)
>> odpl->activate_lat_worst = act_lat;
>> if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
>> odpl->activate_lat = act_lat;
>> - pr_warning("omap_device: %s.%d: new worst case "
>> + pr_debug("omap_device: %s.%d: new worst case "
>> "activate latency %d: %llu\n",
>> od->pdev.name, od->pdev.id,
>> od->pm_lat_level, act_lat);
>> } else
>> - pr_warning("omap_device: %s.%d: activate "
>> + pr_debug("omap_device: %s.%d: activate "
>> "latency %d higher than exptected. "
>> "(%llu > %d)\n",
>> od->pdev.name, od->pdev.id,
>> @@ -213,12 +213,12 @@ static int _omap_device_deactivate(struct omap_device *od, u8 ignore_lat)
>> odpl->deactivate_lat_worst = deact_lat;
>> if (odpl->flags & OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
>> odpl->deactivate_lat = deact_lat;
>> - pr_warning("omap_device: %s.%d: new worst case "
>> + pr_debug("omap_device: %s.%d: new worst case "
>> "deactivate latency %d: %llu\n",
>> od->pdev.name, od->pdev.id,
>> od->pm_lat_level, deact_lat);
>> } else
>> - pr_warning("omap_device: %s.%d: deactivate "
>> + pr_debug("omap_device: %s.%d: deactivate "
>> "latency %d higher than exptected. "
>> "(%llu > %d)\n",
>> od->pdev.name, od->pdev.id,
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 633dfb4..7d8ca45 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -1007,7 +1007,10 @@ serial_omap_console_write(struct console *co, const char *s,
>> struct uart_omap_port *up = serial_omap_console_ports[co->index];
>> unsigned long flags;
>> unsigned int ier;
>> - int locked = 1;
>> + int console_lock = 0, locked = 1;
>> +
>> + if (console_trylock())
>> + console_lock = 1;
>>
>> local_irq_save(flags);
>> if (up->port.sysrq)
>> @@ -1043,6 +1046,9 @@ serial_omap_console_write(struct console *co, const char *s,
>> if (up->msr_saved_flags)
>> check_modem_status(up);
>>
>> + if (console_lock)
>> + console_unlock();
>> +
>> serial_omap_port_disable(up);
>> if (locked)
>> spin_unlock(&up->port.lock);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
More information about the linux-arm-kernel
mailing list