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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1384 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]