From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH v4 01/14] ASoC: SOF: Add Sound Open
	Firmware driver core
Date: Thu, 14 Feb 2019 10:25:20 +0100
Message-ID: <s5h1s4abu2n.wl-tiwai@suse.de>
References: <20190213220734.10471-1-pierre-louis.bossart@linux.intel.com>
 <20190213220734.10471-2-pierre-louis.bossart@linux.intel.com>
Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <alsa-devel-bounces@alsa-project.org>
In-Reply-To: <20190213220734.10471-2-pierre-louis.bossart@linux.intel.com>
List-Unsubscribe: <http://mailman.alsa-project.org/mailman/options/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=unsubscribe>
List-Archive: <http://mailman.alsa-project.org/pipermail/alsa-devel/>
List-Post: <mailto:alsa-devel@alsa-project.org>
List-Help: <mailto:alsa-devel-request@alsa-project.org?subject=help>
List-Subscribe: <http://mailman.alsa-project.org/mailman/listinfo/alsa-devel>,
 <mailto:alsa-devel-request@alsa-project.org?subject=subscribe>
Errors-To: alsa-devel-bounces@alsa-project.org
Sender: alsa-devel-bounces@alsa-project.org
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, andriy.shevchenko@intel.com, Daniel Baluta <daniel.baluta@gmail.com>, liam.r.girdwood@linux.intel.com, vkoul@kernel.org, broonie@kernel.org, Alan Cox <alan@linux.intel.com>, sound-open-firmware@alsa-project.org
List-Id: alsa-devel@alsa-project.org

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