| Message ID | 1444653637-14711-3-git-send-email-Damien.Horsley@imgtec.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 12, 2015 at 01:40:29PM +0100, Damien Horsley wrote: > +static inline u32 img_i2s_in_ch_disable(struct img_i2s_in *i2s, u32 chan) > +{ > + u32 reg; > + > + reg = img_i2s_in_ch_readl(i2s, chan, IMG_I2S_IN_CH_CTL); > + reg &= ~IMG_I2S_IN_CH_CTL_ME_MASK; > + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); > + > + return reg; > +} > + > +static inline void img_i2s_in_ch_enable(struct img_i2s_in *i2s, u32 chan, > + u32 reg) > +{ > + reg |= IMG_I2S_IN_CH_CTL_ME_MASK; > + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); > +} The APIs here all seem a bit odd - for example the enable API taking a register value as an argument (normally reg is a register address BTW) and returning a value but the disable API doing a read/modify/write cycle. > +static inline void img_i2s_in_flush(struct img_i2s_in *i2s) > +{ > + int i; > + u32 reg; > + > + for (i = 0; i < i2s->active_channels; i++) { > + reg = img_i2s_in_ch_disable(i2s, i); > + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; > + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); > + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; > + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); > + img_i2s_in_ch_enable(i2s, i, reg); > + } > +} This all seems to be connected to this, which is itself slightly funky especially in the context of the only user... > + case SNDRV_PCM_TRIGGER_STOP: > + case SNDRV_PCM_TRIGGER_SUSPEND: > + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); > + reg &= ~IMG_I2S_IN_CTL_ME_MASK; > + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); > + img_i2s_in_flush(i2s); > + break; ...which looks like it'll enable everything, then disable and reenable. Plus needing to do a flush on trigger seems weird. > + if ((channels < 2) || > + (channels > (i2s->max_i2s_chan * 2)) || > + (channels % 2)) > + return -EINVAL; This indentation is very weird. > + control_mask = (u32)(~IMG_I2S_IN_CTL_16PACK_MASK & > + ~IMG_I2S_IN_CTL_ACTIVE_CHAN_MASK); > + chan_control_mask = (u32)(~IMG_I2S_IN_CH_CTL_16PACK_MASK & > + ~IMG_I2S_IN_CH_CTL_FEN_MASK & > + ~IMG_I2S_IN_CH_CTL_FMODE_MASK & > + ~IMG_I2S_IN_CH_CTL_SW_MASK & > + ~IMG_I2S_IN_CH_CTL_FW_MASK & > + ~IMG_I2S_IN_CH_CTL_PACKH_MASK); This also looks very odd. Normally we'd write masks as being the valid bits and or them together. > + i2s->clk_sys = devm_clk_get(dev, "sys"); > + if (IS_ERR(i2s->clk_sys)) > + return PTR_ERR(i2s->clk_sys); Please print an error message so people can tell why things failed. > + rst = devm_reset_control_get(dev, "rst"); > + if (IS_ERR(rst)) { > + dev_dbg(dev, "No top level reset found\n"); You should check for -EPROBE_DEFER here and just return the error here if you get it (on the basis that the reset framework ought to be using a different error if there's nothing bound in DT).
On 19/10/15 18:47, Mark Brown wrote: > On Mon, Oct 12, 2015 at 01:40:29PM +0100, Damien Horsley wrote: > >> +static inline u32 img_i2s_in_ch_disable(struct img_i2s_in *i2s, u32 chan) >> +{ >> + u32 reg; >> + >> + reg = img_i2s_in_ch_readl(i2s, chan, IMG_I2S_IN_CH_CTL); >> + reg &= ~IMG_I2S_IN_CH_CTL_ME_MASK; >> + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); >> + >> + return reg; >> +} >> + >> +static inline void img_i2s_in_ch_enable(struct img_i2s_in *i2s, u32 chan, >> + u32 reg) >> +{ >> + reg |= IMG_I2S_IN_CH_CTL_ME_MASK; >> + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); >> +} > > The APIs here all seem a bit odd - for example the enable API taking a > register value as an argument (normally reg is a register address BTW) > and returning a value but the disable API doing a read/modify/write > cycle. > Sure. It reduces the number of register accesses this way, but the difference in execution time is not significant. Would you prefer these to both do read-modify-writes? >> +static inline void img_i2s_in_flush(struct img_i2s_in *i2s) >> +{ >> + int i; >> + u32 reg; >> + >> + for (i = 0; i < i2s->active_channels; i++) { >> + reg = img_i2s_in_ch_disable(i2s, i); >> + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; >> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); >> + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; >> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); >> + img_i2s_in_ch_enable(i2s, i, reg); >> + } >> +} > > This all seems to be connected to this, which is itself slightly funky > especially in the context of the only user... > They are also used during hw_params and set_format. >> + case SNDRV_PCM_TRIGGER_STOP: >> + case SNDRV_PCM_TRIGGER_SUSPEND: >> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: >> + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); >> + reg &= ~IMG_I2S_IN_CTL_ME_MASK; >> + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); >> + img_i2s_in_flush(i2s); >> + break; > > ...which looks like it'll enable everything, then disable and reenable. > Plus needing to do a flush on trigger seems weird. > If the FIFOs are not flushed, some samples from the previous stream will be transferred to the user application when the block is started again >> + if ((channels < 2) || >> + (channels > (i2s->max_i2s_chan * 2)) || >> + (channels % 2)) >> + return -EINVAL; > > This indentation is very weird. > Ok. What is the correct indentation for this? >> + control_mask = (u32)(~IMG_I2S_IN_CTL_16PACK_MASK & >> + ~IMG_I2S_IN_CTL_ACTIVE_CHAN_MASK); > >> + chan_control_mask = (u32)(~IMG_I2S_IN_CH_CTL_16PACK_MASK & >> + ~IMG_I2S_IN_CH_CTL_FEN_MASK & >> + ~IMG_I2S_IN_CH_CTL_FMODE_MASK & >> + ~IMG_I2S_IN_CH_CTL_SW_MASK & >> + ~IMG_I2S_IN_CH_CTL_FW_MASK & >> + ~IMG_I2S_IN_CH_CTL_PACKH_MASK); > > This also looks very odd. Normally we'd write masks as being the valid > bits and or them together. > Ok >> + i2s->clk_sys = devm_clk_get(dev, "sys"); >> + if (IS_ERR(i2s->clk_sys)) >> + return PTR_ERR(i2s->clk_sys); > > Please print an error message so people can tell why things failed. > Ok >> + rst = devm_reset_control_get(dev, "rst"); >> + if (IS_ERR(rst)) { >> + dev_dbg(dev, "No top level reset found\n"); > > You should check for -EPROBE_DEFER here and just return the error here > if you get it (on the basis that the reset framework ought to be using a > different error if there's nothing bound in DT). > Ok
On Thu, Oct 22, 2015 at 08:09:38PM +0100, Damien Horsley wrote: > On 19/10/15 18:47, Mark Brown wrote: > > On Mon, Oct 12, 2015 at 01:40:29PM +0100, Damien Horsley wrote: > > The APIs here all seem a bit odd - for example the enable API taking a > > register value as an argument (normally reg is a register address BTW) > > and returning a value but the disable API doing a read/modify/write > > cycle. > Sure. It reduces the number of register accesses this way, but the > difference in execution time is not significant. Would you prefer these > to both do read-modify-writes? I would prefer that the functions look consistent with each other and ideally resemble common register acceess idioms in the kernel. > >> +static inline void img_i2s_in_flush(struct img_i2s_in *i2s) > >> +{ > >> + int i; > >> + u32 reg; > >> + > >> + for (i = 0; i < i2s->active_channels; i++) { > >> + reg = img_i2s_in_ch_disable(i2s, i); > >> + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; > >> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); > >> + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; > >> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); > >> + img_i2s_in_ch_enable(i2s, i, reg); > >> + } > >> +} > > This all seems to be connected to this, which is itself slightly funky > > especially in the context of the only user... > They are also used during hw_params and set_format. My point is that the flush function has only one user. > >> + case SNDRV_PCM_TRIGGER_STOP: > >> + case SNDRV_PCM_TRIGGER_SUSPEND: > >> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > >> + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); > >> + reg &= ~IMG_I2S_IN_CTL_ME_MASK; > >> + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); > >> + img_i2s_in_flush(i2s); > >> + break; > > ...which looks like it'll enable everything, then disable and reenable. > > Plus needing to do a flush on trigger seems weird. > If the FIFOs are not flushed, some samples from the previous stream will > be transferred to the user application when the block is started again Shouldn't we be doing that flush on stream close instead? If nothing else the flush is going to discard a bit of data if the stream is just paused. > >> + if ((channels < 2) || > >> + (channels > (i2s->max_i2s_chan * 2)) || > >> + (channels % 2)) > >> + return -EINVAL; > > This indentation is very weird. > Ok. What is the correct indentation for this? Align the continuation lines of the if condition with the first line.
On 23/10/15 23:57, Mark Brown wrote: > On Thu, Oct 22, 2015 at 08:09:38PM +0100, Damien Horsley wrote: >> On 19/10/15 18:47, Mark Brown wrote: >>> On Mon, Oct 12, 2015 at 01:40:29PM +0100, Damien Horsley wrote: > >>> The APIs here all seem a bit odd - for example the enable API taking a >>> register value as an argument (normally reg is a register address BTW) >>> and returning a value but the disable API doing a read/modify/write >>> cycle. > >> Sure. It reduces the number of register accesses this way, but the >> difference in execution time is not significant. Would you prefer these >> to both do read-modify-writes? > > I would prefer that the functions look consistent with each other and > ideally resemble common register acceess idioms in the kernel. > Ok. >>>> +static inline void img_i2s_in_flush(struct img_i2s_in *i2s) >>>> +{ >>>> + int i; >>>> + u32 reg; >>>> + >>>> + for (i = 0; i < i2s->active_channels; i++) { >>>> + reg = img_i2s_in_ch_disable(i2s, i); >>>> + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; >>>> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); >>>> + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; >>>> + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); >>>> + img_i2s_in_ch_enable(i2s, i, reg); >>>> + } >>>> +} > >>> This all seems to be connected to this, which is itself slightly funky >>> especially in the context of the only user... > >> They are also used during hw_params and set_format. > > My point is that the flush function has only one user. > >>>> + case SNDRV_PCM_TRIGGER_STOP: >>>> + case SNDRV_PCM_TRIGGER_SUSPEND: >>>> + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: >>>> + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); >>>> + reg &= ~IMG_I2S_IN_CTL_ME_MASK; >>>> + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); >>>> + img_i2s_in_flush(i2s); >>>> + break; > >>> ...which looks like it'll enable everything, then disable and reenable. >>> Plus needing to do a flush on trigger seems weird. > >> If the FIFOs are not flushed, some samples from the previous stream will >> be transferred to the user application when the block is started again > > Shouldn't we be doing that flush on stream close instead? If nothing > else the flush is going to discard a bit of data if the stream is just > paused. > The FIFOs are only 8 frames in size, so I am not sure there is an issue with these frames being lost. I think it also makes sense to keep the blocks consistent with each other. The spdif (out and in), and parallel out, all flush automatically when stopped, and the fifo for the i2s out block is cleared when the reset is asserted. >>>> + if ((channels < 2) || >>>> + (channels > (i2s->max_i2s_chan * 2)) || >>>> + (channels % 2)) >>>> + return -EINVAL; > >>> This indentation is very weird. > >> Ok. What is the correct indentation for this? > > Align the continuation lines of the if condition with the first line. > Ok
On Tue, Oct 27, 2015 at 01:55:27PM +0000, Damien Horsley wrote: > On 23/10/15 23:57, Mark Brown wrote: > > Shouldn't we be doing that flush on stream close instead? If nothing > > else the flush is going to discard a bit of data if the stream is just > > paused. > The FIFOs are only 8 frames in size, so I am not sure there is an > issue with these frames being lost. > I think it also makes sense to keep the blocks consistent with each > other. The spdif (out and in), and parallel out, all flush automatically > when stopped, and the fifo for the i2s out block is cleared when the > reset is asserted. This seems like an issue that got missed in the other drivers then. I'd expect the trigger operation to be a minimal operation which starts and stops the data transfer, not doing anything else.
On 28/10/15 01:04, Mark Brown wrote: > On Tue, Oct 27, 2015 at 01:55:27PM +0000, Damien Horsley wrote: >> On 23/10/15 23:57, Mark Brown wrote: > >>> Shouldn't we be doing that flush on stream close instead? If nothing >>> else the flush is going to discard a bit of data if the stream is just >>> paused. > >> The FIFOs are only 8 frames in size, so I am not sure there is an >> issue with these frames being lost. > >> I think it also makes sense to keep the blocks consistent with each >> other. The spdif (out and in), and parallel out, all flush automatically >> when stopped, and the fifo for the i2s out block is cleared when the >> reset is asserted. > > This seems like an issue that got missed in the other drivers then. I'd > expect the trigger operation to be a minimal operation which starts and > stops the data transfer, not doing anything else. > The spdif out, spdif in, and parallel out blocks auto-flush whenever they are stopped. It is not possible for software to prevent this behavior.
On Wed, Oct 28, 2015 at 09:18:20PM +0000, Damien Horsley wrote: > On 28/10/15 01:04, Mark Brown wrote: > >> I think it also makes sense to keep the blocks consistent with each > >> other. The spdif (out and in), and parallel out, all flush automatically > >> when stopped, and the fifo for the i2s out block is cleared when the > >> reset is asserted. > > This seems like an issue that got missed in the other drivers then. I'd > > expect the trigger operation to be a minimal operation which starts and > > stops the data transfer, not doing anything else. > The spdif out, spdif in, and parallel out blocks auto-flush whenever > they are stopped. It is not possible for software to prevent this behavior. Oh, so this isn't the drivers doing this? In that case it's fine for them to do that, if it's what the hardware does it's what the hardware does. It sounded like you were saying that there was similar code in the other drivers.
On 28/10/15 23:43, Mark Brown wrote: > On Wed, Oct 28, 2015 at 09:18:20PM +0000, Damien Horsley wrote: >> On 28/10/15 01:04, Mark Brown wrote: > >>>> I think it also makes sense to keep the blocks consistent with each >>>> other. The spdif (out and in), and parallel out, all flush automatically >>>> when stopped, and the fifo for the i2s out block is cleared when the >>>> reset is asserted. > >>> This seems like an issue that got missed in the other drivers then. I'd >>> expect the trigger operation to be a minimal operation which starts and >>> stops the data transfer, not doing anything else. > >> The spdif out, spdif in, and parallel out blocks auto-flush whenever >> they are stopped. It is not possible for software to prevent this behavior. > > Oh, so this isn't the drivers doing this? In that case it's fine for > them to do that, if it's what the hardware does it's what the hardware > does. It sounded like you were saying that there was similar code in > the other drivers. > For the I2S In, there is another issue with flushing on stream close. If the stream is stopped, then reconfigured to use a larger number of channels (without the stream being closed), then the per-channel fifos will become inconsistent with each other. The new channels will have no samples in their FIFOs, while the others may contain samples from the previous stream. Would hw_params be the correct place to flush instead?
On Thu, Oct 29, 2015 at 03:42:59PM +0000, Damien Horsley wrote: > For the I2S In, there is another issue with flushing on stream close. If > the stream is stopped, then reconfigured to use a larger number of > channels (without the stream being closed), then the per-channel fifos > will become inconsistent with each other. The new channels will have no > samples in their FIFOs, while the others may contain samples from the > previous stream. > Would hw_params be the correct place to flush instead? Yes, you could flush there (or in both places for that matter).
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index 7de792b..f9984db 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -47,6 +47,7 @@ source "sound/soc/jz4740/Kconfig" source "sound/soc/nuc900/Kconfig" source "sound/soc/omap/Kconfig" source "sound/soc/kirkwood/Kconfig" +source "sound/soc/img/Kconfig" source "sound/soc/intel/Kconfig" source "sound/soc/mediatek/Kconfig" source "sound/soc/mxs/Kconfig" diff --git a/sound/soc/Makefile b/sound/soc/Makefile index af0a571..7ba9de9 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -26,6 +26,7 @@ obj-$(CONFIG_SND_SOC) += davinci/ obj-$(CONFIG_SND_SOC) += dwc/ obj-$(CONFIG_SND_SOC) += fsl/ obj-$(CONFIG_SND_SOC) += jz4740/ +obj-$(CONFIG_SND_SOC) += img/ obj-$(CONFIG_SND_SOC) += intel/ obj-$(CONFIG_SND_SOC) += mediatek/ obj-$(CONFIG_SND_SOC) += mxs/ diff --git a/sound/soc/img/Kconfig b/sound/soc/img/Kconfig new file mode 100644 index 0000000..f9f73d0 --- /dev/null +++ b/sound/soc/img/Kconfig @@ -0,0 +1,12 @@ +config SND_SOC_IMG + bool "Audio support for Imagination Technologies designs" + help + Audio support for Imagination Technologies audio hardware + +config SND_SOC_IMG_I2S_IN + tristate "Imagination I2S Input Device Driver" + depends on SND_SOC_IMG + select SND_SOC_GENERIC_DMAENGINE_PCM + help + Say Y or M if you want to add support for I2S in driver for + Imagination Technologies I2S in device. diff --git a/sound/soc/img/Makefile b/sound/soc/img/Makefile new file mode 100644 index 0000000..fe8426b --- /dev/null +++ b/sound/soc/img/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_SND_SOC_IMG_I2S_IN) += img-i2s-in.o diff --git a/sound/soc/img/img-i2s-in.c b/sound/soc/img/img-i2s-in.c new file mode 100644 index 0000000..dc1e53a --- /dev/null +++ b/sound/soc/img/img-i2s-in.c @@ -0,0 +1,508 @@ +/* + * IMG I2S input controller driver + * + * Copyright (C) 2015 Imagination Technologies Ltd. + * + * Author: Damien Horsley <Damien.Horsley@imgtec.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + */ + +#include <linux/clk.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/reset.h> + +#include <sound/core.h> +#include <sound/dmaengine_pcm.h> +#include <sound/initval.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> + +#define IMG_I2S_IN_RX_FIFO 0x0 + +#define IMG_I2S_IN_CTL 0x4 +#define IMG_I2S_IN_CTL_ACTIVE_CHAN_MASK 0xfffffffc +#define IMG_I2S_IN_CTL_ACTIVE_CH_SHIFT 2 +#define IMG_I2S_IN_CTL_16PACK_MASK BIT(1) +#define IMG_I2S_IN_CTL_ME_MASK BIT(0) + +#define IMG_I2S_IN_CH_CTL 0x4 +#define IMG_I2S_IN_CH_CTL_CCDEL_MASK 0x38000 +#define IMG_I2S_IN_CH_CTL_CCDEL_SHIFT 15 +#define IMG_I2S_IN_CH_CTL_FEN_MASK BIT(14) +#define IMG_I2S_IN_CH_CTL_FMODE_MASK BIT(13) +#define IMG_I2S_IN_CH_CTL_16PACK_MASK BIT(12) +#define IMG_I2S_IN_CH_CTL_JUST_MASK BIT(10) +#define IMG_I2S_IN_CH_CTL_PACKH_MASK BIT(9) +#define IMG_I2S_IN_CH_CTL_CLK_TRANS_MASK BIT(8) +#define IMG_I2S_IN_CH_CTL_BLKP_MASK BIT(7) +#define IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK BIT(6) +#define IMG_I2S_IN_CH_CTL_LRD_MASK BIT(3) +#define IMG_I2S_IN_CH_CTL_FW_MASK BIT(2) +#define IMG_I2S_IN_CH_CTL_SW_MASK BIT(1) +#define IMG_I2S_IN_CH_CTL_ME_MASK BIT(0) + +#define IMG_I2S_IN_CH_STRIDE 0x20 + +struct img_i2s_in { + void __iomem *base; + struct clk *clk_sys; + struct snd_dmaengine_dai_dma_data dma_data; + struct device *dev; + unsigned int max_i2s_chan; + void __iomem *channel_base; + unsigned int active_channels; + struct snd_soc_dai_driver dai_driver; +}; + +static inline void img_i2s_in_writel(struct img_i2s_in *i2s, u32 val, u32 reg) +{ + writel(val, i2s->base + reg); +} + +static inline u32 img_i2s_in_readl(struct img_i2s_in *i2s, u32 reg) +{ + return readl(i2s->base + reg); +} + +static inline void img_i2s_in_ch_writel(struct img_i2s_in *i2s, u32 chan, + u32 val, u32 reg) +{ + writel(val, i2s->channel_base + (chan * IMG_I2S_IN_CH_STRIDE) + reg); +} + +static inline u32 img_i2s_in_ch_readl(struct img_i2s_in *i2s, u32 chan, + u32 reg) +{ + return readl(i2s->channel_base + (chan * IMG_I2S_IN_CH_STRIDE) + reg); +} + +static inline u32 img_i2s_in_ch_disable(struct img_i2s_in *i2s, u32 chan) +{ + u32 reg; + + reg = img_i2s_in_ch_readl(i2s, chan, IMG_I2S_IN_CH_CTL); + reg &= ~IMG_I2S_IN_CH_CTL_ME_MASK; + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); + + return reg; +} + +static inline void img_i2s_in_ch_enable(struct img_i2s_in *i2s, u32 chan, + u32 reg) +{ + reg |= IMG_I2S_IN_CH_CTL_ME_MASK; + img_i2s_in_ch_writel(i2s, chan, reg, IMG_I2S_IN_CH_CTL); +} + +static inline void img_i2s_in_flush(struct img_i2s_in *i2s) +{ + int i; + u32 reg; + + for (i = 0; i < i2s->active_channels; i++) { + reg = img_i2s_in_ch_disable(i2s, i); + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + img_i2s_in_ch_enable(i2s, i, reg); + } +} + +static int img_i2s_in_trigger(struct snd_pcm_substream *substream, int cmd, + struct snd_soc_dai *dai) +{ + struct img_i2s_in *i2s = snd_soc_dai_get_drvdata(dai); + u32 reg; + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); + reg |= IMG_I2S_IN_CTL_ME_MASK; + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); + break; + + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); + reg &= ~IMG_I2S_IN_CTL_ME_MASK; + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); + img_i2s_in_flush(i2s); + break; + default: + return -EINVAL; + } + + return 0; +} + +static int img_i2s_in_check_rate(struct img_i2s_in *i2s, + unsigned int sample_rate, unsigned int frame_size, + unsigned int *bclk_filter_enable, + unsigned int *bclk_filter_value) +{ + unsigned int bclk_freq, cur_freq; + + bclk_freq = sample_rate * frame_size; + + cur_freq = clk_get_rate(i2s->clk_sys); + + if (cur_freq >= bclk_freq * 8) { + *bclk_filter_enable = 1; + *bclk_filter_value = 0; + } else if (cur_freq >= bclk_freq * 7) { + *bclk_filter_enable = 1; + *bclk_filter_value = 1; + } else if (cur_freq >= bclk_freq * 6) { + *bclk_filter_enable = 0; + *bclk_filter_value = 0; + } else { + dev_err(i2s->dev, + "Sys clock rate %u insufficient for sample rate %u\n", + cur_freq, sample_rate); + return -EINVAL; + } + + return 0; +} + +static int img_i2s_in_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) +{ + struct img_i2s_in *i2s = snd_soc_dai_get_drvdata(dai); + unsigned int rate, channels, i2s_channels, frame_size; + unsigned int bclk_filter_enable, bclk_filter_value; + int i, ret = 0; + u32 reg, control_reg, control_mask, chan_control_mask; + u32 control_set = 0, chan_control_set = 0; + snd_pcm_format_t format; + + rate = params_rate(params); + format = params_format(params); + channels = params_channels(params); + i2s_channels = channels / 2; + + switch (format) { + case SNDRV_PCM_FORMAT_S32_LE: + frame_size = 64; + chan_control_set |= IMG_I2S_IN_CH_CTL_SW_MASK; + chan_control_set |= IMG_I2S_IN_CH_CTL_FW_MASK; + chan_control_set |= IMG_I2S_IN_CH_CTL_PACKH_MASK; + break; + case SNDRV_PCM_FORMAT_S24_LE: + frame_size = 64; + chan_control_set |= IMG_I2S_IN_CH_CTL_SW_MASK; + chan_control_set |= IMG_I2S_IN_CH_CTL_FW_MASK; + break; + case SNDRV_PCM_FORMAT_S16_LE: + frame_size = 32; + control_set |= IMG_I2S_IN_CTL_16PACK_MASK; + chan_control_set |= IMG_I2S_IN_CH_CTL_16PACK_MASK; + break; + default: + return -EINVAL; + } + + if ((channels < 2) || + (channels > (i2s->max_i2s_chan * 2)) || + (channels % 2)) + return -EINVAL; + + control_set |= ((i2s_channels - 1) << IMG_I2S_IN_CTL_ACTIVE_CH_SHIFT); + + ret = img_i2s_in_check_rate(i2s, rate, frame_size, + &bclk_filter_enable, &bclk_filter_value); + if (ret < 0) + return ret; + + if (bclk_filter_enable) + chan_control_set |= IMG_I2S_IN_CH_CTL_FEN_MASK; + + if (bclk_filter_value) + chan_control_set |= IMG_I2S_IN_CH_CTL_FMODE_MASK; + + control_mask = (u32)(~IMG_I2S_IN_CTL_16PACK_MASK & + ~IMG_I2S_IN_CTL_ACTIVE_CHAN_MASK); + + chan_control_mask = (u32)(~IMG_I2S_IN_CH_CTL_16PACK_MASK & + ~IMG_I2S_IN_CH_CTL_FEN_MASK & + ~IMG_I2S_IN_CH_CTL_FMODE_MASK & + ~IMG_I2S_IN_CH_CTL_SW_MASK & + ~IMG_I2S_IN_CH_CTL_FW_MASK & + ~IMG_I2S_IN_CH_CTL_PACKH_MASK); + + control_reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); + control_reg = (control_reg & control_mask) | control_set; + img_i2s_in_writel(i2s, control_reg, IMG_I2S_IN_CTL); + + for (i = 0; i < i2s_channels; i++) { + reg = img_i2s_in_ch_disable(i2s, i); + reg = (reg & chan_control_mask) | chan_control_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + img_i2s_in_ch_enable(i2s, i, reg); + } + for (; i < i2s->max_i2s_chan; i++) { + reg = img_i2s_in_ch_disable(i2s, i); + reg = (reg & chan_control_mask) | chan_control_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + } + + i2s->active_channels = i2s_channels; + + return 0; +} + +static int img_i2s_in_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) +{ + struct img_i2s_in *i2s = snd_soc_dai_get_drvdata(dai); + int i; + u32 chan_control_mask, lrd_set = 0, blkp_set = 0, chan_control_set = 0; + u32 reg; + + switch (fmt & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + lrd_set |= IMG_I2S_IN_CH_CTL_LRD_MASK; + break; + case SND_SOC_DAIFMT_NB_IF: + break; + case SND_SOC_DAIFMT_IB_NF: + lrd_set |= IMG_I2S_IN_CH_CTL_LRD_MASK; + blkp_set |= IMG_I2S_IN_CH_CTL_BLKP_MASK; + break; + case SND_SOC_DAIFMT_IB_IF: + blkp_set |= IMG_I2S_IN_CH_CTL_BLKP_MASK; + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + chan_control_set |= IMG_I2S_IN_CH_CTL_CLK_TRANS_MASK; + break; + case SND_SOC_DAIFMT_LEFT_J: + break; + default: + return -EINVAL; + } + + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { + case SND_SOC_DAIFMT_CBM_CFM: + break; + default: + return -EINVAL; + } + + chan_control_mask = (u32)~IMG_I2S_IN_CH_CTL_CLK_TRANS_MASK; + + /* + * BLKP and LRD must be set during separate register writes + */ + for (i = 0; i < i2s->active_channels; i++) { + reg = img_i2s_in_ch_disable(i2s, i); + reg = (reg & chan_control_mask) | chan_control_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg = (reg & ~IMG_I2S_IN_CH_CTL_BLKP_MASK) | blkp_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg = (reg & ~IMG_I2S_IN_CH_CTL_LRD_MASK) | lrd_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + img_i2s_in_ch_enable(i2s, i, reg); + } + + for (; i < i2s->max_i2s_chan; i++) { + reg = img_i2s_in_ch_readl(i2s, i, IMG_I2S_IN_CH_CTL); + reg = (reg & chan_control_mask) | chan_control_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg = (reg & ~IMG_I2S_IN_CH_CTL_BLKP_MASK) | blkp_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg = (reg & ~IMG_I2S_IN_CH_CTL_LRD_MASK) | lrd_set; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + } + + return 0; +} + +static const struct snd_soc_dai_ops img_i2s_in_dai_ops = { + .trigger = img_i2s_in_trigger, + .hw_params = img_i2s_in_hw_params, + .set_fmt = img_i2s_in_set_fmt +}; + +static int img_i2s_in_dai_probe(struct snd_soc_dai *dai) +{ + struct img_i2s_in *i2s = snd_soc_dai_get_drvdata(dai); + + snd_soc_dai_init_dma_data(dai, NULL, &i2s->dma_data); + + return 0; +} + +static const struct snd_soc_component_driver img_i2s_in_component = { + .name = "img-i2s-in" +}; + +static int img_i2s_in_dma_prepare_slave_config(struct snd_pcm_substream *st, + struct snd_pcm_hw_params *params, struct dma_slave_config *sc) +{ + unsigned int i2s_channels = params_channels(params) / 2; + struct snd_soc_pcm_runtime *rtd = st->private_data; + struct snd_dmaengine_dai_dma_data *dma_data; + int ret; + + dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, st); + + ret = snd_hwparams_to_dma_slave_config(st, params, sc); + if (ret) + return ret; + + sc->src_addr = dma_data->addr; + sc->src_addr_width = dma_data->addr_width; + sc->src_maxburst = 4 * i2s_channels; + + return 0; +} + +static const struct snd_dmaengine_pcm_config img_i2s_in_dma_config = { + .prepare_slave_config = img_i2s_in_dma_prepare_slave_config +}; + +static int img_i2s_in_probe(struct platform_device *pdev) +{ + struct img_i2s_in *i2s; + struct resource *res; + void __iomem *base; + int ret, i; + struct reset_control *rst; + u32 reg; + unsigned int max_i2s_chan_pow_2; + struct device *dev = &pdev->dev; + + i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL); + if (!i2s) + return -ENOMEM; + + platform_set_drvdata(pdev, i2s); + + i2s->dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + + i2s->base = base; + + if (of_property_read_u32(pdev->dev.of_node, "img,i2s-channels", + &i2s->max_i2s_chan)) { + dev_err(dev, "No img,i2s-channels property\n"); + return -EINVAL; + } + + max_i2s_chan_pow_2 = 1 << get_count_order(i2s->max_i2s_chan); + + i2s->channel_base = base + (max_i2s_chan_pow_2 * 0x20); + + i2s->clk_sys = devm_clk_get(dev, "sys"); + if (IS_ERR(i2s->clk_sys)) + return PTR_ERR(i2s->clk_sys); + + ret = clk_prepare_enable(i2s->clk_sys); + if (ret) + return ret; + + i2s->active_channels = 1; + i2s->dma_data.addr = res->start + IMG_I2S_IN_RX_FIFO; + i2s->dma_data.addr_width = 4; + + i2s->dai_driver.probe = img_i2s_in_dai_probe; + i2s->dai_driver.capture.channels_min = 2; + i2s->dai_driver.capture.channels_max = i2s->max_i2s_chan * 2; + i2s->dai_driver.capture.rates = SNDRV_PCM_RATE_8000_192000; + i2s->dai_driver.capture.formats = SNDRV_PCM_FMTBIT_S32_LE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE; + i2s->dai_driver.ops = &img_i2s_in_dai_ops; + + rst = devm_reset_control_get(dev, "rst"); + if (IS_ERR(rst)) { + dev_dbg(dev, "No top level reset found\n"); + + reg = img_i2s_in_readl(i2s, IMG_I2S_IN_CTL); + reg &= ~IMG_I2S_IN_CTL_ME_MASK; + img_i2s_in_writel(i2s, reg, IMG_I2S_IN_CTL); + + for (i = 0; i < i2s->max_i2s_chan; i++) { + reg = img_i2s_in_ch_disable(i2s, i); + reg |= IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + reg &= ~IMG_I2S_IN_CH_CTL_FIFO_FLUSH_MASK; + img_i2s_in_ch_writel(i2s, i, reg, IMG_I2S_IN_CH_CTL); + } + } else { + reset_control_assert(rst); + reset_control_deassert(rst); + } + + img_i2s_in_writel(i2s, 0, IMG_I2S_IN_CTL); + + for (i = 0; i < i2s->max_i2s_chan; i++) + img_i2s_in_ch_writel(i2s, i, + (4 << IMG_I2S_IN_CH_CTL_CCDEL_SHIFT) | + IMG_I2S_IN_CH_CTL_JUST_MASK | + IMG_I2S_IN_CH_CTL_FW_MASK, IMG_I2S_IN_CH_CTL); + + ret = devm_snd_soc_register_component(dev, &img_i2s_in_component, + &i2s->dai_driver, 1); + if (ret) + goto err_clk_disable; + + ret = devm_snd_dmaengine_pcm_register(dev, &img_i2s_in_dma_config, 0); + if (ret) + goto err_clk_disable; + + return 0; + +err_clk_disable: + clk_disable_unprepare(i2s->clk_sys); + + return ret; +} + +static int img_i2s_in_dev_remove(struct platform_device *pdev) +{ + struct img_i2s_in *i2s = platform_get_drvdata(pdev); + + clk_disable_unprepare(i2s->clk_sys); + + return 0; +} + +static const struct of_device_id img_i2s_in_of_match[] = { + { .compatible = "img,i2s-in" }, + {} +}; +MODULE_DEVICE_TABLE(of, img_i2s_in_of_match); + +static struct platform_driver img_i2s_in_driver = { + .driver = { + .name = "img-i2s-in", + .of_match_table = img_i2s_in_of_match + }, + .probe = img_i2s_in_probe, + .remove = img_i2s_in_dev_remove +}; +module_platform_driver(img_i2s_in_driver); + +MODULE_AUTHOR("Damien Horsley <Damien.Horsley@imgtec.com>"); +MODULE_DESCRIPTION("IMG I2S Input Driver"); +MODULE_LICENSE("GPL v2");