Re: [PATCHv2 6/7] ASoC: TWL6030: Enable audio interrupt

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Fri, Sep 25, 2009 at 09:03:41PM -0500, Lopez Cruz, Misael wrote:

> +/* audio interrupt handler */
> +irqreturn_t twl6030_naudint_handler(int irq, void *data)
> +{
> +	struct snd_soc_codec *codec = data;
> +	struct twl6030_data *priv = codec->private_data;
> +
> +	schedule_work(&priv->audint_work);
> +
> +	/* disable audint irq to let workqueue to execute */
> +	disable_irq_nosync(irq);
> +
> +	return IRQ_HANDLED;
> +}

You should use request_threaded_irq() here and have the body of the work
function in the threaded IRQ handler.  It essentially does the same
thing that you've open coded here but with less code and is a bit more
friendly to the IRQ infrastructure since it lets it know what's going
on more explicitly.

> @@ -954,8 +1008,15 @@ static int __devinit twl6030_codec_probe(struct platform_device *pdev)
>  	struct twl6030_data *priv;
>  	struct snd_soc_codec *codec;
>  	int audpwron_gpio = twl_codec->audpwron_gpio;
> +	int naudint_irq = twl_codec->naudint_irq;
>  	int ret = 0;
>  
> +	/* prerequisites */
> +	if (!naudint_irq) {
> +		dev_err(&pdev->dev, "no audio interrupt irq supplied\n");
> +		return -EINVAL;
> +	}
> +

Is it worth making this optional?  I wouldn't like to rely on boards
remembering to wire up the interrupt line.