All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@cam.ac.uk>
To: Jonathan Cameron <Jonathan.Cameron@gmail.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Liam Girdwood <lrg@slimlogic.co.uk>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] Regulator: Add a voltage changed event to notify consumers
Date: Mon, 19 Jan 2009 18:08:02 +0000	[thread overview]
Message-ID: <4974C182.1090902@cam.ac.uk> (raw)
In-Reply-To: <4974B107.6050701@gmail.com>

Jonathan Cameron wrote:
> Mark Brown wrote:
>> On Sun, Jan 18, 2009 at 06:47:25PM +0000, Jonathan Cameron wrote:
>>
>>> -out:
>>> +	mutex_unlock(&rdev->mutex);
>>> +	_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE, NULL);
>>> +	return 0;
>>> +out_unlock:
>> It'd be nice if we could modify _notifier_call_chain() to push the 
>> locking out a bit so we don't need to drop the lock before calling the
>> notifier.  On the other hand, for anything that isn't memory mapped or
>> GPIO controlled (most regulators are in this category) the cost of the
>> I/O is going to make this a non-issue.
> Agreed.  On that note, isn't any call to regulator_force_disable
> currently going to deadlock? (lock held in regulator_force_disable,
> then re-called in _notifier_call_chain.)
> 
> I'll have a look into moving the locks out of _notifier_call_chain.
Having had a quick look at this, it comes down to a question of
whether we want to hold the lock on one regulator whilst notifying
any regulators it supplies.

I personally can't see that this would be a problem, but it has definitely
been structured to avoid doing so.

Trying to come up with scenarios that may make this a problem:

Parent notifies child of a voltage change. This change results in 
some complex problem (not covered by constraints - I'm stretching here)
that in turn causes a the child regulator to request a forced disable
from the parent and causes deadlock.

Can anyone come up with a non contrived reason not to move constraints clean
out of _notifier_call_chain and rely on caller holding the lock?
Clearly this also requires applying locks to child regulators in 
the loop at the end of _notifier_call_chain.

Next email contains a patch combing this change with the voltage
notification patch.

Cheers,

Jonathan

  reply	other threads:[~2009-01-19 18:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-03 21:11 [RFC] Regulator: Possibility of passing notifications of non alarm events Jonathan Cameron
2009-01-04 10:31 ` Liam Girdwood
2009-01-18 18:47   ` [RFC] Regulator: Add a voltage changed event to notify consumers Jonathan Cameron
2009-01-19 15:29     ` Mark Brown
2009-01-19 16:57       ` Jonathan Cameron
2009-01-19 18:08         ` Jonathan Cameron [this message]
2009-01-19 18:20           ` [RFC] Regulator: Push lock out of _notifier_call_chain + add voltage change event Jonathan Cameron
2009-01-20 20:09             ` Liam Girdwood

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=4974C182.1090902@cam.ac.uk \
    --to=jic23@cam.ac.uk \
    --cc=Jonathan.Cameron@gmail.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.