From: Eduardo Valentin <eduardo.valentin@ti.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-omap@vger.kernel.org
Subject: Re: [PATCH 2/8] staging: omap-thermal: use spin_lock_irqsave inside IRQ handler
Date: Mon, 18 Mar 2013 15:38:38 -0400 [thread overview]
Message-ID: <51476D3E.6010305@ti.com> (raw)
In-Reply-To: <20130318191618.GU9138@mwanda>
On 18-03-2013 15:16, Dan Carpenter wrote:
> On Mon, Mar 18, 2013 at 10:59:10AM -0400, Eduardo Valentin wrote:
>> Even if the IRQ is not firing because it is ONE_SHOT and disable
>> at INTC level, the IRQ handler must use spin_lock_irqsave.
>> It is necessary to disable IRQs from the current
>> CPU while it is holding a spin_lock which is need.
>>
>
> Gar... I think I was just totally wrong on this. I think your
> original code was fine. Sorry Eduardo and Greg.
>
> This is a threaded IRQ so the regular spin_lock is fine or even the
> mutex would have been.
In fact it is. But I rather prefer to use spinlocks there, just to keep
the irq handler sane, even if it is moved to non-threaded IRQ.
>
> IRQ_ONESHOT is about triggering a second IRQ before the first one
> has been finished, btw.
It is, and that gets done by masking the IRQ at INTC level.
>
> I am an idiot.
Not really. Thanks for your time reviewing the driver.
I will resend this series. Drop this one and split patch 4/8 into two
I think (one for moving files, one for renaming functions)
>
> regards,
> dan carpenter
>
>
>
WARNING: multiple messages have this Message-ID (diff)
From: Eduardo Valentin <eduardo.valentin@ti.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: <gregkh@linuxfoundation.org>, <devel@driverdev.osuosl.org>,
<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-omap@vger.kernel.org>
Subject: Re: [PATCH 2/8] staging: omap-thermal: use spin_lock_irqsave inside IRQ handler
Date: Mon, 18 Mar 2013 15:38:38 -0400 [thread overview]
Message-ID: <51476D3E.6010305@ti.com> (raw)
In-Reply-To: <20130318191618.GU9138@mwanda>
On 18-03-2013 15:16, Dan Carpenter wrote:
> On Mon, Mar 18, 2013 at 10:59:10AM -0400, Eduardo Valentin wrote:
>> Even if the IRQ is not firing because it is ONE_SHOT and disable
>> at INTC level, the IRQ handler must use spin_lock_irqsave.
>> It is necessary to disable IRQs from the current
>> CPU while it is holding a spin_lock which is need.
>>
>
> Gar... I think I was just totally wrong on this. I think your
> original code was fine. Sorry Eduardo and Greg.
>
> This is a threaded IRQ so the regular spin_lock is fine or even the
> mutex would have been.
In fact it is. But I rather prefer to use spinlocks there, just to keep
the irq handler sane, even if it is moved to non-threaded IRQ.
>
> IRQ_ONESHOT is about triggering a second IRQ before the first one
> has been finished, btw.
It is, and that gets done by masking the IRQ at INTC level.
>
> I am an idiot.
Not really. Thanks for your time reviewing the driver.
I will resend this series. Drop this one and split patch 4/8 into two
I think (one for moving files, one for renaming functions)
>
> regards,
> dan carpenter
>
>
>
next prev parent reply other threads:[~2013-03-18 19:38 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-18 14:59 [PATCH 0/8] staging: [omap,ti-soc]-thermal: fixes and renaming Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 1/8] staging: omap-thermal: fix return value Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 16:39 ` Dan Carpenter
2013-03-18 17:20 ` Eduardo Valentin
2013-03-18 17:20 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 2/8] staging: omap-thermal: use spin_lock_irqsave inside IRQ handler Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 19:16 ` Dan Carpenter
2013-03-18 19:38 ` Eduardo Valentin [this message]
2013-03-18 19:38 ` Eduardo Valentin
2013-03-18 19:58 ` Dan Carpenter
2013-03-18 14:59 ` [PATCH 3/8] staging: omap-thermal: rename bg_ptr to bgp Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 4/8] staging: rename omap-thermal driver to ti-soc-thermal Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 19:17 ` Dan Carpenter
2013-03-18 14:59 ` [PATCH 5/8] staging: ti-soc-thermal: make unexported functions local Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 6/8] staging: ti-soc-thermal: split writable data from readonly data Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 7/8] stating: ti-soc-thermal: use sizeof(*pointer) while allocating Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-18 14:59 ` [PATCH 8/8] staging: ti-soc-thermal: fix several kernel-doc warnings and error Eduardo Valentin
2013-03-18 14:59 ` Eduardo Valentin
2013-03-19 14:36 ` [PATCH 0/8] staging: [omap,ti-soc]-thermal: fixes and renaming Eduardo Valentin
2013-03-19 14:36 ` Eduardo Valentin
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=51476D3E.6010305@ti.com \
--to=eduardo.valentin@ti.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-pm@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 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.