Re: [patch 1/2] alsa: ASoC driver for s6000 I2S interface

["Daniel Glöckner" <dg@emlix.com>]

On 03/26/2009 03:15 PM, Mark Brown wrote:
> I'm not seeing any hits on XTENSA_VARIANT_S6000 in next?  While that
> shouldn't hurt immediately since it'll just never appear it'd be good to
> make sure that the APIs this depends on are going into mainline in the
> form they're in.  Do you know what the status is there?

The patch introducing XTENSA_VARIANT_S6000 has already been reviewed by
the architecture maintainer and there is hope that it will make it into
2.6.30. The dma api, which we posted four days ago to LKML, has not
received any comments so far.

>> +#define S6000_I2S_RATES	(SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_KNOT | \
>> +			 SNDRV_PCM_RATE_5512 | SNDRV_PCM_RATE_8000_192000)
> 
> SND_PCM_RATE_KNOT doesn't work with ASoC (though it shouldn't do any
> harm since it'll just get removed).

Except for the maximum bit clock (50 MHz) the interface doesn't care about
the sampling rate, so I put in all SNDRV_PCM_RATE_* constants.
Shall I remove it?

>> +static inline void s6_i2s_write_reg(struct s6000_i2s_dev *dev, int reg, u32 val)
>> +{
>> +	writel(val, dev->scbbase + reg);
>> +}
> 
> Any reason to expose these to users? 
> 
>> +static inline void s6000_i2s_start_channel(struct s6000_i2s_dev *dev,
>> +					   int channel)
>> +{
> 
> This and the rest of the functions in the file look especially like it
> should be in the body of the driver.  It also looks a little large to be
> forcing inline?

This was an attempt to avoid symbol exports..
I'll move everything to s6000-i2s.c and export the three necessary
functions.

>> +	if (unlikely(i2s_errors & S6_I2S_INT_UNDERRUN) &&
>> +	    playback->runtime &&
>> +	    snd_pcm_running(playback)) {
>> +		printk(KERN_WARNING "s6000-i2s: Tx Underrun\n");
>> +		s6000_i2s_start(playback);
>> +
>> +		prtd = playback->runtime->private_data;
>> +		spin_lock(&prtd->lock);
>> +		s6000_pcm_enqueue_dma(playback);
>> +		/* a second descriptor will be queued below */
>> +		spin_unlock(&prtd->lock);
> 
> Hrm.  ALSA has underrun handling mechanisms as standard?

I guess snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN) is the way to go then?

The remaining issues will be addressed in the v2 patch as well.

  Daniel


-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax -11, Bahnhofsallee 1b, 37081 Göttingen, Germany
Geschäftsführung: Dr. Uwe Kracke, Dr. Cord Seele, Ust-IdNr.: DE 205 198 055
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160

emlix - your embedded linux partner

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel