linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Govindraj.R" <govindraj.raja@ti.com>
To: linux-omap@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-pm@lists.linux-foundation.org
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>, Paul Walmsley <paul@pwsan.com>,
	Kevin Hilman <khilman@ti.com>,
	Vishwanath Sripathy <vishwanath.bs@ti.com>,
	Partha Basak <p-basak2@ti.com>, Felipe Balbi <balbi@ti.com>,
	Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Rajendra Nayak <rnayak@ti.com>, Benoit Cousson <b-cousson@ti.com>,
	Tero Kristo <t-kristo@ti.com>,
	Govindraj R <govindraj.raja@ti.com>
Subject: [RFC v2]: Issues implementing clock handling mechanism within UART driver
Date: Thu, 28 Jul 2011 14:59:55 +0530	[thread overview]
Message-ID: <1311845395-31917-1-git-send-email-govindraj.raja@ti.com> (raw)

To handle uart clocks within uart driver we need the *semantics of clock enable
API should not internally trigger any UART PORT operation* even before
completing clock_enable.

Ex: while configuring baud rate we may use runtime api's to enable uart port.
get_sync -> printk -> call console uart to print

For example, say we are calling Uart_configure baud to configure the baud-rate.
Even before this operation completes, if there is print, we may switch to uart
write operation which is undesirable.

This occurs due to any printks called from omap_device ()/omap_hwmod ()/clk layers
which come in the flow from pm_runtime_get_sync/ pm_runtime_put_autosuspend API's.

	1. pm_runtime_get _sync -> printk -> console_write -> pm_runtime_get_sync
	2. pm_runtime_ put_autosuspend -> rpm_suspend -> pm_generic suspend ->
	   omap_device_disable -> debug prints -> console_write -> pm_runtime_get_sync

This leads to a deadlock caused by contention of the power_lock from
the same runtime API context.

Work-around explored:
--------------------
Use console lock aggressively around each get_sync/put_sync.

	Uart_configure baud {
		1. Console_lock
		2. pm_runtime_get_sync
		3. Console_unlock (and send the prints accumulated in the
		   log buffer to the console) which leads to a console_write .
		4. This console_write in turn leads to a console_lock+pm_runtime_get_sync
		5. Also  Console_unlock is undesirable here since it jumps to
		   console write even before baud config was complete.
	}

Locking the console around a pm_runtime_get_sync()will make sure that all prints
are sent to the log-buffer until the console in unlocked, thereby getting rid of
recursively jumping from one port op to another even before current port
operation is complete.

However, this leads to:
	1. Cant bind put_autosuspend with console lock as they get scheduled later
	   put -> print -> console_write -> get_sync (one runtime API context we
	   may enter another runtime API power lock contention)
	2. Using console_lock wrapper everywhere in driver to suppress prints
	   from get_sync is too aggressive in nature.
	3. During Boot-up, console_lock is not available until uart driver
	   completes its registration with kernel console_driver.

Observing the semantics of console_lock/unlock, it seems undesirable to be used
within uart driver. But can rather be used form global stand point to suppress
uart prints.

Example:
1. While system wide suspending
2. In cpu idle path., etc.

Proposal:
--------
	1. For the UART, follow the current approach of locking the console in
	   Idle/Suspend path before cutting the clock but using pm_runtime_putsync.
	   That is, continue using the prepare/resume Idle calls in idle path.
	2. Other Approach would be adding conditions to debug prints from
	   omap_device/ omap_hwmod/clock_framework avoid calling these debug
	   prints for uart.
	   Or Even a debug macro that would not debug prints if the context is
	   from uart.

Any further approaches are welcome.

Have captured little more details compared to v1 posted erlier:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg52707.html

-- 
1.7.4.1


             reply	other threads:[~2011-07-28  9:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-28  9:29 Govindraj.R [this message]
2011-07-29  9:55 ` [RFC v2]: Issues implementing clock handling mechanism within UART driver Felipe Balbi
2011-07-29 11:24   ` Govindraj
2011-07-29 11:37     ` Felipe Balbi
2011-07-29 11:59       ` Govindraj
2011-07-29 12:19         ` Felipe Balbi
2011-07-29 12:58           ` Govindraj
2011-07-29 14:02             ` Felipe Balbi
2011-07-29 15:13               ` Govindraj
2011-08-01  9:03                 ` Felipe Balbi
2011-08-01  9:56                   ` Raja, Govindraj
2011-08-01 10:02                     ` Felipe Balbi
2011-08-01 12:46                       ` Govindraj
2011-08-01 10:00                   ` Govindraj

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1311845395-31917-1-git-send-email-govindraj.raja@ti.com \
    --to=govindraj.raja@ti.com \
    --cc=b-cousson@ti.com \
    --cc=balbi@ti.com \
    --cc=khilman@ti.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=p-basak2@ti.com \
    --cc=paul@pwsan.com \
    --cc=rjw@sisk.pl \
    --cc=rnayak@ti.com \
    --cc=santosh.shilimkar@ti.com \
    --cc=t-kristo@ti.com \
    --cc=vishwanath.bs@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).