| Message ID | 20220706211330.120198-4-aidanmacdonald.0x0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: cleanups and improvements for jz4740-i2s | expand |
Hi Aidan, Le mer., juil. 6 2022 at 22:13:22 +0100, Aidan MacDonald <aidanmacdonald.0x0@gmail.com> a écrit : > Using regmap for accessing the AIC registers makes the driver a > little easier to read, and later refactors can take advantage of > regmap APIs to further simplify the driver. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > --- > sound/soc/jz4740/Kconfig | 1 + > sound/soc/jz4740/jz4740-i2s.c | 99 > +++++++++++++++-------------------- > 2 files changed, 42 insertions(+), 58 deletions(-) > > diff --git a/sound/soc/jz4740/Kconfig b/sound/soc/jz4740/Kconfig > index e72f826062e9..dd3b4507fbe6 100644 > --- a/sound/soc/jz4740/Kconfig > +++ b/sound/soc/jz4740/Kconfig > @@ -3,6 +3,7 @@ config SND_JZ4740_SOC_I2S > tristate "SoC Audio (I2S protocol) for Ingenic JZ4740 SoC" > depends on MIPS || COMPILE_TEST > depends on HAS_IOMEM > + select REGMAP_MMIO > select SND_SOC_GENERIC_DMAENGINE_PCM > help > Say Y if you want to use I2S protocol and I2S codec on Ingenic > JZ4740 > diff --git a/sound/soc/jz4740/jz4740-i2s.c > b/sound/soc/jz4740/jz4740-i2s.c > index ecd8df70d39c..66a901f56392 100644 > --- a/sound/soc/jz4740/jz4740-i2s.c > +++ b/sound/soc/jz4740/jz4740-i2s.c > @@ -9,6 +9,7 @@ > #include <linux/module.h> > #include <linux/mod_devicetable.h> > #include <linux/platform_device.h> > +#include <linux/regmap.h> > #include <linux/slab.h> > > #include <linux/clk.h> > @@ -94,7 +95,7 @@ struct i2s_soc_info { > > struct jz4740_i2s { > struct resource *mem; > - void __iomem *base; > + struct regmap *regmap; > > struct clk *clk_aic; > struct clk *clk_i2s; > @@ -105,39 +106,24 @@ struct jz4740_i2s { > const struct i2s_soc_info *soc_info; > }; > > -static inline uint32_t jz4740_i2s_read(const struct jz4740_i2s *i2s, > - unsigned int reg) > -{ > - return readl(i2s->base + reg); > -} > - > -static inline void jz4740_i2s_write(const struct jz4740_i2s *i2s, > - unsigned int reg, uint32_t value) > -{ > - writel(value, i2s->base + reg); > -} > - > static int jz4740_i2s_startup(struct snd_pcm_substream *substream, > struct snd_soc_dai *dai) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > - uint32_t conf, ctrl; > int ret; > > if (snd_soc_dai_active(dai)) > return 0; > > - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); > - ctrl |= JZ_AIC_CTRL_FLUSH; > - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); > + regmap_write_bits(i2s->regmap, JZ_REG_AIC_CTRL, > + JZ_AIC_CTRL_FLUSH, JZ_AIC_CTRL_FLUSH); I don't think you need regmap_write_bits() here, since there is no cache to bypass. You could use regmap_update_bits(), or even better, regmap_set_bits(). > > ret = clk_prepare_enable(i2s->clk_i2s); > if (ret) > return ret; > > - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); > - conf |= JZ_AIC_CONF_ENABLE; > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, > + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); Use regmap_set_bits() when you want to set all the bits of the mask. > > return 0; > } > @@ -146,14 +132,12 @@ static void jz4740_i2s_shutdown(struct > snd_pcm_substream *substream, > struct snd_soc_dai *dai) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > - uint32_t conf; > > if (snd_soc_dai_active(dai)) > return; > > - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); > - conf &= ~JZ_AIC_CONF_ENABLE; > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, > + JZ_AIC_CONF_ENABLE, 0); Use regmap_clear_bits() when you want to clear all bits of the mask. Otherwise, looks fairly good! Cheers, -Paul > > clk_disable_unprepare(i2s->clk_i2s); > } > @@ -162,8 +146,6 @@ static int jz4740_i2s_trigger(struct > snd_pcm_substream *substream, int cmd, > struct snd_soc_dai *dai) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > - > - uint32_t ctrl; > uint32_t mask; > > if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) > @@ -171,38 +153,30 @@ static int jz4740_i2s_trigger(struct > snd_pcm_substream *substream, int cmd, > else > mask = JZ_AIC_CTRL_ENABLE_CAPTURE | JZ_AIC_CTRL_ENABLE_RX_DMA; > > - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); > - > switch (cmd) { > case SNDRV_PCM_TRIGGER_START: > case SNDRV_PCM_TRIGGER_RESUME: > case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > - ctrl |= mask; > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, mask); > break; > case SNDRV_PCM_TRIGGER_STOP: > case SNDRV_PCM_TRIGGER_SUSPEND: > case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > - ctrl &= ~mask; > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, 0); > break; > default: > return -EINVAL; > } > > - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); > - > return 0; > } > > static int jz4740_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int > fmt) > { > struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); > - > - uint32_t format = 0; > - uint32_t conf; > - > - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); > - > - conf &= ~(JZ_AIC_CONF_BIT_CLK_MASTER | JZ_AIC_CONF_SYNC_CLK_MASTER); > + const unsigned int conf_mask = JZ_AIC_CONF_BIT_CLK_MASTER | > + JZ_AIC_CONF_SYNC_CLK_MASTER; > + unsigned int conf = 0, format = 0; > > switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) { > case SND_SOC_DAIFMT_BP_FP: > @@ -238,8 +212,8 @@ static int jz4740_i2s_set_fmt(struct snd_soc_dai > *dai, unsigned int fmt) > return -EINVAL; > } > > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > - jz4740_i2s_write(i2s, JZ_REG_AIC_I2S_FMT, format); > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, conf_mask, conf); > + regmap_write(i2s->regmap, JZ_REG_AIC_I2S_FMT, format); > > return 0; > } > @@ -252,9 +226,9 @@ static int jz4740_i2s_hw_params(struct > snd_pcm_substream *substream, > uint32_t ctrl, div_reg; > int div; > > - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); > + regmap_read(i2s->regmap, JZ_REG_AIC_CTRL, &ctrl); > + regmap_read(i2s->regmap, JZ_REG_AIC_CLK_DIV, &div_reg); > > - div_reg = jz4740_i2s_read(i2s, JZ_REG_AIC_CLK_DIV); > div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params)); > > switch (params_format(params)) { > @@ -291,8 +265,8 @@ static int jz4740_i2s_hw_params(struct > snd_pcm_substream *substream, > } > } > > - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); > - jz4740_i2s_write(i2s, JZ_REG_AIC_CLK_DIV, div_reg); > + regmap_write(i2s->regmap, JZ_REG_AIC_CTRL, ctrl); > + regmap_write(i2s->regmap, JZ_REG_AIC_CLK_DIV, div_reg); > > return 0; > } > @@ -329,12 +303,10 @@ static int jz4740_i2s_set_sysclk(struct > snd_soc_dai *dai, int clk_id, > static int jz4740_i2s_suspend(struct snd_soc_component *component) > { > struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); > - uint32_t conf; > > if (snd_soc_component_active(component)) { > - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); > - conf &= ~JZ_AIC_CONF_ENABLE; > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, > + JZ_AIC_CONF_ENABLE, 0); > > clk_disable_unprepare(i2s->clk_i2s); > } > @@ -347,7 +319,6 @@ static int jz4740_i2s_suspend(struct > snd_soc_component *component) > static int jz4740_i2s_resume(struct snd_soc_component *component) > { > struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); > - uint32_t conf; > int ret; > > ret = clk_prepare_enable(i2s->clk_aic); > @@ -361,9 +332,8 @@ static int jz4740_i2s_resume(struct > snd_soc_component *component) > return ret; > } > > - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); > - conf |= JZ_AIC_CONF_ENABLE; > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, > + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); > } > > return 0; > @@ -396,8 +366,8 @@ static int jz4740_i2s_dai_probe(struct > snd_soc_dai *dai) > JZ_AIC_CONF_INTERNAL_CODEC; > } > > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); > - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); > + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); > + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, conf); > > return 0; > } > @@ -495,11 +465,19 @@ static const struct of_device_id > jz4740_of_matches[] = { > }; > MODULE_DEVICE_TABLE(of, jz4740_of_matches); > > +static const struct regmap_config jz4740_i2s_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .max_register = JZ_REG_AIC_FIFO, > +}; > + > static int jz4740_i2s_dev_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct jz4740_i2s *i2s; > struct resource *mem; > + void __iomem *regs; > int ret; > > i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL); > @@ -508,9 +486,9 @@ static int jz4740_i2s_dev_probe(struct > platform_device *pdev) > > i2s->soc_info = device_get_match_data(dev); > > - i2s->base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); > - if (IS_ERR(i2s->base)) > - return PTR_ERR(i2s->base); > + regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); > + if (IS_ERR(regs)) > + return PTR_ERR(regs); > > i2s->playback_dma_data.maxburst = 16; > i2s->playback_dma_data.addr = mem->start + JZ_REG_AIC_FIFO; > @@ -526,6 +504,11 @@ static int jz4740_i2s_dev_probe(struct > platform_device *pdev) > if (IS_ERR(i2s->clk_i2s)) > return PTR_ERR(i2s->clk_i2s); > > + i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs, > + &jz4740_i2s_regmap_config); > + if (IS_ERR(i2s->regmap)) > + return PTR_ERR(i2s->regmap); > + > platform_set_drvdata(pdev, i2s); > > ret = devm_snd_soc_register_component(dev, &jz4740_i2s_component, > -- > 2.35.1 >
Paul Cercueil <paul@crapouillou.net> writes: > Hi Aidan, > > Le mer., juil. 6 2022 at 22:13:22 +0100, Aidan MacDonald > <aidanmacdonald.0x0@gmail.com> a écrit : >> Using regmap for accessing the AIC registers makes the driver a >> little easier to read, and later refactors can take advantage of >> regmap APIs to further simplify the driver. >> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> >> --- >> sound/soc/jz4740/Kconfig | 1 + >> sound/soc/jz4740/jz4740-i2s.c | 99 +++++++++++++++-------------------- >> 2 files changed, 42 insertions(+), 58 deletions(-) >> diff --git a/sound/soc/jz4740/Kconfig b/sound/soc/jz4740/Kconfig >> index e72f826062e9..dd3b4507fbe6 100644 >> --- a/sound/soc/jz4740/Kconfig >> +++ b/sound/soc/jz4740/Kconfig >> @@ -3,6 +3,7 @@ config SND_JZ4740_SOC_I2S >> tristate "SoC Audio (I2S protocol) for Ingenic JZ4740 SoC" >> depends on MIPS || COMPILE_TEST >> depends on HAS_IOMEM >> + select REGMAP_MMIO >> select SND_SOC_GENERIC_DMAENGINE_PCM >> help >> Say Y if you want to use I2S protocol and I2S codec on Ingenic JZ4740 >> diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c >> index ecd8df70d39c..66a901f56392 100644 >> --- a/sound/soc/jz4740/jz4740-i2s.c >> +++ b/sound/soc/jz4740/jz4740-i2s.c >> @@ -9,6 +9,7 @@ >> #include <linux/module.h> >> #include <linux/mod_devicetable.h> >> #include <linux/platform_device.h> >> +#include <linux/regmap.h> >> #include <linux/slab.h> >> #include <linux/clk.h> >> @@ -94,7 +95,7 @@ struct i2s_soc_info { >> struct jz4740_i2s { >> struct resource *mem; >> - void __iomem *base; >> + struct regmap *regmap; >> struct clk *clk_aic; >> struct clk *clk_i2s; >> @@ -105,39 +106,24 @@ struct jz4740_i2s { >> const struct i2s_soc_info *soc_info; >> }; >> -static inline uint32_t jz4740_i2s_read(const struct jz4740_i2s *i2s, >> - unsigned int reg) >> -{ >> - return readl(i2s->base + reg); >> -} >> - >> -static inline void jz4740_i2s_write(const struct jz4740_i2s *i2s, >> - unsigned int reg, uint32_t value) >> -{ >> - writel(value, i2s->base + reg); >> -} >> - >> static int jz4740_i2s_startup(struct snd_pcm_substream *substream, >> struct snd_soc_dai *dai) >> { >> struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); >> - uint32_t conf, ctrl; >> int ret; >> if (snd_soc_dai_active(dai)) >> return 0; >> - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); >> - ctrl |= JZ_AIC_CTRL_FLUSH; >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); >> + regmap_write_bits(i2s->regmap, JZ_REG_AIC_CTRL, >> + JZ_AIC_CTRL_FLUSH, JZ_AIC_CTRL_FLUSH); > > I don't think you need regmap_write_bits() here, since there is no cache to > bypass. You could use regmap_update_bits(), or even better, regmap_set_bits(). > write_bits isn't _exactly_ just a cache bypass operation -- it means "write the register even if the value is the same as what was read." An update_bits doesn't necessarily perform a register write, even if there is no cache and the register is volatile. The distinction shouldn't matter here, since the flush bit is supposed to be self-clearing. So I might as well use regmap_set_bits(). Also: I just noticed this will need to be a regmap field. It seems that all SoCs newer than jz4740 have separate transmit/receive flush bits. At least the JZ4760, JZ4780, and X1000 manuals say as much. Not sure about the JZ4770 since I don't have any documentation for that SoC. >> ret = clk_prepare_enable(i2s->clk_i2s); >> if (ret) >> return ret; >> - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); >> - conf |= JZ_AIC_CONF_ENABLE; >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, >> + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); > > Use regmap_set_bits() when you want to set all the bits of the mask. > >> return 0; >> } >> @@ -146,14 +132,12 @@ static void jz4740_i2s_shutdown(struct >> snd_pcm_substream *substream, >> struct snd_soc_dai *dai) >> { >> struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); >> - uint32_t conf; >> if (snd_soc_dai_active(dai)) >> return; >> - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); >> - conf &= ~JZ_AIC_CONF_ENABLE; >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, >> + JZ_AIC_CONF_ENABLE, 0); > > Use regmap_clear_bits() when you want to clear all bits of the mask. > > Otherwise, looks fairly good! > > Cheers, > -Paul > Thanks, I didn't know about set/clear bits but that'll make it even simpler. >> clk_disable_unprepare(i2s->clk_i2s); >> } >> @@ -162,8 +146,6 @@ static int jz4740_i2s_trigger(struct snd_pcm_substream >> *substream, int cmd, >> struct snd_soc_dai *dai) >> { >> struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); >> - >> - uint32_t ctrl; >> uint32_t mask; >> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) >> @@ -171,38 +153,30 @@ static int jz4740_i2s_trigger(struct snd_pcm_substream >> *substream, int cmd, >> else >> mask = JZ_AIC_CTRL_ENABLE_CAPTURE | JZ_AIC_CTRL_ENABLE_RX_DMA; >> - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); >> - >> switch (cmd) { >> case SNDRV_PCM_TRIGGER_START: >> case SNDRV_PCM_TRIGGER_RESUME: >> case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: >> - ctrl |= mask; >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, mask); >> break; >> case SNDRV_PCM_TRIGGER_STOP: >> case SNDRV_PCM_TRIGGER_SUSPEND: >> case SNDRV_PCM_TRIGGER_PAUSE_PUSH: >> - ctrl &= ~mask; >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, 0); >> break; >> default: >> return -EINVAL; >> } >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); >> - >> return 0; >> } >> static int jz4740_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) >> { >> struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); >> - >> - uint32_t format = 0; >> - uint32_t conf; >> - >> - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); >> - >> - conf &= ~(JZ_AIC_CONF_BIT_CLK_MASTER | JZ_AIC_CONF_SYNC_CLK_MASTER); >> + const unsigned int conf_mask = JZ_AIC_CONF_BIT_CLK_MASTER | >> + JZ_AIC_CONF_SYNC_CLK_MASTER; >> + unsigned int conf = 0, format = 0; >> switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) { >> case SND_SOC_DAIFMT_BP_FP: >> @@ -238,8 +212,8 @@ static int jz4740_i2s_set_fmt(struct snd_soc_dai *dai, >> unsigned int fmt) >> return -EINVAL; >> } >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> - jz4740_i2s_write(i2s, JZ_REG_AIC_I2S_FMT, format); >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, conf_mask, conf); >> + regmap_write(i2s->regmap, JZ_REG_AIC_I2S_FMT, format); >> return 0; >> } >> @@ -252,9 +226,9 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream >> *substream, >> uint32_t ctrl, div_reg; >> int div; >> - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); >> + regmap_read(i2s->regmap, JZ_REG_AIC_CTRL, &ctrl); >> + regmap_read(i2s->regmap, JZ_REG_AIC_CLK_DIV, &div_reg); >> - div_reg = jz4740_i2s_read(i2s, JZ_REG_AIC_CLK_DIV); >> div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params)); >> switch (params_format(params)) { >> @@ -291,8 +265,8 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream >> *substream, >> } >> } >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CLK_DIV, div_reg); >> + regmap_write(i2s->regmap, JZ_REG_AIC_CTRL, ctrl); >> + regmap_write(i2s->regmap, JZ_REG_AIC_CLK_DIV, div_reg); >> return 0; >> } >> @@ -329,12 +303,10 @@ static int jz4740_i2s_set_sysclk(struct snd_soc_dai >> *dai, int clk_id, >> static int jz4740_i2s_suspend(struct snd_soc_component *component) >> { >> struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); >> - uint32_t conf; >> if (snd_soc_component_active(component)) { >> - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); >> - conf &= ~JZ_AIC_CONF_ENABLE; >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, >> + JZ_AIC_CONF_ENABLE, 0); >> clk_disable_unprepare(i2s->clk_i2s); >> } >> @@ -347,7 +319,6 @@ static int jz4740_i2s_suspend(struct snd_soc_component >> *component) >> static int jz4740_i2s_resume(struct snd_soc_component *component) >> { >> struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); >> - uint32_t conf; >> int ret; >> ret = clk_prepare_enable(i2s->clk_aic); >> @@ -361,9 +332,8 @@ static int jz4740_i2s_resume(struct snd_soc_component >> *component) >> return ret; >> } >> - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); >> - conf |= JZ_AIC_CONF_ENABLE; >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, >> + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); >> } >> return 0; >> @@ -396,8 +366,8 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) >> JZ_AIC_CONF_INTERNAL_CODEC; >> } >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); >> - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); >> + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); >> + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, conf); >> return 0; >> } >> @@ -495,11 +465,19 @@ static const struct of_device_id jz4740_of_matches[] = >> { >> }; >> MODULE_DEVICE_TABLE(of, jz4740_of_matches); >> +static const struct regmap_config jz4740_i2s_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 32, >> + .max_register = JZ_REG_AIC_FIFO, >> +}; >> + >> static int jz4740_i2s_dev_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct jz4740_i2s *i2s; >> struct resource *mem; >> + void __iomem *regs; >> int ret; >> i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL); >> @@ -508,9 +486,9 @@ static int jz4740_i2s_dev_probe(struct platform_device >> *pdev) >> i2s->soc_info = device_get_match_data(dev); >> - i2s->base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); >> - if (IS_ERR(i2s->base)) >> - return PTR_ERR(i2s->base); >> + regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); >> + if (IS_ERR(regs)) >> + return PTR_ERR(regs); >> i2s->playback_dma_data.maxburst = 16; >> i2s->playback_dma_data.addr = mem->start + JZ_REG_AIC_FIFO; >> @@ -526,6 +504,11 @@ static int jz4740_i2s_dev_probe(struct platform_device >> *pdev) >> if (IS_ERR(i2s->clk_i2s)) >> return PTR_ERR(i2s->clk_i2s); >> + i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs, >> + &jz4740_i2s_regmap_config); >> + if (IS_ERR(i2s->regmap)) >> + return PTR_ERR(i2s->regmap); >> + >> platform_set_drvdata(pdev, i2s); >> ret = devm_snd_soc_register_component(dev, &jz4740_i2s_component, >> -- >> 2.35.1 >>
diff --git a/sound/soc/jz4740/Kconfig b/sound/soc/jz4740/Kconfig index e72f826062e9..dd3b4507fbe6 100644 --- a/sound/soc/jz4740/Kconfig +++ b/sound/soc/jz4740/Kconfig @@ -3,6 +3,7 @@ config SND_JZ4740_SOC_I2S tristate "SoC Audio (I2S protocol) for Ingenic JZ4740 SoC" depends on MIPS || COMPILE_TEST depends on HAS_IOMEM + select REGMAP_MMIO select SND_SOC_GENERIC_DMAENGINE_PCM help Say Y if you want to use I2S protocol and I2S codec on Ingenic JZ4740 diff --git a/sound/soc/jz4740/jz4740-i2s.c b/sound/soc/jz4740/jz4740-i2s.c index ecd8df70d39c..66a901f56392 100644 --- a/sound/soc/jz4740/jz4740-i2s.c +++ b/sound/soc/jz4740/jz4740-i2s.c @@ -9,6 +9,7 @@ #include <linux/module.h> #include <linux/mod_devicetable.h> #include <linux/platform_device.h> +#include <linux/regmap.h> #include <linux/slab.h> #include <linux/clk.h> @@ -94,7 +95,7 @@ struct i2s_soc_info { struct jz4740_i2s { struct resource *mem; - void __iomem *base; + struct regmap *regmap; struct clk *clk_aic; struct clk *clk_i2s; @@ -105,39 +106,24 @@ struct jz4740_i2s { const struct i2s_soc_info *soc_info; }; -static inline uint32_t jz4740_i2s_read(const struct jz4740_i2s *i2s, - unsigned int reg) -{ - return readl(i2s->base + reg); -} - -static inline void jz4740_i2s_write(const struct jz4740_i2s *i2s, - unsigned int reg, uint32_t value) -{ - writel(value, i2s->base + reg); -} - static int jz4740_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); - uint32_t conf, ctrl; int ret; if (snd_soc_dai_active(dai)) return 0; - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); - ctrl |= JZ_AIC_CTRL_FLUSH; - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); + regmap_write_bits(i2s->regmap, JZ_REG_AIC_CTRL, + JZ_AIC_CTRL_FLUSH, JZ_AIC_CTRL_FLUSH); ret = clk_prepare_enable(i2s->clk_i2s); if (ret) return ret; - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); - conf |= JZ_AIC_CONF_ENABLE; - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); return 0; } @@ -146,14 +132,12 @@ static void jz4740_i2s_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); - uint32_t conf; if (snd_soc_dai_active(dai)) return; - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); - conf &= ~JZ_AIC_CONF_ENABLE; - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, + JZ_AIC_CONF_ENABLE, 0); clk_disable_unprepare(i2s->clk_i2s); } @@ -162,8 +146,6 @@ static int jz4740_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); - - uint32_t ctrl; uint32_t mask; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) @@ -171,38 +153,30 @@ static int jz4740_i2s_trigger(struct snd_pcm_substream *substream, int cmd, else mask = JZ_AIC_CTRL_ENABLE_CAPTURE | JZ_AIC_CTRL_ENABLE_RX_DMA; - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); - switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - ctrl |= mask; + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, mask); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - ctrl &= ~mask; + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CTRL, mask, 0); break; default: return -EINVAL; } - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); - return 0; } static int jz4740_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { struct jz4740_i2s *i2s = snd_soc_dai_get_drvdata(dai); - - uint32_t format = 0; - uint32_t conf; - - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); - - conf &= ~(JZ_AIC_CONF_BIT_CLK_MASTER | JZ_AIC_CONF_SYNC_CLK_MASTER); + const unsigned int conf_mask = JZ_AIC_CONF_BIT_CLK_MASTER | + JZ_AIC_CONF_SYNC_CLK_MASTER; + unsigned int conf = 0, format = 0; switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) { case SND_SOC_DAIFMT_BP_FP: @@ -238,8 +212,8 @@ static int jz4740_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return -EINVAL; } - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); - jz4740_i2s_write(i2s, JZ_REG_AIC_I2S_FMT, format); + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, conf_mask, conf); + regmap_write(i2s->regmap, JZ_REG_AIC_I2S_FMT, format); return 0; } @@ -252,9 +226,9 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream, uint32_t ctrl, div_reg; int div; - ctrl = jz4740_i2s_read(i2s, JZ_REG_AIC_CTRL); + regmap_read(i2s->regmap, JZ_REG_AIC_CTRL, &ctrl); + regmap_read(i2s->regmap, JZ_REG_AIC_CLK_DIV, &div_reg); - div_reg = jz4740_i2s_read(i2s, JZ_REG_AIC_CLK_DIV); div = clk_get_rate(i2s->clk_i2s) / (64 * params_rate(params)); switch (params_format(params)) { @@ -291,8 +265,8 @@ static int jz4740_i2s_hw_params(struct snd_pcm_substream *substream, } } - jz4740_i2s_write(i2s, JZ_REG_AIC_CTRL, ctrl); - jz4740_i2s_write(i2s, JZ_REG_AIC_CLK_DIV, div_reg); + regmap_write(i2s->regmap, JZ_REG_AIC_CTRL, ctrl); + regmap_write(i2s->regmap, JZ_REG_AIC_CLK_DIV, div_reg); return 0; } @@ -329,12 +303,10 @@ static int jz4740_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id, static int jz4740_i2s_suspend(struct snd_soc_component *component) { struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); - uint32_t conf; if (snd_soc_component_active(component)) { - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); - conf &= ~JZ_AIC_CONF_ENABLE; - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, + JZ_AIC_CONF_ENABLE, 0); clk_disable_unprepare(i2s->clk_i2s); } @@ -347,7 +319,6 @@ static int jz4740_i2s_suspend(struct snd_soc_component *component) static int jz4740_i2s_resume(struct snd_soc_component *component) { struct jz4740_i2s *i2s = snd_soc_component_get_drvdata(component); - uint32_t conf; int ret; ret = clk_prepare_enable(i2s->clk_aic); @@ -361,9 +332,8 @@ static int jz4740_i2s_resume(struct snd_soc_component *component) return ret; } - conf = jz4740_i2s_read(i2s, JZ_REG_AIC_CONF); - conf |= JZ_AIC_CONF_ENABLE; - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); + regmap_update_bits(i2s->regmap, JZ_REG_AIC_CONF, + JZ_AIC_CONF_ENABLE, JZ_AIC_CONF_ENABLE); } return 0; @@ -396,8 +366,8 @@ static int jz4740_i2s_dai_probe(struct snd_soc_dai *dai) JZ_AIC_CONF_INTERNAL_CODEC; } - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); - jz4740_i2s_write(i2s, JZ_REG_AIC_CONF, conf); + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, JZ_AIC_CONF_RESET); + regmap_write(i2s->regmap, JZ_REG_AIC_CONF, conf); return 0; } @@ -495,11 +465,19 @@ static const struct of_device_id jz4740_of_matches[] = { }; MODULE_DEVICE_TABLE(of, jz4740_of_matches); +static const struct regmap_config jz4740_i2s_regmap_config = { + .reg_bits = 32, + .reg_stride = 4, + .val_bits = 32, + .max_register = JZ_REG_AIC_FIFO, +}; + static int jz4740_i2s_dev_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct jz4740_i2s *i2s; struct resource *mem; + void __iomem *regs; int ret; i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL); @@ -508,9 +486,9 @@ static int jz4740_i2s_dev_probe(struct platform_device *pdev) i2s->soc_info = device_get_match_data(dev); - i2s->base = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); - if (IS_ERR(i2s->base)) - return PTR_ERR(i2s->base); + regs = devm_platform_get_and_ioremap_resource(pdev, 0, &mem); + if (IS_ERR(regs)) + return PTR_ERR(regs); i2s->playback_dma_data.maxburst = 16; i2s->playback_dma_data.addr = mem->start + JZ_REG_AIC_FIFO; @@ -526,6 +504,11 @@ static int jz4740_i2s_dev_probe(struct platform_device *pdev) if (IS_ERR(i2s->clk_i2s)) return PTR_ERR(i2s->clk_i2s); + i2s->regmap = devm_regmap_init_mmio(&pdev->dev, regs, + &jz4740_i2s_regmap_config); + if (IS_ERR(i2s->regmap)) + return PTR_ERR(i2s->regmap); + platform_set_drvdata(pdev, i2s); ret = devm_snd_soc_register_component(dev, &jz4740_i2s_component,
Using regmap for accessing the AIC registers makes the driver a little easier to read, and later refactors can take advantage of regmap APIs to further simplify the driver. Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> --- sound/soc/jz4740/Kconfig | 1 + sound/soc/jz4740/jz4740-i2s.c | 99 +++++++++++++++-------------------- 2 files changed, 42 insertions(+), 58 deletions(-)