Re: [RFC 6/9] ASoC: hda: Add Code Loader DMA support

[Mark Brown <broonie@kernel.org>]

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

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

> +void ssth_cldma_int_enable(struct ssth_lib *ctx)
> +{
> +	ssth_updatel_bits(ctx, HDA_ADSP_REG_ADSPIC,
> +			ADSPIC_CL_DMA, 0x2);
> +}
> +void ssth_cldma_int_disable(struct ssth_lib *ctx)
> +{
> +	ssth_updatel_bits(ctx, HDA_ADSP_REG_ADSPIC,
> +			ADSPIC_CL_DMA, 0);
> +}

Blank lines between functions.  Seems to be an Intel coding style thing?
:P

> +/* Code loader helper APIs */
> +static void ssth_skl_cl_setup_bdle(struct snd_dma_buffer *dmab_data,
> +		u32 **bdlp, u32 count)
> +{
> +	u32 *bdl = *bdlp;
> +	int i = 0;
> +
> +	for (i = 0; i < count; i++) {
> +		phys_addr_t addr = virt_to_phys(dmab_data->area + i * PAGE_SIZE);

So this we index by i and...

> +
> +		bdl[0] = cpu_to_le32(lower_32_bits(addr));
> +		bdl[1] = cpu_to_le32(upper_32_bits(addr));
> +		bdl[2] = cpu_to_le32(PAGE_SIZE);
> +		bdl[3] = 0;
> +		bdl += 4;
> +	}

...this we index by stepping through the array with increments in the
body of the loop.  Consistency would be nice (and more obviously
correct).

> +static void ssth_skl_cl_cleaup(struct ssth_lib  *ctx)
> +{

Can't we clean it up instead?

> +	if (ctx->cl_dev.bytes_left <= ctx->cl_dev.bufsize &&
> +			ctx->cl_dev.bytes_left > ctx->cl_dev.period_size) {
> +
> +		dev_dbg(ctx->dev, "%s: size less than buffer size: %u\n",
> +			       __func__, ctx->cl_dev.bytes_left);
> +		ssth_cldma_int_disable(ctx);
> +		ctx->cl_dev.curr_spib_pos = ctx->cl_dev.bytes_left;
> +		ssth_cl_dma_fill_buffer(ctx, size, false, false, 0, false, true);
> +		do {
> +			mdelay(5);
> +			link_pos = ssth_readl(ctx, CL_SD_LPIB);
> +		} while (link_pos < size);

Should time out in case the DMA got stuck somehow.

> +		goto cleanup;
> +	}

What if the buffer is just too big?  Looks like this would loop for
ever.

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

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