public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
To: Andres Salomon <dilinger@collabora.co.uk>
Cc: linux-kernel@vger.kernel.org, linux-geode@bombadil.infradead.org
Subject: [PATCH] geode-mfgpt: restore previous behavior for selecting IRQ
Date: Tue, 12 Jan 2010 11:14:30 +0100	[thread overview]
Message-ID: <4B4C4B86.7090509@LiPPERTEmbedded.de> (raw)

geode-mfgpt: restore previous behavior for selecting IRQ

The MFGPT IRQ used to be, in order of decreasing priority,
* IRQ supplied by the user as a boot-time parameter,
* IRQ previously set by the BIOS or another driver,
* default IRQ given at compile time.

Return to this behavior, which got broken when splitting the
MFGPT/clocksource driver for 2.6.33-rc1.

Signed-off-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
---

Hi Andres,

a bit more detail. Up to 2.6.32 the MFGPT IRQ was chosen like this:

static int irq;
static int __init mfgpt_setup(char *str)
{
	get_option(&str, &irq);
	return 1;
}
__setup("mfgpt_irq=", mfgpt_setup);
[...]
/* Choose IRQ: if none supplied, keep IRQ already set or use default */
if (!*irq)
	*irq = (zsel >> shift) & 0xF;	// may have been set by the BIOS
if (!*irq)
	*irq = MFGPT_DEFAULT_IRQ;	// #defined as 7

The code in 2.6.33-rc3 after the MFGPT/clocksource split looks like this:

static int timer_irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ;	// defaults to 7
module_param_named(irq, timer_irq, int, 0644);
[...]
if (!*irq)
	*irq = (zsel >> shift) & 0xF;		// gets ignored now
if (!*irq)
	*irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ;	// again!?

The patch removes the first of the two ocurrances of
CONFIG_CS5535_MFGPT_DEFAULT_IRQ, reverting the IRQ selection to the previous
(working) behavior.


BTW, while looking into this I stumbled upon the fact that cs5535_mfgpt_init()
doesn't free up the timer in the error path if cs5535_mfgpt_setup_irq() or
setup_irq() fail. However, some comments in cs5535-mfgpt.c (stating that it's
not possible to release the individual timer hardware) make me think this might
have been skipped on purpose? Could you please have a look?

Thanks,
Jens

--- linux-2.6.33-rc3-git4/drivers/clocksource/cs5535-clockevt.c
+++ previous_behavior/drivers/clocksource/cs5535-clockevt.c
@@ -21,7 +21,7 @@
 
 #define DRV_NAME "cs5535-clockevt"
 
-static int timer_irq = CONFIG_CS5535_MFGPT_DEFAULT_IRQ;
+static int timer_irq;
 module_param_named(irq, timer_irq, int, 0644);
 MODULE_PARM_DESC(irq, "Which IRQ to use for the clock source MFGPT ticks.");
 
_


             reply	other threads:[~2010-01-12 10:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-12 10:14 Jens Rottmann [this message]
2010-01-18 15:34 ` [PATCH] geode-mfgpt: restore previous behavior for selecting IRQ Andres Salomon
2010-01-25 13:13   ` [PATCH] cs5535-clockevt: free timer in IRQ setup error path Jens Rottmann
2010-07-08 15:04     ` [PATCH] cs5535-mfgpt: reuse timers that have never been set up Jens Rottmann

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=4B4C4B86.7090509@LiPPERTEmbedded.de \
    --to=jrottmann@lippertembedded.de \
    --cc=dilinger@collabora.co.uk \
    --cc=linux-geode@bombadil.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    /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