At Thu, 01 Jun 2006 13:58:48 +0200,
Johannes Berg wrote:
>
> --- /dev/null
> +++ b/sound/aoa/soundbus/i2sbus/i2sbus-pcm.c
> +static int clock_and_divisors(int mclk, int sclk, int rate, int *out)
> +{
> + /* sclk must be derived from mclk! */
> + if (mclk % sclk)
> + return -1;
> + /* derive sclk register value */
> + if (i2s_sf_sclkdiv(mclk / sclk, out))
> + return -1;
> +
> + if (I2S_CLOCK_SPEED_18MHz % rate == 0) {
> + if ((I2S_CLOCK_SPEED_18MHz / rate) % mclk == 0) {
Equivalent with "I2S_CLOCK_SPEED_18MHZ % (rate * mclk) == 0" ?
> +static int i2sbus_pcm_open(struct i2sbus_dev *i2sdev, int in)
> +{
(snip)
> + list_for_each_entry(cii, &sdev->codec_list, list) {
> + if (cii->codec->open) {
> + err = cii->codec->open(cii, pi->substream);
> + if (err) {
> + result = err;
> + goto out_unlock;
What happens if the first code is opened but fail the secondary?
No need to close the first?
> +static snd_pcm_uframes_t i2sbus_pcm_pointer(struct i2sbus_dev *i2sdev, int in)
> +{
> + struct pcm_info *pi;
> + u32 fc;
> +
> + get_pcm_info(i2sdev, in, &pi, NULL);
> +
> + fc = in_le32(&i2sdev->intfregs->frame_count);
> + fc = fc - pi->frame_count;
> +
> + return (bytes_to_frames(pi->substream->runtime,
> + pi->current_period *
> + snd_pcm_lib_period_bytes(pi->substream)) + fc) % pi->substream->runtime->buffer_size;
> +}
> +
> +static inline void handle_interrupt(struct i2sbus_dev *i2sdev, int in)
> +{
> + struct pcm_info *pi;
> + u32 fc;
> + u32 delta;
> +
> + spin_lock(&i2sdev->low_lock);
> + get_pcm_info(i2sdev, in, &pi, NULL);
> + if (!pi->substream) {
> + printk(KERN_INFO "i2sbus: got %s irq while not active!\n",
> + in ? "rx" : "tx");
> + goto out_unlock;
> + }
> +
> + fc = in_le32(&i2sdev->intfregs->frame_count);
> + /* a counter overflow does not change the calculation. */
> + delta = fc - pi->frame_count;
> +
> + /* update current_period */
> + while (delta >= pi->substream->runtime->period_size) {
> + pi->current_period++;
> + delta = delta - pi->substream->runtime->period_size;
> + }
> +
> + if (unlikely(delta)) {
> + /* Some interrupt came late, so check the dbdma.
> + * This special case exists to syncronize the frame_count with the
> + * dbdma transfers, but is hit every once in a while. */
> + int period;
> +
> + period = (in_le32(&pi->dbdma->cmdptr) - pi->dbdma_ring.bus_cmd_start) / sizeof(struct dbdma_cmd);
> + pi->current_period = pi->current_period % pi->substream->runtime->periods;
> +
> + while (pi->current_period != period) {
> + pi->current_period = (pi->current_period + 1) % pi->substream->runtime->periods;
> + /* Set delta to zero, as the frame_count value is too high (otherwise the code path
> + * will not be executed).
> + * This is to correct the fact that the frame_count is too low at the beginning
> + * due to the dbdma's buffer. */
> + delta = 0;
Too long lines...
> +/* FIXME: this function needs an error handling strategy with labels */
> +int
> +i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card,
> + struct codec_info *ci, void *data)
> +{
> + int err, in = 0, out = 0;
> + struct transfer_info *tmp;
> + struct i2sbus_dev *i2sdev = soundbus_dev_to_i2sbus_dev(dev);
> + struct codec_info_item *cii;
> +
> + if (!dev->pcmname || dev->pcmid == -1) {
> + printk(KERN_ERR "i2sbus: pcm name and id must be set!\n");
No error return?
> + /* well, we really should support scatter/gather DMA */
> + /* FIXME FIXME FIXME: If this fails, we BUG() when the alsa layer
> + * later tries to allocate memory. Apparently we should be setting
> + * some device pointer for that ...
> + */
> + snd_pcm_lib_preallocate_pages_for_all(
> + dev->pcm, SNDRV_DMA_TYPE_DEV,
> + snd_dma_pci_data(macio_get_pci_dev(i2sdev->macio)),
> + 64 * 1024, 64 * 1024);
Is the comment true? Yes, you have to set the device pointer via
snd_pcm_lib_preallocate*(). But it must be OK even if preallocate
fails.
> --- /dev/null
> +++ b/sound/aoa/soundbus/i2sbus/i2sbus-core.c
> +static int alloc_dbdma_descriptor_ring(struct i2sbus_dev *i2sdev,
> + struct dbdma_command_mem *r,
> + int numcmds)
> +{
> + /* one more for rounding */
> + r->size = (numcmds+1) * sizeof(struct dbdma_cmd);
> + /* We use the PCI APIs for now until the generic one gets fixed
> + * enough or until we get some macio-specific versions
> + */
> + r->space = pci_alloc_consistent(macio_get_pci_dev(i2sdev->macio),
> + r->size,
> + &r->bus_addr);
Better to use dma_alloc_coherent(). pci_alloc_consistent() implies
GFP_ATOMIC.
> +static irqreturn_t i2sbus_bus_intr(int irq, void *devid, struct pt_regs *regs)
> +{
> + struct i2sbus_dev *dev = devid;
> + u32 intreg;
> +
> + spin_lock(&dev->low_lock);
> + intreg = in_le32(&dev->intfregs->intr_ctl);
> +
> + printk(KERN_INFO "i2sbus: interrupt, intr reg is 0x%x!\n", intreg);
Should this be really always printed?
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
