Re: [PATCH 18/31] ASoC: tegra: allocate AHUB FIFO during probe() not startup()

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

On Tue, Dec 03, 2013 at 12:55:55PM -0700, Stephen Warren wrote:
> On 11/29/2013 07:40 AM, Thierry Reding wrote:
> > On Fri, Nov 15, 2013 at 01:54:13PM -0700, Stephen Warren wrote:
> >> Teh Tegra30 I2S driver currently allocates DMA FIFOs from the
> >> AHUB only when an audio stream starts playback. This is
> >> theoretically nice for resource sharing, but makes no practical
> >> difference for any configuration the drivers currently support.
> >> However, this deferral prevents conversion to the standard DMA DT
> >> bindings, since conversion requires knowledge of the specific DMA
> >> channel to be allocated, which in turn depends on which specific
> >> FIFO was allocated.
> >> 
> >> For this reason, move the FIFO allocate into probe() to allow
> >> later
> ...
> >> +	i2s->playback_dma_data.addr_width =
> >> DMA_SLAVE_BUSWIDTH_4_BYTES; +	i2s->playback_dma_data.maxburst =
> >> 4; +	ret =
> >> tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, +
> >> &i2s->playback_dma_data.addr, +
> >> &i2s->playback_dma_data.slave_id); +	if (ret) { +
> >> dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret); +
> >> goto err_suspend; +	} +	ret =
> >> tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif, +
> >> i2s->playback_fifo_cif); +	if (ret) { +		dev_err(&pdev->dev,
> >> "Could not route TX FIFO: %d\n", ret); +		goto err_free_tx_fifo; 
> >> +	} + +	i2s->capture_dma_data.addr_width =
> >> DMA_SLAVE_BUSWIDTH_4_BYTES; +	i2s->capture_dma_data.maxburst =
> >> 4; +	ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif, 
> >> +					    &i2s->capture_dma_data.addr, +
> >> &i2s->capture_dma_data.slave_id); +	if (ret) { +
> >> dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret); +
> >> goto err_unroute_tx_fifo; +	} +	ret =
> >> tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif, +
> >> i2s->capture_i2s_cif); +	if (ret) { +		dev_err(&pdev->dev, "Could
> >> not route TX FIFO: %d\n", ret); +		goto err_free_rx_fifo; +	} +
> > 
> > It could be useful to have these in a separate function so as not
> > to make the .probe() any larger. It's already pretty big as it is.
> 
> May I ignore this; I personally prefer larger linear functions; it's
> much easier to follow the code-flow without having to jump back/forth
> between a bunch of single-use functions.

Alright, I won't lose any sleep over it.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]