On 03/31/2012 03:34 PM, Mark Brown wrote:
On Fri, Mar 30, 2012 at 05:07:30PM -0600, Stephen Warren wrote:
+#ifdef CONFIG_DEBUG_FS +static int tegra30_i2s_show(struct seq_file *s, void *unused) +{
Abstraction please - this is open coded in several of your drivers.
There's very little code here; unifying it would require passing the clock enable/disable and register read functions to any common code, which would end up making everything rather more complex. Sometimes there are multiple register read functions for different chunks of the address space. I guess if we did have regmap for MMIO, these debugfs files would pretty much come for free. Is that what you're looking for here?
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
ret = tegra30_ahub_allocate_tx_fifo(&i2s->txcif,&i2s->playback_dma_data.addr,&i2s->playback_dma_data.req_sel);i2s->playback_dma_data.wrap = 4;i2s->playback_dma_data.width = 32;tegra30_ahub_set_rx_cif_source(TEGRA30_AHUB_RXCIF_I2S0_RX0 + i2s->cif_id,i2s->txcif);So. This is all fairly straightforward, simple and non-invasive for CPU<->I2S streams but obviously it's locking out any loopbacks within the AHUB which is a bit sad.
Yes, I certainly foresee the way the DAIs interact with AHUB changing when adding more advanced features, such as if representing AHUB as a CODEC.
Our downstream kernels do have some code that sets up loopbacks between I2S modules, and it's all derived from these patches as a starting point. The downstream solution probably isn't upstreamable though; it wasn't written with upstream approaches in mind:-(
Looking at this it seems like all that's required is to propagate stream events back up the chain and do the routing, there doesn't seem to be much other interaction between the AHUB and the interfaces?
I suspect that's pretty much what representing AHUB as a CODEC would turn into, or very similar anyway. Is it OK to get the basic functionality in first and then refactor this, or would you prefer going straight to the complete solution?