All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@nokia.com>
To: "Nurkkala Eero.An (EXT-Offcode/Oulu)" <ext-Eero.Nurkkala@nokia.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"tony@atomide.com" <tony@atomide.com>,
	"broonie@opensource.wolfsonmicro.com"
	<broonie@opensource.wolfsonmicro.com>,
	"Valentin Eduardo (Nokia-D/Helsinki)"
	<eduardo.valentin@nokia.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>
Subject: Re: [PATCH] OMAP: McBSP: Do not use extensive spin locks for dma_op_mode
Date: Thu, 29 Oct 2009 08:35:51 +0200	[thread overview]
Message-ID: <200910290835.51475.peter.ujfalusi@nokia.com> (raw)
In-Reply-To: <1256712814.30157.388.camel@eenurkka-desktop>

On Wednesday 28 October 2009 08:53:34 Nurkkala Eero.An (EXT-Offcode/Oulu) wrote:
> Yeah, maybe I took the SMP safeness into play without looking any code,
> my bad =) I was remembering a different version of this McBSP thing, now
> that I looked into it, it looked different.
> 
> Right, I reviewed the code, and it was first looking really bad at
> sound/soc/omap/omap-mcbsp.c, where it calls omap_mcbsp_get_dma_op_mode()
> from different places. However, it's not an issue because in:
> arch/arm/plat-omap/mcbsp.c : dma_op_mode_store(),
> the dma_op_mode is written only if the mcbsp is unoccupied. So it is SMP
> safe.
> 
> ..and a single read is always atomic, so this is buggy code:
> 
> 301         spin_lock_irq(&mcbsp->lock);
> 302         dma_op_mode = mcbsp->dma_op_mode;
> 303         spin_unlock_irq(&mcbsp->lock);
> 304
> 305         return dma_op_mode;
> 
> The spinlocks are unnecessary. In the above example, you get the same
> with just "return mcbsp->dma_op_mode;"
> 
> -> Peter's patch is a good cleanup.

Jarkko: are we going to take this?


-- 
Péter

  reply	other threads:[~2009-10-29  6:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-27 11:00 [PATCH] OMAP: McBSP: Do not use extensive spin locks for dma_op_mode Peter Ujfalusi
2009-10-27 11:07 ` Eero Nurkkala
2009-10-27 11:17   ` Peter Ujfalusi
2009-10-27 12:00     ` Mark Brown
2009-10-27 12:04       ` Eero Nurkkala
2009-10-27 12:04       ` [alsa-devel] " Peter Ujfalusi
2009-10-27 14:00   ` Jarkko Nikula
2009-10-28  5:52     ` Peter Ujfalusi
2009-10-28  6:53       ` Eero Nurkkala
2009-10-29  6:35         ` Peter Ujfalusi [this message]
2009-10-29  7:15           ` [alsa-devel] " Jarkko Nikula
2009-11-09  7:49             ` Peter Ujfalusi
2009-11-09 13:18               ` Mark Brown
2009-10-29  7:13         ` [alsa-devel] " Jarkko Nikula
2009-11-12 22:17 ` [APPLIED] " Tony Lindgren

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=200910290835.51475.peter.ujfalusi@nokia.com \
    --to=peter.ujfalusi@nokia.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=eduardo.valentin@nokia.com \
    --cc=ext-Eero.Nurkkala@nokia.com \
    --cc=linux-omap@vger.kernel.org \
    --cc=tony@atomide.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 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.