On Mon, Jun 20, 2011 at 1:06 PM, Sangbeom Kim sbkim73@samsung.com wrote:
+static struct device *dev;
Dear there is more to the first argument of dev_dbg, than to silence the compiler. Please extract _real_ device underneath samsung_idma, rather than using this placeholder.
+static const struct snd_pcm_hardware idma_hardware = {
- .info = SNDRV_PCM_INFO_INTERLEAVED |
- SNDRV_PCM_INFO_BLOCK_TRANSFER |
- SNDRV_PCM_INFO_MMAP |
- SNDRV_PCM_INFO_MMAP_VALID |
- SNDRV_PCM_INFO_PAUSE |
- SNDRV_PCM_INFO_RESUME,
- .formats = SNDRV_PCM_FMTBIT_S16_LE |
- SNDRV_PCM_FMTBIT_U16_LE |
- SNDRV_PCM_FMTBIT_S24_LE |
- SNDRV_PCM_FMTBIT_U24_LE |
- SNDRV_PCM_FMTBIT_U8 |
- SNDRV_PCM_FMTBIT_S8,
- .channels_min = 2,
- .channels_max = 2,
- .buffer_bytes_max = MAX_IDMA_BUFFER,
- .period_bytes_min = 128,
- .period_bytes_max = MAX_IDMA_PERIOD,
- .periods_min = 1,
- .periods_max = 2,
How about ouput of aply -v sample.wav ? Most probably ALSA uses 2 periods of MAX_IDMA_BUFFER/2 bytes each. Is that what you expect ? I would want ALSA to always use 1 period of MAX_IDMA_PERIOD bytes and ring-buffer of MAX_IDMA_BUFFER bytes.
+struct idma_ctrl {
- spinlock_t lock;
- int state;
- dma_addr_t start;
- dma_addr_t pos;
- dma_addr_t end;
This 'end' member is useless. Perhaps ringbuffer 'size' will be more useful.
- dma_addr_t period;
- dma_addr_t periodsz;
- void *token;
- void (*cb)(void *dt, int bytes_xfer);
+};
- if (res >= snd_pcm_lib_buffer_bytes(substream)) {
- res = 0;
- }
Dropping braces would be nice.
- addr += prtd->period;
- addr %= prtd->periodsz;
This is wrong. Perhaps you want... addr += prtd->periodsz; addr %= prtd->size; I doubt one could miss this error if the driver was tested.
+MODULE_AUTHOR("Jaswinder Singh, jassi.brar@samsung.com");
Sent emails will bounce from jassi.brar@samsung.com. Please use jassisinghbrar@gmail.com
+#define MAX_IDMA_PERIOD (105 * 1024) +#define MAX_IDMA_BUFFER (128 * 1024)
IIRC not all SoCs have same size iBuff ? Maybe passing via platform_data would be good ?