linux-kernel - Re: [alsa-devel] [PATCH] ASoC: max98090 add irq valid check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 30 Jul 2014 17:21:42 +0300
From:	Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To:	jianqun <xjq@...k-chips.com>, broonie@...nel.org,
	lgirdwood@...il.com, perex@...ex.cz, tiwai@...e.de,
	tbleung@...omium.org, dgreid@...omium.org,
	kevin.strasser@...el.com, swarren@...dia.com,
	ralph.birt@...imintegrated.com
CC:	alsa-devel@...a-project.org, heiko@...ech.de,
	linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH] ASoC: max98090 add irq valid check

Hi

On 07/30/2014 11:54 AM, jianqun wrote:
> From: Jianqun <xjq@...k-chips.com>
>
> Since IRQ pin from max98090 may NC, the irq number will be zero, that is
> invalid for request_threaded_irq, so just add irq valid check there.
>
> Since hardware may not MUST to use IRQ pin of max98090 as jack detect, the
> driver can work well without it, can report jack trigger to CPU by a GPIO.
>
> But here driver will register fail caused by failed to request irq.
>
> Signed-off-by: Jianqun <xjq@...k-chips.com>
> ---
>   sound/soc/codecs/max98090.c |   14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
> index 566919c..9dc0e8c 100644
> --- a/sound/soc/codecs/max98090.c
> +++ b/sound/soc/codecs/max98090.c
> @@ -2478,12 +2478,14 @@ static int max98090_probe(struct snd_soc_codec *codec)
>   	/* Register for interrupts */
>   	dev_dbg(codec->dev, "irq = %d\n", max98090->irq);
>   
> -	ret = request_threaded_irq(max98090->irq, NULL,
> -		max98090_interrupt, IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -		"max98090_interrupt", codec);
> -	if (ret < 0) {
> -		dev_err(codec->dev, "request_irq failed: %d\n",
> -			ret);
> +	if (max98090->irq) {
> +		ret = request_threaded_irq(max98090->irq, NULL,
> +			max98090_interrupt, IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +			"max98090_interrupt", codec);
> +		if (ret < 0) {
> +			dev_err(codec->dev, "request_irq failed: %d\n",
> +				ret);
> +		}
>   	}
>   
Remember to add also "return ret;" at the end of if () {} in case 
request_threaded_irq() fails since code should not continue probing now 
after adding a test for valid irq.

Should the if (max98090->irq) test be >0? I'm not sure is there 
possibility that can i2c->irq actually pass both -1 and 0 depending how 
struct i2c_board_info etc are initialized?

I forgot that I have a fix changing request_threaded_irq() to 
devm_request_threaded_irq() so you should redo your patch on top of 
for-next branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git.

-- 
Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/