From patchwork Thu May 5 10:25:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Govindraj R X-Patchwork-Id: 756442 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p45APkcO010830 for ; Thu, 5 May 2011 10:25:46 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753814Ab1EEKZo (ORCPT ); Thu, 5 May 2011 06:25:44 -0400 Received: from mail-bw0-f46.google.com ([209.85.214.46]:40951 "EHLO mail-bw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753790Ab1EEKZn convert rfc822-to-8bit (ORCPT ); Thu, 5 May 2011 06:25:43 -0400 Received: by bwz15 with SMTP id 15so1694710bwz.19 for ; Thu, 05 May 2011 03:25:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=Dv3zmDYeDopAAOJwVcV4FkOZhJy3cc+X3/JeT0UgQQw=; b=RMv2ALQgn9wd8qC2uolNsOt21e0mfwKIU2vvxhpd4wsqf4LbDrhIfgYGA71n2rm1nn xVsHkfHriXg3GO5NzM71jODovot7397Wv6qssv/8ZQhSyAWAoI9Tg8c2UqU4Zu1JtqOz Ri4AcQT74VppJdQzB0tVeKEkmpeKPK9KJXSsg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=ZW9cvylx/1cQhp2lI3vTOx8TDM6/ktHz7TBpYRJW5PMpkN0i1MV6D5hdJD4OHhI4/q FeSh0hFwUHraXt9amRLgg0Qiva3yqdWsRr/VulcZZ+u5cb1onZ694qgO6JqBdtt7Cnjs jmmmgZBooP8+QLG/NlzaZbmCp9StbkcMS2uqA= Received: by 10.204.7.209 with SMTP id e17mr666847bke.162.1304591137153; Thu, 05 May 2011 03:25:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.204.34.150 with HTTP; Thu, 5 May 2011 03:25:17 -0700 (PDT) In-Reply-To: <87vcxqp577.fsf@ti.com> References: <1304080796-625-1-git-send-email-govindraj.raja@ti.com> <1304080796-625-6-git-send-email-govindraj.raja@ti.com> <87vcxqp577.fsf@ti.com> From: Govindraj Date: Thu, 5 May 2011 15:55:17 +0530 Message-ID: Subject: Re: [PATCH v2 05/12] OMAP: Serial: Hold console lock for console usage. To: Kevin Hilman Cc: "Govindraj.R" , linux-omap@vger.kernel.org, linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Tony Lindgren , Benoit Cousson , Paul Walmsley , Rajendra Nayak Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Thu, 05 May 2011 10:25:47 +0000 (UTC) On Thu, May 5, 2011 at 2:13 AM, Kevin Hilman wrote: > "Govindraj.R" 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 >> --------------- 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 >> --- >>  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@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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)