All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Quentin Schulz <quentin.schulz@free-electrons.com>,
	knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
	wens@csie.org, linux-iio@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	thomas.petazzoni@free-electrons.com
Subject: Re: [PATCH] iio: adc: sun4i-gpadc-iio: fix unbalanced irq enable/disable
Date: Tue, 4 Jul 2017 20:30:05 +0100	[thread overview]
Message-ID: <20170704203005.49d9887e@kernel.org> (raw)
In-Reply-To: <20170703211945.4vjnnjwl2lkcfr7u@flea>

On Mon, 3 Jul 2017 23:19:45 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Mon, Jul 03, 2017 at 03:09:26PM +0200, Quentin Schulz wrote:
> > When initializing interrupts, the devm_request_any_context_irq will
> > enable them right away. An atomic flag was set in sun4i_irq_init and read
> > in the interrupt handler to make sure no unwanted interrupts were
> > handled. If an unwanted interrupt occurred, the handler would disable
> > the irq and return IRQ_HANDLED. However, at the end of sun4i_irq_init,
> > the irq would be disabled as well, resulting in an unbalanced enable
> > (since there are more disables than enables, the code enabling the
> > interrupt would never be called).
> > 
> > When reading the ADC or the temperature, the respective irq would be
> > enabled in the read function and disabled in the irq handler. In the
> > read function, we would wait for a completion (with a timeout) that will
> > be set in the irq handler. However, if the completion is never set or if
> > the wait for completion times out, the irq would not be disabled in the
> > read function resulting in an unbalanced enable once the read function
> > is called again (since there are 2+ enables for no disable).
> > 
> > Moving disable_irq from the irq handler to the read function get rid of
> > these two cases of unbalanced enable.
> > 
> > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC")
> > 
> > Reported-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>  
> 
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Nice detailed explanation - thanks.

Applied to the fixes-togreg branch of iio.git.

Jonathan
> 
> Thanks!
> Maxime
> 


WARNING: multiple messages have this Message-ID (diff)
From: jic23@kernel.org (Jonathan Cameron)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] iio: adc: sun4i-gpadc-iio: fix unbalanced irq enable/disable
Date: Tue, 4 Jul 2017 20:30:05 +0100	[thread overview]
Message-ID: <20170704203005.49d9887e@kernel.org> (raw)
In-Reply-To: <20170703211945.4vjnnjwl2lkcfr7u@flea>

On Mon, 3 Jul 2017 23:19:45 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> On Mon, Jul 03, 2017 at 03:09:26PM +0200, Quentin Schulz wrote:
> > When initializing interrupts, the devm_request_any_context_irq will
> > enable them right away. An atomic flag was set in sun4i_irq_init and read
> > in the interrupt handler to make sure no unwanted interrupts were
> > handled. If an unwanted interrupt occurred, the handler would disable
> > the irq and return IRQ_HANDLED. However, at the end of sun4i_irq_init,
> > the irq would be disabled as well, resulting in an unbalanced enable
> > (since there are more disables than enables, the code enabling the
> > interrupt would never be called).
> > 
> > When reading the ADC or the temperature, the respective irq would be
> > enabled in the read function and disabled in the irq handler. In the
> > read function, we would wait for a completion (with a timeout) that will
> > be set in the irq handler. However, if the completion is never set or if
> > the wait for completion times out, the irq would not be disabled in the
> > read function resulting in an unbalanced enable once the read function
> > is called again (since there are 2+ enables for no disable).
> > 
> > Moving disable_irq from the irq handler to the read function get rid of
> > these two cases of unbalanced enable.
> > 
> > Fixes: d1caa9905538 ("iio: adc: add support for Allwinner SoCs ADC")
> > 
> > Reported-by: Andreas F?rber <afaerber@suse.de>
> > Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>  
> 
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Nice detailed explanation - thanks.

Applied to the fixes-togreg branch of iio.git.

Jonathan
> 
> Thanks!
> Maxime
> 

  reply	other threads:[~2017-07-04 19:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-03 13:09 [PATCH] iio: adc: sun4i-gpadc-iio: fix unbalanced irq enable/disable Quentin Schulz
2017-07-03 13:09 ` Quentin Schulz
2017-07-03 21:19 ` Maxime Ripard
2017-07-03 21:19   ` Maxime Ripard
2017-07-04 19:30   ` Jonathan Cameron [this message]
2017-07-04 19:30     ` Jonathan Cameron

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=20170704203005.49d9887e@kernel.org \
    --to=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.ripard@free-electrons.com \
    --cc=pmeerw@pmeerw.net \
    --cc=quentin.schulz@free-electrons.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=wens@csie.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.