Re: [alsa-devel] [RFC 4/9] ASoC: hda: Add DSP init and boot up functionality

On Fri, Apr 17, 2015 at 06:46:02PM +0530, Vinod Koul wrote:

+static int ssth_acquire_irq(struct ssth_lib *ctx) +{

• if (request_threaded_irq(ctx->irq, ssth_interrupt,

• 	NULL, IRQF_SHARED, KBUILD_MODNAME, ctx)) {
  

• dev_err(ctx->dev, "unable to grab threaded IRQ %d, disabling device\n", ctx->irq);
  

• return -1;
  

Don't discard the return code, pass it back. I'm pretty sure the error wasn't -EPERM anyway. Though...

• /* initialize IPC */
• ctx->ipc = ssth_ipc_init(ctx->dev, ctx);
• if (ctx->ipc == NULL)

• ret = -ENODEV;
  

• /* Now let's request the IRQ */
• ssth_acquire_irq(ctx);

..of course we ignore errors anyway. Why not just inline this?

+int ssth_dsp_free0(struct ssth_lib *dsp) +{

• int ret = 0;
• dev_dbg(dsp->dev, "In %s\n", __func__);
• ssth_ipc_int_disable(dsp);
• free_irq(dsp->irq, dsp);
• ssth_ipc_free(dsp->ipc);
• ssth_disable_dsp_core(dsp);
• kfree(dsp);
• return ret;

+} +EXPORT_SYMBOL_GPL(ssth_dsp_free0);

free0?

+bool ssth_dsp_is_running(struct ssth_lib *ctx) +{

• bool ret = 0;
• mutex_lock(&ctx->sst_lock);
• ret = (ctx->sst_state == SST_DSP_RUNNING) ? 1 : 0;
• mutex_unlock(&ctx->sst_lock);

How does this get used? The state could change immediately after returning. Also no need for the ternery operator there, logic operations generate logic results anyway.