On Tue, Apr 10, 2012 at 01:16:05PM -0600, Stephen Warren wrote: > From: Stephen Warren > > For the Tegra I2S, SPDIF, and DAS drivers > > Signed-off-by: Stephen Warren > --- > Mark, this depends on the regmap->ASoC git pull request that I just sent. ...which would've been much better placed here so they don't get misplaced. > sound/soc/tegra/Kconfig | 1 + > sound/soc/tegra/tegra20_das.c | 99 ++++++++-------------- > sound/soc/tegra/tegra20_das.h | 3 +- > sound/soc/tegra/tegra20_i2s.c | 149 ++++++++++++++++----------------- > sound/soc/tegra/tegra20_i2s.h | 3 +- > sound/soc/tegra/tegra20_spdif.c | 177 ++++++++++++++++++++------------------- > sound/soc/tegra/tegra20_spdif.h | 3 +- It would have have been useful to split this into three smaller patches, the I2S and S/PDIF changes look good but there's one funny in the DAS driver. > +#define LAST_REG(name) \ > + (TEGRA20_DAS_##name + \ > + (TEGRA20_DAS_##name##_STRIDE * TEGRA20_DAS_##name##_COUNT) - 4) > +#define REG_IN_ARRAY(reg, name) \ > + ((reg >= TEGRA20_DAS_##name) && \ > + (reg <= LAST_REG(name) && \ > + (!((reg - TEGRA20_DAS_##name) % TEGRA20_DAS_##name##_STRIDE)))) > +static bool tegra20_das_wr_rd_reg(struct device *dev, unsigned int reg) > { > + if (REG_IN_ARRAY(reg, DAP_CTRL_SEL) || > + REG_IN_ARRAY(reg, DAC_INPUT_DATA_CLK_SEL)) > + return true; This seems a little too obscure for what's normally a very simple function. REG_IN_ARRAY() in particular seems odd, what's the array in question and why do we need to do anything more than a simple mod to check if the register is correctly aligned? Some comments seem to be in order at least, or just expanding REG_IN_ARRAY - there's only two uses.