On Mon, Mar 19, 2012 at 09:00:59PM +0900, Tomoya MORINAGA wrote:
This driver is for LAPIS Semiconductor ML7213 IOH I2S.
So, I just said to the SPEAr guys that I really want to see support for non-cyclic dmaengine added to the helper library rather than open coding. We're seeing lots of platforms moving to dmaengine (as well as those that are in already yours is one of three out of tree platforms I'm aware of that have actively submitted stuff) and with this number of platforms we really need to get stuff factored out - there's enough people working on these platforms that it should be possible to add the support to the dmaengine library code.
A few relatively minor comments below and due to the above I haven't really reviwed the DMA but overall this looks pretty good.
+static struct ioh_i2s_data *i2s_data; +static struct ioh_i2s_dma *dmadata;
These shouldn't be global, they should be device specific and passed around.
+static int ml7213i2s_dai_set_dai_sysclk(struct snd_soc_dai *dai,
int clk_id, unsigned int freq, int dir)+{
- u32 reg[MAX_I2S_CH];
- void *iobase = i2s_data->iobase;
- int i;
- for (i = 0; i < MAX_I2S_CH; i++) {
reg[i] = ioread32(iobase + I2SCLKCNT0_OFFSET + 0x10 * i);if (clk_id == IOH_MASTERCLKSEL_MCLK)reg[i] &= ~ML7213I2S_MASTER_CLK_SEL;else if (clk_id == IOH_MASTERCLKSEL_MLBCLK)reg[i] |= ML7213I2S_MASTER_CLK_SEL;iowrite32(reg[i], iobase + I2SCLKCNT0_OFFSET + 0x10 * i);
This looks like it should be a switch statement on clk_id, this will also catch bad parameters that might get passed in.
if ((slot < MAX_I2S_CH) &&(!(tx_mapped & (1 << slot)))) {dmadata[i].mapped_tx_ch = slot;tx_mapped |= 1 << slot;} elsereturn -EINVAL;
Braces on both sides of the if please.