Re: [PATCH v4 01/14] ASoC: SOF: Add Sound Open Firmware driver core

[Takashi Iwai <tiwai@suse.de>]

On Wed, 13 Feb 2019 23:07:21 +0100,
Pierre-Louis Bossart wrote:
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index eb7db605955b..da3c13185263 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1232,6 +1232,9 @@ struct snd_soc_pcm_runtime {
>  	/* bit field */
>  	unsigned int dev_registered:1;
>  	unsigned int pop_wait:1;
> +
> +	/* private data - core does not touch */
> +	void *private; /* FIXME: still SOF-specific, needs to less ambiguous */

Then better to name it sof_private?


> +int snd_sof_create_page_table(struct snd_sof_dev *sdev,
> +			      struct snd_dma_buffer *dmab,
> +			      unsigned char *page_table, size_t size)
> +{
> +	int i, pages;
> +
> +	pages = snd_sgbuf_aligned_pages(size);
> +
> +	dev_dbg(sdev->dev, "generating page table for %p size 0x%zx pages %d\n",
> +		dmab->area, size, pages);
> +
> +	for (i = 0; i < pages; i++) {
> +		/*
> +		 * The number of valid address bits for each page is 20.
> +		 * idx determines the byte position within page_table
> +		 * where the current page's address is stored
> +		 * in the compressed page_table.
> +		 * This can be calculated by multiplying the page number by 2.5.
> +		 */
> +		u32 idx = (5 * i) >> 1;
> +		u32 pfn = snd_sgbuf_get_addr(dmab, i * PAGE_SIZE) >> PAGE_SHIFT;
> +		u32 *pg_table;
> +
> +		dev_vdbg(sdev->dev, "pfn i %i idx %d pfn %x\n", i, idx, pfn);
> +
> +		pg_table = (u32 *)(page_table + idx);
> +
> +		if (i & 1)
> +			*pg_table |= (pfn << 4);
> +		else
> +			*pg_table |= pfn;

I guess this is OK for now, but better to give some notes that it's
x86-specific.  To be more generic, we'd have to consider endianess and
the unaligned access in the code above.


thanks,

Takashi