[alsa-devel] [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
Hi,
This series is a cleanup of the fsl-ssi driver.
There is no DT binding for regmap yet, as this should be solved in the regmap framework first. The current configuration should at least work on the same SoCs it was previously used. As soon as we have a common DT binding to set the endianess we replace it in this driver.
Best regards,
Markus
Changes in v3: - Some new patches to improve/fix i2s master mode by Sascha - baudclock is enabled/disabled in startup/shutdown now - bitclock setup moved to a seperate function (not set_dai_sysclk) - Regmap config changed to NATIVE now.
Markus Pargmann (11): ASoC: fsl-ssi: Fix register values when disabling ASoC: fsl-ssi: Move debugging to seperate file ASoC: fsl-ssi: Use dev_name for DAI driver struct ASoC: fsl-ssi: Move imx-specific probe to seperate function ASoC: fsl-ssi: Remove useless DMA code ASoC: fsl-ssi: Cleanup probe function ASoC: fsl-ssi: Remove unnecessary variables from ssi_private ASoC: fsl-ssi: make fsl,mode property optional ASoC: fsl-ssi: Transmit enable synchronization ASoC: fsl-ssi: reorder and document fsl_ssi_private ASoC: fsl-ssi: Use regmap
Sascha Hauer (7): ASoC: fsl-ssi: introduce SoC specific data ASoC: fsl-ssi: Only enable baudclk when used ASoC: fsl-ssi: Move fsl_ssi_set_dai_sysclk above fsl_ssi_hw_params ASoC: fsl-ssi: set bitclock in master mode from hw_params ASoC: fsl-ssi: remove unnecessary spinlock ASoC: fsl-ssi: Allow first stream to set the bitclock ASoC: fsl-ssi: Set framerate divider correctly for i2s master mode
sound/soc/fsl/Kconfig | 1 + sound/soc/fsl/Makefile | 3 +- sound/soc/fsl/fsl_ssi.c | 1249 +++++++++++++++++++------------------------ sound/soc/fsl/fsl_ssi.h | 112 +++- sound/soc/fsl/fsl_ssi_dbg.c | 163 ++++++ 5 files changed, 807 insertions(+), 721 deletions(-) create mode 100644 sound/soc/fsl/fsl_ssi_dbg.c
The bits we have to clear when disabling are different when the other stream is still active.
This patch fixes the calculation of new register values after disabling one stream. It also adds a more detailed description of the new register value calculation.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index fdb123d..fadb264 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -490,6 +490,26 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi_private *ssi_private, }
/* + * Calculate the bits that have to be disabled for the current stream that is + * getting disabled. This keeps the bits enabled that are necessary for the + * second stream to work if 'stream_active' is true. + * + * Detailed calculation: + * These are the values that need to be active after disabling. For non-active + * second stream, this is 0: + * vals_stream * !!stream_active + * + * The following computes the overall differences between the setup for the + * to-disable stream and the active stream, a simple XOR: + * vals_disable ^ (vals_stream * !!(stream_active)) + * + * The full expression adds a mask on all values we care about + */ +#define fsl_ssi_disable_val(vals_disable, vals_stream, stream_active) \ + ((vals_disable) & \ + ((vals_disable) ^ ((vals_stream) * (u32)!!(stream_active)))) + +/* * Enable/Disable a ssi configuration. You have to pass either * ssi_private->rxtx_reg_val.rx or tx as vals parameter. */ @@ -501,6 +521,12 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, u32 scr_val = read_ssi(&ssi->scr); int nr_active_streams = !!(scr_val & CCSR_SSI_SCR_TE) + !!(scr_val & CCSR_SSI_SCR_RE); + int keep_active; + + if (nr_active_streams - 1 > 0) + keep_active = 1; + else + keep_active = 0;
/* Find the other direction values rx or tx which we do not want to * modify */ @@ -511,7 +537,8 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
/* If vals should be disabled, start with disabling the unit */ if (!enable) { - u32 scr = vals->scr & (vals->scr ^ avals->scr); + u32 scr = fsl_ssi_disable_val(vals->scr, avals->scr, + keep_active); write_ssi_mask(&ssi->scr, scr, 0); }
@@ -522,7 +549,7 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, */ if (ssi_private->offline_config) { if ((enable && !nr_active_streams) || - (!enable && nr_active_streams == 1)) + (!enable && !keep_active)) fsl_ssi_rxtx_config(ssi_private, enable);
goto config_done; @@ -551,9 +578,12 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, */
/* These assignments are simply vals without bits set in avals*/ - sier = vals->sier & (vals->sier ^ avals->sier); - srcr = vals->srcr & (vals->srcr ^ avals->srcr); - stcr = vals->stcr & (vals->stcr ^ avals->stcr); + sier = fsl_ssi_disable_val(vals->sier, avals->sier, + keep_active); + srcr = fsl_ssi_disable_val(vals->srcr, avals->srcr, + keep_active); + stcr = fsl_ssi_disable_val(vals->stcr, avals->stcr, + keep_active);
write_ssi_mask(&ssi->srcr, srcr, 0); write_ssi_mask(&ssi->stcr, stcr, 0);
Move all code that is only used for debugging to a seperate file. This makes it easier to see what functions are used for debugging only.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/Makefile | 3 +- sound/soc/fsl/fsl_ssi.c | 241 ++------------------------------------------ sound/soc/fsl/fsl_ssi.h | 61 ++++++++++- sound/soc/fsl/fsl_ssi_dbg.c | 163 ++++++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 236 deletions(-) create mode 100644 sound/soc/fsl/fsl_ssi_dbg.c
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index b12ad4b..db254e3 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -12,7 +12,8 @@ obj-$(CONFIG_SND_SOC_P1022_RDK) += snd-soc-p1022-rdk.o
# Freescale SSI/DMA/SAI/SPDIF Support snd-soc-fsl-sai-objs := fsl_sai.o -snd-soc-fsl-ssi-objs := fsl_ssi.o +snd-soc-fsl-ssi-y := fsl_ssi.o +snd-soc-fsl-ssi-$(CONFIG_DEBUG_FS) += fsl_ssi_dbg.o snd-soc-fsl-spdif-objs := fsl_spdif.o snd-soc-fsl-esai-objs := fsl_esai.o snd-soc-fsl-utils-objs := fsl_utils.o diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index fadb264..344f752 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -35,7 +35,6 @@ #include <linux/module.h> #include <linux/interrupt.h> #include <linux/clk.h> -#include <linux/debugfs.h> #include <linux/device.h> #include <linux/delay.h> #include <linux/slab.h> @@ -113,8 +112,6 @@ static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set) #define FSLSSI_SIER_DBG_TX_FLAGS (CCSR_SSI_SIER_TFE0_EN | \ CCSR_SSI_SIER_TLS_EN | CCSR_SSI_SIER_TFS_EN | \ CCSR_SSI_SIER_TUE0_EN | CCSR_SSI_SIER_TFRC_EN) -#define FSLSSI_SISR_MASK (FSLSSI_SIER_DBG_RX_FLAGS | FSLSSI_SIER_DBG_TX_FLAGS) -
enum fsl_ssi_type { FSL_SSI_MCP8610, @@ -177,31 +174,7 @@ struct fsl_ssi_private { /* Register values for rx/tx configuration */ struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
- struct { - unsigned int rfrc; - unsigned int tfrc; - unsigned int cmdau; - unsigned int cmddu; - unsigned int rxt; - unsigned int rdr1; - unsigned int rdr0; - unsigned int tde1; - unsigned int tde0; - unsigned int roe1; - unsigned int roe0; - unsigned int tue1; - unsigned int tue0; - unsigned int tfs; - unsigned int rfs; - unsigned int tls; - unsigned int rls; - unsigned int rff1; - unsigned int rff0; - unsigned int tfe1; - unsigned int tfe0; - } stats; - struct dentry *dbg_dir; - struct dentry *dbg_stats; + struct fsl_ssi_dbg dbg_stats;
char name[1]; }; @@ -231,7 +204,6 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) { struct fsl_ssi_private *ssi_private = dev_id; struct ccsr_ssi __iomem *ssi = ssi_private->ssi; - irqreturn_t ret = IRQ_NONE; __be32 sisr; __be32 sisr2; __be32 sisr_write_mask = 0; @@ -258,217 +230,18 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) were interrupted for. We mask it with the Interrupt Enable register so that we only check for events that we're interested in. */ - sisr = read_ssi(&ssi->sisr) & FSLSSI_SISR_MASK; - - if (sisr & CCSR_SSI_SISR_RFRC) { - ssi_private->stats.rfrc++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TFRC) { - ssi_private->stats.tfrc++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_CMDAU) { - ssi_private->stats.cmdau++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_CMDDU) { - ssi_private->stats.cmddu++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RXT) { - ssi_private->stats.rxt++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RDR1) { - ssi_private->stats.rdr1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RDR0) { - ssi_private->stats.rdr0++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TDE1) { - ssi_private->stats.tde1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TDE0) { - ssi_private->stats.tde0++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_ROE1) { - ssi_private->stats.roe1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_ROE0) { - ssi_private->stats.roe0++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TUE1) { - ssi_private->stats.tue1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TUE0) { - ssi_private->stats.tue0++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TFS) { - ssi_private->stats.tfs++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RFS) { - ssi_private->stats.rfs++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TLS) { - ssi_private->stats.tls++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RLS) { - ssi_private->stats.rls++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RFF1) { - ssi_private->stats.rff1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_RFF0) { - ssi_private->stats.rff0++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TFE1) { - ssi_private->stats.tfe1++; - ret = IRQ_HANDLED; - } - - if (sisr & CCSR_SSI_SISR_TFE0) { - ssi_private->stats.tfe0++; - ret = IRQ_HANDLED; - } + sisr = read_ssi(&ssi->sisr);
sisr2 = sisr & sisr_write_mask; /* Clear the bits that we set */ if (sisr2) write_ssi(sisr2, &ssi->sisr);
- return ret; -} - -#if IS_ENABLED(CONFIG_DEBUG_FS) -/* Show the statistics of a flag only if its interrupt is enabled. The - * compiler will optimze this code to a no-op if the interrupt is not - * enabled. - */ -#define SIER_SHOW(flag, name) \ - do { \ - if (FSLSSI_SISR_MASK & CCSR_SSI_SIER_##flag) \ - seq_printf(s, #name "=%u\n", ssi_private->stats.name); \ - } while (0) - - -/** - * fsl_sysfs_ssi_show: display SSI statistics - * - * Display the statistics for the current SSI device. To avoid confusion, - * we only show those counts that are enabled. - */ -static int fsl_ssi_stats_show(struct seq_file *s, void *unused) -{ - struct fsl_ssi_private *ssi_private = s->private; - - SIER_SHOW(RFRC_EN, rfrc); - SIER_SHOW(TFRC_EN, tfrc); - SIER_SHOW(CMDAU_EN, cmdau); - SIER_SHOW(CMDDU_EN, cmddu); - SIER_SHOW(RXT_EN, rxt); - SIER_SHOW(RDR1_EN, rdr1); - SIER_SHOW(RDR0_EN, rdr0); - SIER_SHOW(TDE1_EN, tde1); - SIER_SHOW(TDE0_EN, tde0); - SIER_SHOW(ROE1_EN, roe1); - SIER_SHOW(ROE0_EN, roe0); - SIER_SHOW(TUE1_EN, tue1); - SIER_SHOW(TUE0_EN, tue0); - SIER_SHOW(TFS_EN, tfs); - SIER_SHOW(RFS_EN, rfs); - SIER_SHOW(TLS_EN, tls); - SIER_SHOW(RLS_EN, rls); - SIER_SHOW(RFF1_EN, rff1); - SIER_SHOW(RFF0_EN, rff0); - SIER_SHOW(TFE1_EN, tfe1); - SIER_SHOW(TFE0_EN, tfe0); - - return 0; -} - -static int fsl_ssi_stats_open(struct inode *inode, struct file *file) -{ - return single_open(file, fsl_ssi_stats_show, inode->i_private); -} - -static const struct file_operations fsl_ssi_stats_ops = { - .open = fsl_ssi_stats_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; - -static int fsl_ssi_debugfs_create(struct fsl_ssi_private *ssi_private, - struct device *dev) -{ - ssi_private->dbg_dir = debugfs_create_dir(dev_name(dev), NULL); - if (!ssi_private->dbg_dir) - return -ENOMEM; - - ssi_private->dbg_stats = debugfs_create_file("stats", S_IRUGO, - ssi_private->dbg_dir, ssi_private, &fsl_ssi_stats_ops); - if (!ssi_private->dbg_stats) { - debugfs_remove(ssi_private->dbg_dir); - return -ENOMEM; - } - - return 0; -} - -static void fsl_ssi_debugfs_remove(struct fsl_ssi_private *ssi_private) -{ - debugfs_remove(ssi_private->dbg_stats); - debugfs_remove(ssi_private->dbg_dir); -} - -#else - -static int fsl_ssi_debugfs_create(struct fsl_ssi_private *ssi_private, - struct device *dev) -{ - return 0; -} + fsl_ssi_dbg_isr(&ssi_private->dbg_stats, sisr);
-static void fsl_ssi_debugfs_remove(struct fsl_ssi_private *ssi_private) -{ + return IRQ_HANDLED; }
-#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */ - /* * Enable/Disable all rx/tx config flags at once. */ @@ -1452,7 +1225,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) goto error_dev; }
- ret = fsl_ssi_debugfs_create(ssi_private, &pdev->dev); + ret = fsl_ssi_debugfs_create(&ssi_private->dbg_stats, &pdev->dev); if (ret) goto error_dbgfs;
@@ -1522,7 +1295,7 @@ error_dai: imx_pcm_fiq_exit(pdev);
error_pcm: - fsl_ssi_debugfs_remove(ssi_private); + fsl_ssi_debugfs_remove(&ssi_private->dbg_stats);
error_dbgfs: snd_soc_unregister_component(&pdev->dev); @@ -1548,7 +1321,7 @@ static int fsl_ssi_remove(struct platform_device *pdev) { struct fsl_ssi_private *ssi_private = dev_get_drvdata(&pdev->dev);
- fsl_ssi_debugfs_remove(ssi_private); + fsl_ssi_debugfs_remove(&ssi_private->dbg_stats);
if (!ssi_private->new_binding) platform_device_unregister(ssi_private->pdev); diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h index e6b6324..2e95dd7 100644 --- a/sound/soc/fsl/fsl_ssi.h +++ b/sound/soc/fsl/fsl_ssi.h @@ -206,5 +206,64 @@ struct ccsr_ssi { #define CCSR_SSI_SACNT_FV 0x00000002 #define CCSR_SSI_SACNT_AC97EN 0x00000001
-#endif
+struct device; + +#if IS_ENABLED(CONFIG_DEBUG_FS) + +struct fsl_ssi_dbg { + struct dentry *dbg_dir; + struct dentry *dbg_stats; + + struct { + unsigned int rfrc; + unsigned int tfrc; + unsigned int cmdau; + unsigned int cmddu; + unsigned int rxt; + unsigned int rdr1; + unsigned int rdr0; + unsigned int tde1; + unsigned int tde0; + unsigned int roe1; + unsigned int roe0; + unsigned int tue1; + unsigned int tue0; + unsigned int tfs; + unsigned int rfs; + unsigned int tls; + unsigned int rls; + unsigned int rff1; + unsigned int rff0; + unsigned int tfe1; + unsigned int tfe0; + } stats; +}; + +void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *ssi_dbg, u32 sisr); + +int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev); + +void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg); + +#else + +struct fsl_ssi_dbg { +}; + +static inline void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *stats, u32 sisr) +{ +} + +static inline int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, + struct device *dev) +{ + return 0; +} + +static inline void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg) +{ +} +#endif /* ! IS_ENABLED(CONFIG_DEBUG_FS) */ + +#endif diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c new file mode 100644 index 0000000..5469ffb --- /dev/null +++ b/sound/soc/fsl/fsl_ssi_dbg.c @@ -0,0 +1,163 @@ +/* + * Freescale SSI ALSA SoC Digital Audio Interface (DAI) debugging functions + * + * Copyright 2014 Markus Pargmann mpa@pengutronix.de, Pengutronix + * + * Splitted from fsl_ssi.c + * + * This file is licensed under the terms of the GNU General Public License + * version 2. This program is licensed "as is" without any warranty of any + * kind, whether express or implied. + */ + +#include <linux/debugfs.h> +#include <linux/device.h> +#include <linux/kernel.h> + +#include "fsl_ssi.h" + +void fsl_ssi_dbg_isr(struct fsl_ssi_dbg *dbg, u32 sisr) +{ + if (sisr & CCSR_SSI_SISR_RFRC) + dbg->stats.rfrc++; + + if (sisr & CCSR_SSI_SISR_TFRC) + dbg->stats.tfrc++; + + if (sisr & CCSR_SSI_SISR_CMDAU) + dbg->stats.cmdau++; + + if (sisr & CCSR_SSI_SISR_CMDDU) + dbg->stats.cmddu++; + + if (sisr & CCSR_SSI_SISR_RXT) + dbg->stats.rxt++; + + if (sisr & CCSR_SSI_SISR_RDR1) + dbg->stats.rdr1++; + + if (sisr & CCSR_SSI_SISR_RDR0) + dbg->stats.rdr0++; + + if (sisr & CCSR_SSI_SISR_TDE1) + dbg->stats.tde1++; + + if (sisr & CCSR_SSI_SISR_TDE0) + dbg->stats.tde0++; + + if (sisr & CCSR_SSI_SISR_ROE1) + dbg->stats.roe1++; + + if (sisr & CCSR_SSI_SISR_ROE0) + dbg->stats.roe0++; + + if (sisr & CCSR_SSI_SISR_TUE1) + dbg->stats.tue1++; + + if (sisr & CCSR_SSI_SISR_TUE0) + dbg->stats.tue0++; + + if (sisr & CCSR_SSI_SISR_TFS) + dbg->stats.tfs++; + + if (sisr & CCSR_SSI_SISR_RFS) + dbg->stats.rfs++; + + if (sisr & CCSR_SSI_SISR_TLS) + dbg->stats.tls++; + + if (sisr & CCSR_SSI_SISR_RLS) + dbg->stats.rls++; + + if (sisr & CCSR_SSI_SISR_RFF1) + dbg->stats.rff1++; + + if (sisr & CCSR_SSI_SISR_RFF0) + dbg->stats.rff0++; + + if (sisr & CCSR_SSI_SISR_TFE1) + dbg->stats.tfe1++; + + if (sisr & CCSR_SSI_SISR_TFE0) + dbg->stats.tfe0++; +} + +/* Show the statistics of a flag only if its interrupt is enabled. The + * compiler will optimze this code to a no-op if the interrupt is not + * enabled. + */ +#define SIER_SHOW(flag, name) \ + do { \ + if (CCSR_SSI_SIER_##flag) \ + seq_printf(s, #name "=%u\n", ssi_dbg->stats.name); \ + } while (0) + + +/** + * fsl_sysfs_ssi_show: display SSI statistics + * + * Display the statistics for the current SSI device. To avoid confusion, + * we only show those counts that are enabled. + */ +static int fsl_ssi_stats_show(struct seq_file *s, void *unused) +{ + struct fsl_ssi_dbg *ssi_dbg = s->private; + + SIER_SHOW(RFRC_EN, rfrc); + SIER_SHOW(TFRC_EN, tfrc); + SIER_SHOW(CMDAU_EN, cmdau); + SIER_SHOW(CMDDU_EN, cmddu); + SIER_SHOW(RXT_EN, rxt); + SIER_SHOW(RDR1_EN, rdr1); + SIER_SHOW(RDR0_EN, rdr0); + SIER_SHOW(TDE1_EN, tde1); + SIER_SHOW(TDE0_EN, tde0); + SIER_SHOW(ROE1_EN, roe1); + SIER_SHOW(ROE0_EN, roe0); + SIER_SHOW(TUE1_EN, tue1); + SIER_SHOW(TUE0_EN, tue0); + SIER_SHOW(TFS_EN, tfs); + SIER_SHOW(RFS_EN, rfs); + SIER_SHOW(TLS_EN, tls); + SIER_SHOW(RLS_EN, rls); + SIER_SHOW(RFF1_EN, rff1); + SIER_SHOW(RFF0_EN, rff0); + SIER_SHOW(TFE1_EN, tfe1); + SIER_SHOW(TFE0_EN, tfe0); + + return 0; +} + +static int fsl_ssi_stats_open(struct inode *inode, struct file *file) +{ + return single_open(file, fsl_ssi_stats_show, inode->i_private); +} + +static const struct file_operations fsl_ssi_stats_ops = { + .open = fsl_ssi_stats_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev) +{ + ssi_dbg->dbg_dir = debugfs_create_dir(dev_name(dev), NULL); + if (!ssi_dbg->dbg_dir) + return -ENOMEM; + + ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO, + ssi_dbg->dbg_dir, ssi_dbg, &fsl_ssi_stats_ops); + if (!ssi_dbg->dbg_stats) { + debugfs_remove(ssi_dbg->dbg_dir); + return -ENOMEM; + } + + return 0; +} + +void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg) +{ + debugfs_remove(ssi_dbg->dbg_stats); + debugfs_remove(ssi_dbg->dbg_dir); +}
Instead of creating a name using string manipulation functions, we can simply use the device name for the DAI driver struct.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 344f752..65d9da1 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -1027,17 +1027,13 @@ static int fsl_ssi_probe(struct platform_device *pdev) if (!strcmp(sprop, "ac97-slave")) ac97 = true;
- /* The DAI name is the last part of the full name of the node. */ - p = strrchr(np->full_name, '/') + 1; - ssi_private = devm_kzalloc(&pdev->dev, sizeof(*ssi_private) + strlen(p), - GFP_KERNEL); + ssi_private = devm_kzalloc(&pdev->dev, sizeof(*ssi_private), + GFP_KERNEL); if (!ssi_private) { dev_err(&pdev->dev, "could not allocate DAI object\n"); return -ENOMEM; }
- strcpy(ssi_private->name, p); - ssi_private->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter"); ssi_private->hw_type = hw_type; @@ -1055,7 +1051,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) memcpy(&ssi_private->cpu_dai_drv, &fsl_ssi_dai_template, sizeof(fsl_ssi_dai_template)); } - ssi_private->cpu_dai_drv.name = ssi_private->name; + ssi_private->cpu_dai_drv.name = dev_name(&pdev->dev);
/* Get the addresses and IRQ */ ret = of_address_to_resource(np, 0, &res); @@ -1203,7 +1199,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) * different writeable interrupt status registers. */ if (ssi_private->use_dma) { - /* The 'name' should not have any slashes in it. */ ret = devm_request_irq(&pdev->dev, ssi_private->irq, fsl_ssi_isr, 0, ssi_private->name, ssi_private);
Move imx specific probe code to a seperate function. It reduces the size of the probe() function and makes the code and error handling easier to understand.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 185 +++++++++++++++++++++++++++--------------------- 1 file changed, 103 insertions(+), 82 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 65d9da1..418c646 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -992,6 +992,100 @@ static void make_lowercase(char *s) } }
+static int fsl_ssi_imx_probe(struct platform_device *pdev, + struct fsl_ssi_private *ssi_private) +{ + struct device_node *np = pdev->dev.of_node; + u32 dma_events[2], dmas[4]; + bool shared; + int ret; + + ssi_private->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(ssi_private->clk)) { + ret = PTR_ERR(ssi_private->clk); + dev_err(&pdev->dev, "could not get clock: %d\n", ret); + return ret; + } + + ret = clk_prepare_enable(ssi_private->clk); + if (ret) { + dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", ret); + return ret; + } + + /* For those SLAVE implementations, we ingore non-baudclk cases + * and, instead, abandon MASTER mode that needs baud clock. + */ + ssi_private->baudclk = devm_clk_get(&pdev->dev, "baud"); + if (IS_ERR(ssi_private->baudclk)) + dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", + PTR_ERR(ssi_private->baudclk)); + else + clk_prepare_enable(ssi_private->baudclk); + + /* + * We have burstsize be "fifo_depth - 2" to match the SSI + * watermark setting in fsl_ssi_startup(). + */ + ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2; + ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2; + ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + + offsetof(struct ccsr_ssi, stx0); + ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + + offsetof(struct ccsr_ssi, srx0); + ssi_private->dma_params_tx.filter_data = &ssi_private->filter_data_tx; + ssi_private->dma_params_rx.filter_data = &ssi_private->filter_data_rx; + + if (!of_property_read_bool(pdev->dev.of_node, "dmas") && + ssi_private->use_dma) { + /* + * FIXME: This is a temporary solution until all + * necessary dma drivers support the generic dma + * bindings. + */ + ret = of_property_read_u32_array(pdev->dev.of_node, + "fsl,ssi-dma-events", dma_events, 2); + if (ret && ssi_private->use_dma) { + dev_err(&pdev->dev, "could not get dma events but fsl-ssi is configured to use DMA\n"); + goto error_dma_events; + } + } + /* Should this be merge with the above? */ + if (!of_property_read_u32_array(pdev->dev.of_node, "dmas", dmas, 4) + && dmas[2] == IMX_DMATYPE_SSI_DUAL) { + ssi_private->use_dual_fifo = true; + /* When using dual fifo mode, we need to keep watermark + * as even numbers due to dma script limitation. + */ + ssi_private->dma_params_tx.maxburst &= ~0x1; + ssi_private->dma_params_rx.maxburst &= ~0x1; + } + + shared = of_device_is_compatible(of_get_parent(np), "fsl,spba-bus"); + + imx_pcm_dma_params_init_data(&ssi_private->filter_data_tx, + dma_events[0], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); + imx_pcm_dma_params_init_data(&ssi_private->filter_data_rx, + dma_events[1], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); + + return 0; + +error_dma_events: + if (!IS_ERR(ssi_private->baudclk)) + clk_disable_unprepare(ssi_private->baudclk); + clk_disable_unprepare(ssi_private->clk); + + return ret; +} + +static void fsl_ssi_imx_clean(struct platform_device *pdev, + struct fsl_ssi_private *ssi_private) +{ + if (!IS_ERR(ssi_private->baudclk)) + clk_disable_unprepare(ssi_private->baudclk); + clk_disable_unprepare(ssi_private->clk); +} + static int fsl_ssi_probe(struct platform_device *pdev) { struct fsl_ssi_private *ssi_private; @@ -1004,7 +1098,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) const uint32_t *iprop; struct resource res; char name[64]; - bool shared; bool ac97 = false;
/* SSIs that are not connected on the board should have a @@ -1118,80 +1211,11 @@ static int fsl_ssi_probe(struct platform_device *pdev)
if (hw_type == FSL_SSI_MX21 || hw_type == FSL_SSI_MX51 || hw_type == FSL_SSI_MX35) { - u32 dma_events[2], dmas[4]; ssi_private->ssi_on_imx = true;
- ssi_private->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(ssi_private->clk)) { - ret = PTR_ERR(ssi_private->clk); - dev_err(&pdev->dev, "could not get clock: %d\n", ret); + ret = fsl_ssi_imx_probe(pdev, ssi_private); + if (ret) goto error_irqmap; - } - ret = clk_prepare_enable(ssi_private->clk); - if (ret) { - dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n", - ret); - goto error_irqmap; - } - - /* For those SLAVE implementations, we ingore non-baudclk cases - * and, instead, abandon MASTER mode that needs baud clock. - */ - ssi_private->baudclk = devm_clk_get(&pdev->dev, "baud"); - if (IS_ERR(ssi_private->baudclk)) - dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", - PTR_ERR(ssi_private->baudclk)); - else - clk_prepare_enable(ssi_private->baudclk); - - /* - * We have burstsize be "fifo_depth - 2" to match the SSI - * watermark setting in fsl_ssi_startup(). - */ - ssi_private->dma_params_tx.maxburst = - ssi_private->fifo_depth - 2; - ssi_private->dma_params_rx.maxburst = - ssi_private->fifo_depth - 2; - ssi_private->dma_params_tx.addr = - ssi_private->ssi_phys + offsetof(struct ccsr_ssi, stx0); - ssi_private->dma_params_rx.addr = - ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0); - ssi_private->dma_params_tx.filter_data = - &ssi_private->filter_data_tx; - ssi_private->dma_params_rx.filter_data = - &ssi_private->filter_data_rx; - if (!of_property_read_bool(pdev->dev.of_node, "dmas") && - ssi_private->use_dma) { - /* - * FIXME: This is a temporary solution until all - * necessary dma drivers support the generic dma - * bindings. - */ - ret = of_property_read_u32_array(pdev->dev.of_node, - "fsl,ssi-dma-events", dma_events, 2); - if (ret && ssi_private->use_dma) { - dev_err(&pdev->dev, "could not get dma events but fsl-ssi is configured to use DMA\n"); - goto error_clk; - } - } - /* Should this be merge with the above? */ - if (!of_property_read_u32_array(pdev->dev.of_node, "dmas", dmas, 4) - && dmas[2] == IMX_DMATYPE_SSI_DUAL) { - ssi_private->use_dual_fifo = true; - /* When using dual fifo mode, we need to keep watermark - * as even numbers due to dma script limitation. - */ - ssi_private->dma_params_tx.maxburst &= ~0x1; - ssi_private->dma_params_rx.maxburst &= ~0x1; - } - - shared = of_device_is_compatible(of_get_parent(np), - "fsl,spba-bus"); - - imx_pcm_dma_params_init_data(&ssi_private->filter_data_tx, - dma_events[0], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); - imx_pcm_dma_params_init_data(&ssi_private->filter_data_rx, - dma_events[1], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); }
/* @@ -1206,7 +1230,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) if (ret < 0) { dev_err(&pdev->dev, "could not claim irq %u\n", ssi_private->irq); - goto error_clk; + goto error_irq; } }
@@ -1298,11 +1322,9 @@ error_dbgfs: error_dev: device_remove_file(&pdev->dev, dev_attr);
-error_clk: +error_irq: if (ssi_private->ssi_on_imx) { - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); - clk_disable_unprepare(ssi_private->clk); + fsl_ssi_imx_clean(pdev, ssi_private); }
error_irqmap: @@ -1321,11 +1343,10 @@ static int fsl_ssi_remove(struct platform_device *pdev) if (!ssi_private->new_binding) platform_device_unregister(ssi_private->pdev); snd_soc_unregister_component(&pdev->dev); - if (ssi_private->ssi_on_imx) { - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); - clk_disable_unprepare(ssi_private->clk); - } + + if (ssi_private->ssi_on_imx) + fsl_ssi_imx_clean(pdev, ssi_private); + if (ssi_private->irq_stats) irq_dispose_mapping(ssi_private->irq);
Simplify dma DT property handling. fsl,ssi-dma-events is not used anymore. It passes invalid data to imx_pcm_dma_params_init_data() which copies some data into an imx dma struct. This struct is never used in imx-dma or imx-sdma because of generic OF DMA handling. The "fsl,ssi-dma-events" is not used anywhere in dts files.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 40 +++------------------------------------- 1 file changed, 3 insertions(+), 37 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 418c646..8072926 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -168,8 +168,6 @@ struct fsl_ssi_private { struct clk *clk; struct snd_dmaengine_dai_dma_data dma_params_tx; struct snd_dmaengine_dai_dma_data dma_params_rx; - struct imx_dma_data filter_data_tx; - struct imx_dma_data filter_data_rx; struct imx_pcm_fiq_params fiq_params; /* Register values for rx/tx configuration */ struct fsl_ssi_rxtx_reg_val rxtx_reg_val; @@ -996,8 +994,7 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, struct fsl_ssi_private *ssi_private) { struct device_node *np = pdev->dev.of_node; - u32 dma_events[2], dmas[4]; - bool shared; + u32 dmas[4]; int ret;
ssi_private->clk = devm_clk_get(&pdev->dev, NULL); @@ -1033,26 +1030,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, offsetof(struct ccsr_ssi, stx0); ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + offsetof(struct ccsr_ssi, srx0); - ssi_private->dma_params_tx.filter_data = &ssi_private->filter_data_tx; - ssi_private->dma_params_rx.filter_data = &ssi_private->filter_data_rx;
- if (!of_property_read_bool(pdev->dev.of_node, "dmas") && - ssi_private->use_dma) { - /* - * FIXME: This is a temporary solution until all - * necessary dma drivers support the generic dma - * bindings. - */ - ret = of_property_read_u32_array(pdev->dev.of_node, - "fsl,ssi-dma-events", dma_events, 2); - if (ret && ssi_private->use_dma) { - dev_err(&pdev->dev, "could not get dma events but fsl-ssi is configured to use DMA\n"); - goto error_dma_events; - } - } - /* Should this be merge with the above? */ - if (!of_property_read_u32_array(pdev->dev.of_node, "dmas", dmas, 4) - && dmas[2] == IMX_DMATYPE_SSI_DUAL) { + ret = !of_property_read_u32_array(np, "dmas", dmas, 4); + if (ssi_private->use_dma && !ret && dmas[2] == IMX_DMATYPE_SSI_DUAL) { ssi_private->use_dual_fifo = true; /* When using dual fifo mode, we need to keep watermark * as even numbers due to dma script limitation. @@ -1061,21 +1041,7 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, ssi_private->dma_params_rx.maxburst &= ~0x1; }
- shared = of_device_is_compatible(of_get_parent(np), "fsl,spba-bus"); - - imx_pcm_dma_params_init_data(&ssi_private->filter_data_tx, - dma_events[0], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); - imx_pcm_dma_params_init_data(&ssi_private->filter_data_rx, - dma_events[1], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI); - return 0; - -error_dma_events: - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); - clk_disable_unprepare(ssi_private->clk); - - return ret; }
static void fsl_ssi_imx_clean(struct platform_device *pdev,
Reorder the probe function to be able to move the second imx-specific block to the seperate imx probe function. The patch also removes some comments/variables/code that are not used anymore or could be simply replaced by other variables.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 113 +++++++++++++++++++++++------------------------- 1 file changed, 53 insertions(+), 60 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 8072926..af1b82f 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -159,7 +159,6 @@ struct fsl_ssi_private { bool imx_ac97; bool use_dma; bool baudclk_locked; - bool irq_stats; bool offline_config; bool use_dual_fifo; u8 i2s_mode; @@ -991,7 +990,7 @@ static void make_lowercase(char *s) }
static int fsl_ssi_imx_probe(struct platform_device *pdev, - struct fsl_ssi_private *ssi_private) + struct fsl_ssi_private *ssi_private, void __iomem *iomem) { struct device_node *np = pdev->dev.of_node; u32 dmas[4]; @@ -1041,12 +1040,47 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, ssi_private->dma_params_rx.maxburst &= ~0x1; }
+ if (!ssi_private->use_dma) { + + /* + * Some boards use an incompatible codec. To get it + * working, we are using imx-fiq-pcm-audio, that + * can handle those codecs. DMA is not possible in this + * situation. + */ + + ssi_private->fiq_params.irq = ssi_private->irq; + ssi_private->fiq_params.base = iomem; + ssi_private->fiq_params.dma_params_rx = + &ssi_private->dma_params_rx; + ssi_private->fiq_params.dma_params_tx = + &ssi_private->dma_params_tx; + + ret = imx_pcm_fiq_init(pdev, &ssi_private->fiq_params); + if (ret) + goto error_pcm; + } else { + ret = imx_pcm_dma_init(pdev); + if (ret) + goto error_pcm; + } + return 0; + +error_pcm: + if (!IS_ERR(ssi_private->baudclk)) + clk_disable_unprepare(ssi_private->baudclk); + + clk_disable_unprepare(ssi_private->clk); + + return ret; }
static void fsl_ssi_imx_clean(struct platform_device *pdev, struct fsl_ssi_private *ssi_private) { + if (!ssi_private->use_dma) + imx_pcm_fiq_exit(pdev); if (!IS_ERR(ssi_private->baudclk)) clk_disable_unprepare(ssi_private->baudclk); clk_disable_unprepare(ssi_private->clk); @@ -1056,7 +1090,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) { struct fsl_ssi_private *ssi_private; int ret = 0; - struct device_attribute *dev_attr = NULL; struct device_node *np = pdev->dev.of_node; const struct of_device_id *of_id; enum fsl_ssi_type hw_type; @@ -1149,6 +1182,8 @@ static int fsl_ssi_probe(struct platform_device *pdev) ssi_private->baudclk_locked = false; spin_lock_init(&ssi_private->baudclk_lock);
+ dev_set_drvdata(&pdev->dev, ssi_private); + /* * imx51 and later SoCs have a slightly different IP that allows the * SSI configuration while the SSI unit is running. @@ -1179,20 +1214,22 @@ static int fsl_ssi_probe(struct platform_device *pdev) hw_type == FSL_SSI_MX35) { ssi_private->ssi_on_imx = true;
- ret = fsl_ssi_imx_probe(pdev, ssi_private); + ret = fsl_ssi_imx_probe(pdev, ssi_private, ssi_private->ssi); if (ret) goto error_irqmap; }
- /* - * Enable interrupts only for MCP8610 and MX51. The other MXs have - * different writeable interrupt status registers. - */ + ret = snd_soc_register_component(&pdev->dev, &fsl_ssi_component, + &ssi_private->cpu_dai_drv, 1); + if (ret) { + dev_err(&pdev->dev, "failed to register DAI: %d\n", ret); + goto error_asoc_register; + } + if (ssi_private->use_dma) { ret = devm_request_irq(&pdev->dev, ssi_private->irq, fsl_ssi_isr, 0, ssi_private->name, ssi_private); - ssi_private->irq_stats = true; if (ret < 0) { dev_err(&pdev->dev, "could not claim irq %u\n", ssi_private->irq); @@ -1200,46 +1237,9 @@ static int fsl_ssi_probe(struct platform_device *pdev) } }
- /* Register with ASoC */ - dev_set_drvdata(&pdev->dev, ssi_private); - - ret = snd_soc_register_component(&pdev->dev, &fsl_ssi_component, - &ssi_private->cpu_dai_drv, 1); - if (ret) { - dev_err(&pdev->dev, "failed to register DAI: %d\n", ret); - goto error_dev; - } - ret = fsl_ssi_debugfs_create(&ssi_private->dbg_stats, &pdev->dev); if (ret) - goto error_dbgfs; - - if (ssi_private->ssi_on_imx) { - if (!ssi_private->use_dma) { - - /* - * Some boards use an incompatible codec. To get it - * working, we are using imx-fiq-pcm-audio, that - * can handle those codecs. DMA is not possible in this - * situation. - */ - - ssi_private->fiq_params.irq = ssi_private->irq; - ssi_private->fiq_params.base = ssi_private->ssi; - ssi_private->fiq_params.dma_params_rx = - &ssi_private->dma_params_rx; - ssi_private->fiq_params.dma_params_tx = - &ssi_private->dma_params_tx; - - ret = imx_pcm_fiq_init(pdev, &ssi_private->fiq_params); - if (ret) - goto error_pcm; - } else { - ret = imx_pcm_dma_init(pdev); - if (ret) - goto error_pcm; - } - } + goto error_asoc_register;
/* * If codec-handle property is missing from SSI node, we assume @@ -1269,32 +1269,25 @@ static int fsl_ssi_probe(struct platform_device *pdev) if (IS_ERR(ssi_private->pdev)) { ret = PTR_ERR(ssi_private->pdev); dev_err(&pdev->dev, "failed to register platform: %d\n", ret); - goto error_dai; + goto error_sound_card; }
done: return 0;
-error_dai: - if (ssi_private->ssi_on_imx && !ssi_private->use_dma) - imx_pcm_fiq_exit(pdev); - -error_pcm: +error_sound_card: fsl_ssi_debugfs_remove(&ssi_private->dbg_stats);
-error_dbgfs: +error_irq: snd_soc_unregister_component(&pdev->dev);
-error_dev: - device_remove_file(&pdev->dev, dev_attr); - -error_irq: +error_asoc_register: if (ssi_private->ssi_on_imx) { fsl_ssi_imx_clean(pdev, ssi_private); }
error_irqmap: - if (ssi_private->irq_stats) + if (ssi_private->use_dma) irq_dispose_mapping(ssi_private->irq);
return ret; @@ -1313,7 +1306,7 @@ static int fsl_ssi_remove(struct platform_device *pdev) if (ssi_private->ssi_on_imx) fsl_ssi_imx_clean(pdev, ssi_private);
- if (ssi_private->irq_stats) + if (ssi_private->use_dma) irq_dispose_mapping(ssi_private->irq);
return 0;
There are some variables defined in struct fsl_ssi_private that describe states that are also described by other variables.
This patch adds some helper functions that return exactly the same information based on available variables. This helps to clean up struct fsl_ssi_private and remove them from the probe function.
It also removes some not really used variables (new_binding, name).
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 117 ++++++++++++++++++++++++++---------------------- 1 file changed, 64 insertions(+), 53 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index af1b82f..794f25f 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -143,7 +143,6 @@ struct fsl_ssi_rxtx_reg_val { * @cpu_dai: the CPU DAI for this device * @dev_attr: the sysfs device attribute structure * @stats: SSI statistics - * @name: name for this device */ struct fsl_ssi_private { struct ccsr_ssi __iomem *ssi; @@ -152,14 +151,11 @@ struct fsl_ssi_private { unsigned int fifo_depth; struct snd_soc_dai_driver cpu_dai_drv; struct platform_device *pdev; + unsigned int dai_fmt;
enum fsl_ssi_type hw_type; - bool new_binding; - bool ssi_on_imx; - bool imx_ac97; bool use_dma; bool baudclk_locked; - bool offline_config; bool use_dual_fifo; u8 i2s_mode; spinlock_t baudclk_lock; @@ -172,8 +168,6 @@ struct fsl_ssi_private { struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
struct fsl_ssi_dbg dbg_stats; - - char name[1]; };
static const struct of_device_id fsl_ssi_ids[] = { @@ -185,6 +179,54 @@ static const struct of_device_id fsl_ssi_ids[] = { }; MODULE_DEVICE_TABLE(of, fsl_ssi_ids);
+static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) +{ + return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); +} + +static bool fsl_ssi_on_imx(struct fsl_ssi_private *ssi_private) +{ + switch (ssi_private->hw_type) { + case FSL_SSI_MX21: + case FSL_SSI_MX35: + case FSL_SSI_MX51: + return true; + case FSL_SSI_MCP8610: + return false; + } + + return false; +} + +/* + * imx51 and later SoCs have a slightly different IP that allows the + * SSI configuration while the SSI unit is running. + * + * More important, it is necessary on those SoCs to configure the + * sperate TX/RX DMA bits just before starting the stream + * (fsl_ssi_trigger). The SDMA unit has to be configured before fsl_ssi + * sends any DMA requests to the SDMA unit, otherwise it is not defined + * how the SDMA unit handles the DMA request. + * + * SDMA units are present on devices starting at imx35 but the imx35 + * reference manual states that the DMA bits should not be changed + * while the SSI unit is running (SSIEN). So we support the necessary + * online configuration of fsl-ssi starting at imx51. + */ +static bool fsl_ssi_offline_config(struct fsl_ssi_private *ssi_private) +{ + switch (ssi_private->hw_type) { + case FSL_SSI_MCP8610: + case FSL_SSI_MX21: + case FSL_SSI_MX35: + return true; + case FSL_SSI_MX51: + return false; + } + + return true; +} + /** * fsl_ssi_isr: SSI interrupt handler * @@ -317,7 +359,7 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, * reconfiguration, so we have to enable all necessary flags at once * even if we do not use them later (capture and playback configuration) */ - if (ssi_private->offline_config) { + if (fsl_ssi_offline_config(ssi_private)) { if ((enable && !nr_active_streams) || (!enable && !keep_active)) fsl_ssi_rxtx_config(ssi_private, enable); @@ -393,7 +435,7 @@ static void fsl_ssi_setup_reg_vals(struct fsl_ssi_private *ssi_private) reg->tx.stcr = CCSR_SSI_STCR_TFEN0; reg->tx.scr = 0;
- if (!ssi_private->imx_ac97) { + if (!fsl_ssi_is_ac97(ssi_private)) { reg->rx.scr = CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_RE; reg->rx.sier |= CCSR_SSI_SIER_RFF0_EN; reg->tx.scr = CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE; @@ -458,7 +500,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, snd_soc_dai_get_drvdata(rtd->cpu_dai); unsigned long flags;
- if (!dai->active && !ssi_private->imx_ac97) { + if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags); ssi_private->baudclk_locked = false; spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); @@ -524,7 +566,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, else write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
- if (!ssi_private->imx_ac97) + if (!fsl_ssi_is_ac97(ssi_private)) write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_NET | CCSR_SSI_SCR_I2S_MODE_MASK, channels == 1 ? 0 : ssi_private->i2s_mode); @@ -542,6 +584,8 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) u32 strcr = 0, stcr, srcr, scr, mask; u8 wm;
+ ssi_private->dai_fmt = fmt; + fsl_ssi_setup_reg_vals(ssi_private);
scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); @@ -840,7 +884,7 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, else fsl_ssi_rx_config(ssi_private, false);
- if (!ssi_private->imx_ac97 && (read_ssi(&ssi->scr) & + if (!fsl_ssi_is_ac97(ssi_private) && (read_ssi(&ssi->scr) & (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags); ssi_private->baudclk_locked = false; @@ -852,7 +896,7 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, return -EINVAL; }
- if (ssi_private->imx_ac97) { + if (fsl_ssi_is_ac97(ssi_private)) { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) write_ssi(CCSR_SSI_SOR_TX_CLR, &ssi->sor); else @@ -866,7 +910,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(dai);
- if (ssi_private->ssi_on_imx && ssi_private->use_dma) { + if (fsl_ssi_on_imx(ssi_private) && ssi_private->use_dma) { dai->playback_dma_data = &ssi_private->dma_params_tx; dai->capture_dma_data = &ssi_private->dma_params_rx; } @@ -1135,7 +1179,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) sizeof(fsl_ssi_ac97_dai));
fsl_ac97_data = ssi_private; - ssi_private->imx_ac97 = true;
snd_soc_set_ac97_ops_of_reset(&fsl_ssi_ac97_ops, pdev); } else { @@ -1184,36 +1227,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, ssi_private);
- /* - * imx51 and later SoCs have a slightly different IP that allows the - * SSI configuration while the SSI unit is running. - * - * More important, it is necessary on those SoCs to configure the - * sperate TX/RX DMA bits just before starting the stream - * (fsl_ssi_trigger). The SDMA unit has to be configured before fsl_ssi - * sends any DMA requests to the SDMA unit, otherwise it is not defined - * how the SDMA unit handles the DMA request. - * - * SDMA units are present on devices starting at imx35 but the imx35 - * reference manual states that the DMA bits should not be changed - * while the SSI unit is running (SSIEN). So we support the necessary - * online configuration of fsl-ssi starting at imx51. - */ - switch (hw_type) { - case FSL_SSI_MCP8610: - case FSL_SSI_MX21: - case FSL_SSI_MX35: - ssi_private->offline_config = true; - break; - case FSL_SSI_MX51: - ssi_private->offline_config = false; - break; - } - - if (hw_type == FSL_SSI_MX21 || hw_type == FSL_SSI_MX51 || - hw_type == FSL_SSI_MX35) { - ssi_private->ssi_on_imx = true; - + if (fsl_ssi_on_imx(ssi_private)) { ret = fsl_ssi_imx_probe(pdev, ssi_private, ssi_private->ssi); if (ret) goto error_irqmap; @@ -1228,7 +1242,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
if (ssi_private->use_dma) { ret = devm_request_irq(&pdev->dev, ssi_private->irq, - fsl_ssi_isr, 0, ssi_private->name, + fsl_ssi_isr, 0, dev_name(&pdev->dev), ssi_private); if (ret < 0) { dev_err(&pdev->dev, "could not claim irq %u\n", @@ -1246,10 +1260,8 @@ static int fsl_ssi_probe(struct platform_device *pdev) * that the machine driver uses new binding which does not require * SSI driver to trigger machine driver's probe. */ - if (!of_get_property(np, "codec-handle", NULL)) { - ssi_private->new_binding = true; + if (!of_get_property(np, "codec-handle", NULL)) goto done; - }
/* Trigger the machine driver's probe function. The platform driver * name of the machine driver is taken from /compatible property of the @@ -1282,9 +1294,8 @@ error_irq: snd_soc_unregister_component(&pdev->dev);
error_asoc_register: - if (ssi_private->ssi_on_imx) { + if (fsl_ssi_on_imx(ssi_private)) fsl_ssi_imx_clean(pdev, ssi_private); - }
error_irqmap: if (ssi_private->use_dma) @@ -1299,11 +1310,11 @@ static int fsl_ssi_remove(struct platform_device *pdev)
fsl_ssi_debugfs_remove(&ssi_private->dbg_stats);
- if (!ssi_private->new_binding) + if (ssi_private->pdev) platform_device_unregister(ssi_private->pdev); snd_soc_unregister_component(&pdev->dev);
- if (ssi_private->ssi_on_imx) + if (fsl_ssi_on_imx(ssi_private)) fsl_ssi_imx_clean(pdev, ssi_private);
if (ssi_private->use_dma)
From: Sascha Hauer s.hauer@pengutronix.de
Introduce a SoC data struct which contains the differences between the different SoCs this driver supports. This makes it easy to support more differences without having to introduce a new switch/case each time.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 125 ++++++++++++++++++++++-------------------------- 1 file changed, 57 insertions(+), 68 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 794f25f..f2255cb 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -132,6 +132,12 @@ struct fsl_ssi_rxtx_reg_val { struct fsl_ssi_reg_val tx; };
+struct fsl_ssi_soc_data { + bool imx; + bool offline_config; + u32 sisr_write_mask; +}; + /** * fsl_ssi_private: per-SSI private data * @@ -153,7 +159,6 @@ struct fsl_ssi_private { struct platform_device *pdev; unsigned int dai_fmt;
- enum fsl_ssi_type hw_type; bool use_dma; bool baudclk_locked; bool use_dual_fifo; @@ -168,35 +173,9 @@ struct fsl_ssi_private { struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
struct fsl_ssi_dbg dbg_stats; -};
-static const struct of_device_id fsl_ssi_ids[] = { - { .compatible = "fsl,mpc8610-ssi", .data = (void *) FSL_SSI_MCP8610}, - { .compatible = "fsl,imx51-ssi", .data = (void *) FSL_SSI_MX51}, - { .compatible = "fsl,imx35-ssi", .data = (void *) FSL_SSI_MX35}, - { .compatible = "fsl,imx21-ssi", .data = (void *) FSL_SSI_MX21}, - {} + const struct fsl_ssi_soc_data *soc; }; -MODULE_DEVICE_TABLE(of, fsl_ssi_ids); - -static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) -{ - return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); -} - -static bool fsl_ssi_on_imx(struct fsl_ssi_private *ssi_private) -{ - switch (ssi_private->hw_type) { - case FSL_SSI_MX21: - case FSL_SSI_MX35: - case FSL_SSI_MX51: - return true; - case FSL_SSI_MCP8610: - return false; - } - - return false; -}
/* * imx51 and later SoCs have a slightly different IP that allows the @@ -213,18 +192,48 @@ static bool fsl_ssi_on_imx(struct fsl_ssi_private *ssi_private) * while the SSI unit is running (SSIEN). So we support the necessary * online configuration of fsl-ssi starting at imx51. */ -static bool fsl_ssi_offline_config(struct fsl_ssi_private *ssi_private) -{ - switch (ssi_private->hw_type) { - case FSL_SSI_MCP8610: - case FSL_SSI_MX21: - case FSL_SSI_MX35: - return true; - case FSL_SSI_MX51: - return false; - }
- return true; +static struct fsl_ssi_soc_data fsl_ssi_mpc8610 = { + .imx = false, + .offline_config = true, + .sisr_write_mask = CCSR_SSI_SISR_RFRC | CCSR_SSI_SISR_TFRC | + CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 | + CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1, +}; + +static struct fsl_ssi_soc_data fsl_ssi_imx21 = { + .imx = true, + .offline_config = true, + .sisr_write_mask = 0, +}; + +static struct fsl_ssi_soc_data fsl_ssi_imx35 = { + .imx = true, + .offline_config = true, + .sisr_write_mask = CCSR_SSI_SISR_RFRC | CCSR_SSI_SISR_TFRC | + CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 | + CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1, +}; + +static struct fsl_ssi_soc_data fsl_ssi_imx51 = { + .imx = true, + .offline_config = false, + .sisr_write_mask = CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 | + CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1, +}; + +static const struct of_device_id fsl_ssi_ids[] = { + { .compatible = "fsl,mpc8610-ssi", .data = &fsl_ssi_mpc8610 }, + { .compatible = "fsl,imx51-ssi", .data = &fsl_ssi_imx51 }, + { .compatible = "fsl,imx35-ssi", .data = &fsl_ssi_imx35 }, + { .compatible = "fsl,imx21-ssi", .data = &fsl_ssi_imx21 }, + {} +}; +MODULE_DEVICE_TABLE(of, fsl_ssi_ids); + +static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) +{ + return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); }
/** @@ -245,25 +254,6 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) struct ccsr_ssi __iomem *ssi = ssi_private->ssi; __be32 sisr; __be32 sisr2; - __be32 sisr_write_mask = 0; - - switch (ssi_private->hw_type) { - case FSL_SSI_MX21: - sisr_write_mask = 0; - break; - - case FSL_SSI_MCP8610: - case FSL_SSI_MX35: - sisr_write_mask = CCSR_SSI_SISR_RFRC | CCSR_SSI_SISR_TFRC | - CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 | - CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1; - break; - - case FSL_SSI_MX51: - sisr_write_mask = CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 | - CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1; - break; - }
/* We got an interrupt, so read the status register to see what we were interrupted for. We mask it with the Interrupt Enable register @@ -271,7 +261,7 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) */ sisr = read_ssi(&ssi->sisr);
- sisr2 = sisr & sisr_write_mask; + sisr2 = sisr & ssi_private->soc->sisr_write_mask; /* Clear the bits that we set */ if (sisr2) write_ssi(sisr2, &ssi->sisr); @@ -359,7 +349,7 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, * reconfiguration, so we have to enable all necessary flags at once * even if we do not use them later (capture and playback configuration) */ - if (fsl_ssi_offline_config(ssi_private)) { + if (ssi_private->soc->offline_config) { if ((enable && !nr_active_streams) || (!enable && !keep_active)) fsl_ssi_rxtx_config(ssi_private, enable); @@ -910,7 +900,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(dai);
- if (fsl_ssi_on_imx(ssi_private) && ssi_private->use_dma) { + if (ssi_private->soc->imx && ssi_private->use_dma) { dai->playback_dma_data = &ssi_private->dma_params_tx; dai->capture_dma_data = &ssi_private->dma_params_rx; } @@ -1136,7 +1126,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) int ret = 0; struct device_node *np = pdev->dev.of_node; const struct of_device_id *of_id; - enum fsl_ssi_type hw_type; const char *p, *sprop; const uint32_t *iprop; struct resource res; @@ -1151,9 +1140,8 @@ static int fsl_ssi_probe(struct platform_device *pdev) return -ENODEV;
of_id = of_match_device(fsl_ssi_ids, &pdev->dev); - if (!of_id) + if (!of_id || !of_id->data) return -EINVAL; - hw_type = (enum fsl_ssi_type) of_id->data;
sprop = of_get_property(np, "fsl,mode", NULL); if (!sprop) { @@ -1170,9 +1158,10 @@ static int fsl_ssi_probe(struct platform_device *pdev) return -ENOMEM; }
+ ssi_private->soc = of_id->data; + ssi_private->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter"); - ssi_private->hw_type = hw_type;
if (ac97) { memcpy(&ssi_private->cpu_dai_drv, &fsl_ssi_ac97_dai, @@ -1227,7 +1216,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, ssi_private);
- if (fsl_ssi_on_imx(ssi_private)) { + if (ssi_private->soc->imx) { ret = fsl_ssi_imx_probe(pdev, ssi_private, ssi_private->ssi); if (ret) goto error_irqmap; @@ -1294,7 +1283,7 @@ error_irq: snd_soc_unregister_component(&pdev->dev);
error_asoc_register: - if (fsl_ssi_on_imx(ssi_private)) + if (ssi_private->soc->imx) fsl_ssi_imx_clean(pdev, ssi_private);
error_irqmap: @@ -1314,7 +1303,7 @@ static int fsl_ssi_remove(struct platform_device *pdev) platform_device_unregister(ssi_private->pdev); snd_soc_unregister_component(&pdev->dev);
- if (fsl_ssi_on_imx(ssi_private)) + if (ssi_private->soc->imx) fsl_ssi_imx_clean(pdev, ssi_private);
if (ssi_private->use_dma)
From: Sascha Hauer s.hauer@pengutronix.de
Enable baudclk only when it is used. The baudclock is only needed in master mode and only when thr driver is active, so move enabling to fsl_ssi_startup and disabling to fsl_ssi_shutdown.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index f2255cb..d6d3f0a3 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); }
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{ + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == + SND_SOC_DAIFMT_CBS_CFS; +} + /** * fsl_ssi_isr: SSI interrupt handler * @@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); unsigned long flags; + int ret;
if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags); @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
+ if (fsl_ssi_is_i2s_master(ssi_private)) { + ret = clk_prepare_enable(ssi_private->baudclk); + if (ret) + return ret; + } + /* When using dual fifo mode, it is safer to ensure an even period * size. If appearing to an odd number while DMA always starts its * task from fifo0, fifo1 would be neglected at the end of each @@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, return 0; }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct fsl_ssi_private *ssi_private = + snd_soc_dai_get_drvdata(rtd->cpu_dai); + + if (fsl_ssi_is_i2s_master(ssi_private)) + clk_disable_unprepare(ssi_private->baudclk); +} + /** * fsl_ssi_hw_params - program the sample size * @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
+ if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); + return -EINVAL; + } + fsl_ssi_setup_reg_vals(ssi_private);
scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); @@ -910,6 +939,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .startup = fsl_ssi_startup, + .shutdown = fsl_ssi_shutdown, .hw_params = fsl_ssi_hw_params, .set_fmt = fsl_ssi_set_dai_fmt, .set_sysclk = fsl_ssi_set_dai_sysclk, @@ -1050,8 +1080,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, if (IS_ERR(ssi_private->baudclk)) dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", PTR_ERR(ssi_private->baudclk)); - else - clk_prepare_enable(ssi_private->baudclk);
/* * We have burstsize be "fifo_depth - 2" to match the SSI @@ -1102,9 +1130,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, return 0;
error_pcm: - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); - clk_disable_unprepare(ssi_private->clk);
return ret; @@ -1115,8 +1140,6 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, { if (!ssi_private->use_dma) imx_pcm_fiq_exit(pdev); - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); clk_disable_unprepare(ssi_private->clk); }
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote:
From: Sascha Hauer s.hauer@pengutronix.de
Enable baudclk only when it is used. The baudclock is only needed in master mode and only when thr driver is active, so move enabling to fsl_ssi_startup and disabling to fsl_ssi_shutdown.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de
sound/soc/fsl/fsl_ssi.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index f2255cb..d6d3f0a3 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); }
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
+}
/**
- fsl_ssi_isr: SSI interrupt handler
@@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); unsigned long flags;
int ret;
if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags);
@@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
- if (fsl_ssi_is_i2s_master(ssi_private)) {
ret = clk_prepare_enable(ssi_private->baudclk);
if (ret)
return ret;
- }
- /* When using dual fifo mode, it is safer to ensure an even period
- size. If appearing to an odd number while DMA always starts its
- task from fifo0, fifo1 would be neglected at the end of each
@@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, return 0; }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
- if (fsl_ssi_is_i2s_master(ssi_private))
clk_disable_unprepare(ssi_private->baudclk);
+}
/**
- fsl_ssi_hw_params - program the sample size
@@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
- if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) {
dev_err(cpu_dai->dev, "no baudclk needed for master mode\n");
return -EINVAL;
- }
I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case.
A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry.
Any suggestion?
Thank you, Nicolin
fsl_ssi_setup_reg_vals(ssi_private);
scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); @@ -910,6 +939,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .startup = fsl_ssi_startup,
- .shutdown = fsl_ssi_shutdown, .hw_params = fsl_ssi_hw_params, .set_fmt = fsl_ssi_set_dai_fmt, .set_sysclk = fsl_ssi_set_dai_sysclk,
@@ -1050,8 +1080,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, if (IS_ERR(ssi_private->baudclk)) dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", PTR_ERR(ssi_private->baudclk));
else
clk_prepare_enable(ssi_private->baudclk);
/*
- We have burstsize be "fifo_depth - 2" to match the SSI
@@ -1102,9 +1130,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, return 0;
error_pcm:
if (!IS_ERR(ssi_private->baudclk))
clk_disable_unprepare(ssi_private->baudclk);
clk_disable_unprepare(ssi_private->clk);
return ret;
@@ -1115,8 +1140,6 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, { if (!ssi_private->use_dma) imx_pcm_fiq_exit(pdev);
- if (!IS_ERR(ssi_private->baudclk))
clk_disable_unprepare(ssi_private->clk);clk_disable_unprepare(ssi_private->baudclk);
}
-- 1.9.1
Hi,
On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote:
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote:
From: Sascha Hauer s.hauer@pengutronix.de
Enable baudclk only when it is used. The baudclock is only needed in master mode and only when thr driver is active, so move enabling to fsl_ssi_startup and disabling to fsl_ssi_shutdown.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de
sound/soc/fsl/fsl_ssi.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index f2255cb..d6d3f0a3 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); }
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
+}
/**
- fsl_ssi_isr: SSI interrupt handler
@@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); unsigned long flags;
int ret;
if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags);
@@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
- if (fsl_ssi_is_i2s_master(ssi_private)) {
ret = clk_prepare_enable(ssi_private->baudclk);
if (ret)
return ret;
- }
- /* When using dual fifo mode, it is safer to ensure an even period
- size. If appearing to an odd number while DMA always starts its
- task from fifo0, fifo1 would be neglected at the end of each
@@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, return 0; }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
- if (fsl_ssi_is_i2s_master(ssi_private))
clk_disable_unprepare(ssi_private->baudclk);
+}
/**
- fsl_ssi_hw_params - program the sample size
@@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
- if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) {
dev_err(cpu_dai->dev, "no baudclk needed for master mode\n");
return -EINVAL;
- }
I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case.
A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry.
Any suggestion?
Yes this is a problem. Although I am not really convinced of the concept of setting up the DAIs in hw_params, we should support it as there are some users.
Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times", so I am not sure if we can directly use clk_prepare_enable or if we need to remember the clock state?
Thanks,
Markus
Hi Markus,
Please allow me to drop some content.
On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote:
On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote:
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote:
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
+}
@@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
- if (fsl_ssi_is_i2s_master(ssi_private)) {
ret = clk_prepare_enable(ssi_private->baudclk);
if (ret)
return ret;
- }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
- if (fsl_ssi_is_i2s_master(ssi_private))
clk_disable_unprepare(ssi_private->baudclk);
+}
@@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
- if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) {
dev_err(cpu_dai->dev, "no baudclk needed for master mode\n");
return -EINVAL;
- }
I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case.
A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry.
Any suggestion?
Yes this is a problem. Although I am not really convinced of the concept of setting up the DAIs in hw_params, we should support it as there are some users.
I think it might be an old fashion way. I saw quite a lot machine drivers doing the DAI format setting in its hw_params(). Even in the current tree we can also grep some out by using:
find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt"
So it should be plausible considering there might be some special cases, using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example.
Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times",
That's a good question. IIRC, snd_pcm_release_substream() would call its hw_free() right before calling snd_pcm_close(). So there would be at least twice for hw_free()'s execution.
so I am not sure if we can directly use clk_prepare_enable or if we need to remember the clock state?
We need to if applying that. Or put them into trigger() as an alternative approach even if it sounds weird to me.
Thanks, Nicolin
Hi Nicolin,
On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote:
Hi Markus,
Please allow me to drop some content.
On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote:
On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote:
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote:
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
+}
@@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
- if (fsl_ssi_is_i2s_master(ssi_private)) {
ret = clk_prepare_enable(ssi_private->baudclk);
if (ret)
return ret;
- }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
- if (fsl_ssi_is_i2s_master(ssi_private))
clk_disable_unprepare(ssi_private->baudclk);
+}
@@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
- if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) {
dev_err(cpu_dai->dev, "no baudclk needed for master mode\n");
return -EINVAL;
- }
I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case.
A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry.
Any suggestion?
Yes this is a problem. Although I am not really convinced of the concept of setting up the DAIs in hw_params, we should support it as there are some users.
I think it might be an old fashion way. I saw quite a lot machine drivers doing the DAI format setting in its hw_params(). Even in the current tree we can also grep some out by using:
find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt"
So it should be plausible considering there might be some special cases, using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example.
Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times",
That's a good question. IIRC, snd_pcm_release_substream() would call its hw_free() right before calling snd_pcm_close(). So there would be at least twice for hw_free()'s execution.
so I am not sure if we can directly use clk_prepare_enable or if we need to remember the clock state?
We need to if applying that. Or put them into trigger() as an alternative approach even if it sounds weird to me.
Unfortunately trigger() is not an option, it also may be called multiple times for one command due to strange error handling in the layers above. For example a failing START command in the DMA driver may lead to a STOP command in the fsl-ssi driver without a previous START command.
So it seems the only option is to save the clock state? I will move it to hw_params() and hw_free().
Thanks,
Markus
On Wed, Apr 16, 2014 at 10:43:31AM +0200, Markus Pargmann wrote:
Hi Nicolin,
On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote:
Hi Markus,
Please allow me to drop some content.
On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote:
On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote:
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote:
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
+}
@@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); }
- if (fsl_ssi_is_i2s_master(ssi_private)) {
ret = clk_prepare_enable(ssi_private->baudclk);
if (ret)
return ret;
- }
+static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct fsl_ssi_private *ssi_private =
snd_soc_dai_get_drvdata(rtd->cpu_dai);
- if (fsl_ssi_is_i2s_master(ssi_private))
clk_disable_unprepare(ssi_private->baudclk);
+}
@@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
ssi_private->dai_fmt = fmt;
- if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) {
dev_err(cpu_dai->dev, "no baudclk needed for master mode\n");
return -EINVAL;
- }
I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case.
A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry.
Any suggestion?
Yes this is a problem. Although I am not really convinced of the concept of setting up the DAIs in hw_params, we should support it as there are some users.
I think it might be an old fashion way. I saw quite a lot machine drivers doing the DAI format setting in its hw_params(). Even in the current tree we can also grep some out by using:
find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt"
So it should be plausible considering there might be some special cases, using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example.
Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times",
That's a good question. IIRC, snd_pcm_release_substream() would call its hw_free() right before calling snd_pcm_close(). So there would be at least twice for hw_free()'s execution.
so I am not sure if we can directly use clk_prepare_enable or if we need to remember the clock state?
We need to if applying that. Or put them into trigger() as an alternative approach even if it sounds weird to me.
Unfortunately trigger() is not an option, it also may be called multiple times for one command due to strange error handling in the layers above. For example a failing START command in the DMA driver may lead to a STOP command in the fsl-ssi driver without a previous START command.
Nice judgement. Okay, just forget about trigger() then.
So it seems the only option is to save the clock state? I will move it to hw_params() and hw_free().
I think we might wait other's opinion on this topic or try to send an extra patch for it meanwhile you can just drop the single patch from this series so that the other patches will be easily applied since they are really useful and do fix a few problems.
Cheers, Nicolin
Thanks,
Markus
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote:
On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote:
Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times",
That's a good question. IIRC, snd_pcm_release_substream() would call its hw_free() right before calling snd_pcm_close(). So there would be at least twice for hw_free()'s execution.
There may be any number of hw_params() calls.
Markus Pargmann wrote:
Is there always exactly one hw_free call for one hw_params call?
I don't think so. It's been a while since I've worked on this code, but I do remember noticing that hw_params could be called multiple times, whereas hw_free was called only once. This made a big impact on my design of the code.
The simple soundcard binding has its own way for specifying the dai format. To be able to use this binding we have to make the fsl,mode property optional. As the property is used in existing devicetrees keep the option around for compatibility reasons.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index d6d3f0a3..5c4b4bb 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -588,12 +588,9 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, return 0; }
-/** - * fsl_ssi_set_dai_fmt - configure Digital Audio Interface Format. - */ -static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) +static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, + unsigned int fmt) { - struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); struct ccsr_ssi __iomem *ssi = ssi_private->ssi; u32 strcr = 0, stcr, srcr, scr, mask; u8 wm; @@ -601,7 +598,7 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) ssi_private->dai_fmt = fmt;
if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { - dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); + dev_err(&ssi_private->pdev->dev, "no baudclk needed for master mode\n"); return -EINVAL; }
@@ -736,6 +733,17 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) fsl_ssi_setup_ac97(ssi_private);
return 0; + +} + +/** + * fsl_ssi_set_dai_fmt - configure Digital Audio Interface Format. + */ +static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) +{ + struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); + + return _fsl_ssi_set_dai_fmt(ssi_private, fmt); }
/** @@ -1153,7 +1161,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) const uint32_t *iprop; struct resource res; char name[64]; - bool ac97 = false;
/* SSIs that are not connected on the board should have a * status = "disabled" @@ -1166,14 +1173,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) if (!of_id || !of_id->data) return -EINVAL;
- sprop = of_get_property(np, "fsl,mode", NULL); - if (!sprop) { - dev_err(&pdev->dev, "fsl,mode property is necessary\n"); - return -EINVAL; - } - if (!strcmp(sprop, "ac97-slave")) - ac97 = true; - ssi_private = devm_kzalloc(&pdev->dev, sizeof(*ssi_private), GFP_KERNEL); if (!ssi_private) { @@ -1183,10 +1182,19 @@ static int fsl_ssi_probe(struct platform_device *pdev)
ssi_private->soc = of_id->data;
+ sprop = of_get_property(np, "fsl,mode", NULL); + if (sprop) { + if (!strcmp(sprop, "ac97-slave")) + ssi_private->dai_fmt = SND_SOC_DAIFMT_AC97; + else if (!strcmp(sprop, "i2s-slave")) + ssi_private->dai_fmt = SND_SOC_DAIFMT_I2S | + SND_SOC_DAIFMT_CBM_CFM; + } + ssi_private->use_dma = !of_property_read_bool(np, "fsl,fiq-stream-filter");
- if (ac97) { + if (fsl_ssi_is_ac97(ssi_private)) { memcpy(&ssi_private->cpu_dai_drv, &fsl_ssi_ac97_dai, sizeof(fsl_ssi_ac97_dai));
@@ -1297,6 +1305,9 @@ static int fsl_ssi_probe(struct platform_device *pdev) }
done: + if (ssi_private->dai_fmt) + _fsl_ssi_set_dai_fmt(ssi_private, ssi_private->dai_fmt); + return 0;
error_sound_card:
When the fsl-ssi unit is used in i2s slave mode, it is possible that the SSI unit starts transmitting data on the wrong channel. This happens because the SSI does not synchronize with the left-right-clock by default.
This patch enables transmit enable synchronization.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 1 + sound/soc/fsl/fsl_ssi.h | 1 + 2 files changed, 2 insertions(+)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 5c4b4bb..d5f91b1 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -605,6 +605,7 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, fsl_ssi_setup_reg_vals(ssi_private);
scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); + scr |= CCSR_SSI_SCR_SYNC_TX_FS;
mask = CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFDIR | CCSR_SSI_STCR_TXDIR | CCSR_SSI_STCR_TSCKP | CCSR_SSI_STCR_TFSI | CCSR_SSI_STCR_TFSL | diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h index 2e95dd7..71c3e7e 100644 --- a/sound/soc/fsl/fsl_ssi.h +++ b/sound/soc/fsl/fsl_ssi.h @@ -39,6 +39,7 @@ struct ccsr_ssi { __be32 saccdis; /* 0x.0058 - SSI AC97 Channel Disable Register */ };
+#define CCSR_SSI_SCR_SYNC_TX_FS 0x00001000 #define CCSR_SSI_SCR_RFR_CLK_DIS 0x00000800 #define CCSR_SSI_SCR_TFR_CLK_DIS 0x00000400 #define CCSR_SSI_SCR_TCH_EN 0x00000100
From: Sascha Hauer s.hauer@pengutronix.de
fsl_ssi_set_dai_sysclk will be called from fsl_ssi_hw_params in the next patch. Move up to avoid forward declaration and to keep the next patch more readable. No functional change.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 190 ++++++++++++++++++++++++------------------------ 1 file changed, 96 insertions(+), 94 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index d5f91b1..74f5a91 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -533,6 +533,102 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, }
/** + * fsl_ssi_set_dai_sysclk - configure Digital Audio Interface bit clock + * + * Note: This function can be only called when using SSI as DAI master + * + * Quick instruction for parameters: + * freq: Output BCLK frequency = samplerate * 32 (fixed) * channels + * dir: SND_SOC_CLOCK_OUT -> TxBCLK, SND_SOC_CLOCK_IN -> RxBCLK. + */ +static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, + int clk_id, unsigned int freq, int dir) +{ + struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; + u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; + unsigned long flags, clkrate, baudrate, tmprate; + u64 sub, savesub = 100000; + + /* Don't apply it to any non-baudclk circumstance */ + if (IS_ERR(ssi_private->baudclk)) + return -EINVAL; + + /* It should be already enough to divide clock by setting pm alone */ + psr = 0; + div2 = 0; + + factor = (div2 + 1) * (7 * psr + 1) * 2; + + for (i = 0; i < 255; i++) { + /* The bclk rate must be smaller than 1/5 sysclk rate */ + if (factor * (i + 1) < 5) + continue; + + tmprate = freq * factor * (i + 2); + clkrate = clk_round_rate(ssi_private->baudclk, tmprate); + + do_div(clkrate, factor); + afreq = (u32)clkrate / (i + 1); + + if (freq == afreq) + sub = 0; + else if (freq / afreq == 1) + sub = freq - afreq; + else if (afreq / freq == 1) + sub = afreq - freq; + else + continue; + + /* Calculate the fraction */ + sub *= 100000; + do_div(sub, freq); + + if (sub < savesub) { + baudrate = tmprate; + savesub = sub; + pm = i; + } + + /* We are lucky */ + if (savesub == 0) + break; + } + + /* No proper pm found if it is still remaining the initial value */ + if (pm == 999) { + dev_err(cpu_dai->dev, "failed to handle the required sysclk\n"); + return -EINVAL; + } + + stccr = CCSR_SSI_SxCCR_PM(pm + 1) | (div2 ? CCSR_SSI_SxCCR_DIV2 : 0) | + (psr ? CCSR_SSI_SxCCR_PSR : 0); + mask = CCSR_SSI_SxCCR_PM_MASK | CCSR_SSI_SxCCR_DIV2 | + CCSR_SSI_SxCCR_PSR; + + if (dir == SND_SOC_CLOCK_OUT || synchronous) + write_ssi_mask(&ssi->stccr, mask, stccr); + else + write_ssi_mask(&ssi->srccr, mask, stccr); + + spin_lock_irqsave(&ssi_private->baudclk_lock, flags); + if (!ssi_private->baudclk_locked) { + ret = clk_set_rate(ssi_private->baudclk, baudrate); + if (ret) { + spin_unlock_irqrestore(&ssi_private->baudclk_lock, + flags); + dev_err(cpu_dai->dev, "failed to set baudclk rate\n"); + return -EINVAL; + } + ssi_private->baudclk_locked = true; + } + spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); + + return 0; +} + +/** * fsl_ssi_hw_params - program the sample size * * Most of the SSI registers have been programmed in the startup function, @@ -748,100 +844,6 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) }
/** - * fsl_ssi_set_dai_sysclk - configure Digital Audio Interface bit clock - * - * Note: This function can be only called when using SSI as DAI master - * - * Quick instruction for parameters: - * freq: Output BCLK frequency = samplerate * 32 (fixed) * channels - * dir: SND_SOC_CLOCK_OUT -> TxBCLK, SND_SOC_CLOCK_IN -> RxBCLK. - */ -static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, - int clk_id, unsigned int freq, int dir) -{ - struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; - int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; - u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; - unsigned long flags, clkrate, baudrate, tmprate; - u64 sub, savesub = 100000; - - /* Don't apply it to any non-baudclk circumstance */ - if (IS_ERR(ssi_private->baudclk)) - return -EINVAL; - - /* It should be already enough to divide clock by setting pm alone */ - psr = 0; - div2 = 0; - - factor = (div2 + 1) * (7 * psr + 1) * 2; - - for (i = 0; i < 255; i++) { - /* The bclk rate must be smaller than 1/5 sysclk rate */ - if (factor * (i + 1) < 5) - continue; - - tmprate = freq * factor * (i + 2); - clkrate = clk_round_rate(ssi_private->baudclk, tmprate); - - do_div(clkrate, factor); - afreq = (u32)clkrate / (i + 1); - - if (freq == afreq) - sub = 0; - else if (freq / afreq == 1) - sub = freq - afreq; - else if (afreq / freq == 1) - sub = afreq - freq; - else - continue; - - /* Calculate the fraction */ - sub *= 100000; - do_div(sub, freq); - - if (sub < savesub) { - baudrate = tmprate; - savesub = sub; - pm = i; - } - - /* We are lucky */ - if (savesub == 0) - break; - } - - /* No proper pm found if it is still remaining the initial value */ - if (pm == 999) { - dev_err(cpu_dai->dev, "failed to handle the required sysclk\n"); - return -EINVAL; - } - - stccr = CCSR_SSI_SxCCR_PM(pm + 1) | (div2 ? CCSR_SSI_SxCCR_DIV2 : 0) | - (psr ? CCSR_SSI_SxCCR_PSR : 0); - mask = CCSR_SSI_SxCCR_PM_MASK | CCSR_SSI_SxCCR_DIV2 | CCSR_SSI_SxCCR_PSR; - - if (dir == SND_SOC_CLOCK_OUT || synchronous) - write_ssi_mask(&ssi->stccr, mask, stccr); - else - write_ssi_mask(&ssi->srccr, mask, stccr); - - spin_lock_irqsave(&ssi_private->baudclk_lock, flags); - if (!ssi_private->baudclk_locked) { - ret = clk_set_rate(ssi_private->baudclk, baudrate); - if (ret) { - spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); - dev_err(cpu_dai->dev, "failed to set baudclk rate\n"); - return -EINVAL; - } - ssi_private->baudclk_locked = true; - } - spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); - - return 0; -} - -/** * fsl_ssi_set_dai_tdm_slot - set TDM slot number * * Note: This function can be only called when using SSI as DAI master
From: Sascha Hauer s.hauer@pengutronix.de
The fsl_ssi driver uses the .set_sysclk callback to configure the bitclock for master mode. This is unnecessary since the bitclock is known in hw_params. This patch removes the .set_sysclk callback and configures the bitclock from .hw_params.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 74f5a91..e3a93f0 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -533,7 +533,7 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, }
/** - * fsl_ssi_set_dai_sysclk - configure Digital Audio Interface bit clock + * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock * * Note: This function can be only called when using SSI as DAI master * @@ -541,8 +541,8 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, * freq: Output BCLK frequency = samplerate * 32 (fixed) * channels * dir: SND_SOC_CLOCK_OUT -> TxBCLK, SND_SOC_CLOCK_IN -> RxBCLK. */ -static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, - int clk_id, unsigned int freq, int dir) +static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, + struct snd_soc_dai *cpu_dai, unsigned int freq) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); struct ccsr_ssi __iomem *ssi = ssi_private->ssi; @@ -607,7 +607,7 @@ static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, mask = CCSR_SSI_SxCCR_PM_MASK | CCSR_SSI_SxCCR_DIV2 | CCSR_SSI_SxCCR_PSR;
- if (dir == SND_SOC_CLOCK_OUT || synchronous) + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || synchronous) write_ssi_mask(&ssi->stccr, mask, stccr); else write_ssi_mask(&ssi->srccr, mask, stccr); @@ -651,6 +651,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, snd_pcm_format_width(params_format(hw_params)); u32 wl = CCSR_SSI_SxCCR_WL(sample_size); int enabled = read_ssi(&ssi->scr) & CCSR_SSI_SCR_SSIEN; + int ret;
/* * If we're in synchronous mode, and the SSI is already enabled, @@ -659,6 +660,14 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, if (enabled && ssi_private->cpu_dai_drv.symmetric_rates) return 0;
+ if (fsl_ssi_is_i2s_master(ssi_private)) { + ret = fsl_ssi_set_bclk(substream, cpu_dai, + params_channels(hw_params) * 32 * + params_rate(hw_params)); + if (ret) + return ret; + } + /* * FIXME: The documentation says that SxCCR[WL] should not be * modified while the SSI is enabled. The only time this can @@ -953,7 +962,6 @@ static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .shutdown = fsl_ssi_shutdown, .hw_params = fsl_ssi_hw_params, .set_fmt = fsl_ssi_set_dai_fmt, - .set_sysclk = fsl_ssi_set_dai_sysclk, .set_tdm_slot = fsl_ssi_set_dai_tdm_slot, .trigger = fsl_ssi_trigger, };
From: Sascha Hauer s.hauer@pengutronix.de
The baudclock_locked variable is only used in functions which are serialized anyway from the core. No need to have a lock around the variable, so remove it.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index e3a93f0..e5fa626 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -163,7 +163,6 @@ struct fsl_ssi_private { bool baudclk_locked; bool use_dual_fifo; u8 i2s_mode; - spinlock_t baudclk_lock; struct clk *baudclk; struct clk *clk; struct snd_dmaengine_dai_dma_data dma_params_tx; @@ -494,14 +493,10 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = substream->private_data; struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); - unsigned long flags; int ret;
- if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { - spin_lock_irqsave(&ssi_private->baudclk_lock, flags); + if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) ssi_private->baudclk_locked = false; - spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); - }
if (fsl_ssi_is_i2s_master(ssi_private)) { ret = clk_prepare_enable(ssi_private->baudclk); @@ -548,7 +543,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, struct ccsr_ssi __iomem *ssi = ssi_private->ssi; int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; - unsigned long flags, clkrate, baudrate, tmprate; + unsigned long clkrate, baudrate, tmprate; u64 sub, savesub = 100000;
/* Don't apply it to any non-baudclk circumstance */ @@ -612,18 +607,14 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, else write_ssi_mask(&ssi->srccr, mask, stccr);
- spin_lock_irqsave(&ssi_private->baudclk_lock, flags); if (!ssi_private->baudclk_locked) { ret = clk_set_rate(ssi_private->baudclk, baudrate); if (ret) { - spin_unlock_irqrestore(&ssi_private->baudclk_lock, - flags); dev_err(cpu_dai->dev, "failed to set baudclk rate\n"); return -EINVAL; } ssi_private->baudclk_locked = true; } - spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags);
return 0; } @@ -905,7 +896,6 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_pcm_runtime *rtd = substream->private_data; struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); struct ccsr_ssi __iomem *ssi = ssi_private->ssi; - unsigned long flags;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: @@ -924,11 +914,9 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, fsl_ssi_rx_config(ssi_private, false);
if (!fsl_ssi_is_ac97(ssi_private) && (read_ssi(&ssi->scr) & - (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0) { - spin_lock_irqsave(&ssi_private->baudclk_lock, flags); + (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0) ssi_private->baudclk_locked = false; - spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); - } + break;
default: @@ -1254,7 +1242,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) ssi_private->fifo_depth = 8;
ssi_private->baudclk_locked = false; - spin_lock_init(&ssi_private->baudclk_lock);
dev_set_drvdata(&pdev->dev, ssi_private);
From: Sascha Hauer s.hauer@pengutronix.de
Allow to set the bitlcock exactly once when the ssi unit is opened and do not touch the clock until both streams are closed again. We should not unlock the bitclock in the stop trigger since a trigger could be the result of an underrun and doesn't mean the bitclock can be reconfigured again.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index e5fa626..e97e30f 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -495,9 +495,6 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, snd_soc_dai_get_drvdata(rtd->cpu_dai); int ret;
- if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) - ssi_private->baudclk_locked = false; - if (fsl_ssi_is_i2s_master(ssi_private)) { ret = clk_prepare_enable(ssi_private->baudclk); if (ret) @@ -525,6 +522,9 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
if (fsl_ssi_is_i2s_master(ssi_private)) clk_disable_unprepare(ssi_private->baudclk); + + if (!dai->active) + ssi_private->baudclk_locked = 0; }
/** @@ -562,7 +562,11 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, continue;
tmprate = freq * factor * (i + 2); - clkrate = clk_round_rate(ssi_private->baudclk, tmprate); + + if (ssi_private->baudclk_locked) + clkrate = clk_get_rate(ssi_private->baudclk); + else + clkrate = clk_round_rate(ssi_private->baudclk, tmprate);
do_div(clkrate, factor); afreq = (u32)clkrate / (i + 1); @@ -912,11 +916,6 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, fsl_ssi_tx_config(ssi_private, false); else fsl_ssi_rx_config(ssi_private, false); - - if (!fsl_ssi_is_ac97(ssi_private) && (read_ssi(&ssi->scr) & - (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0) - ssi_private->baudclk_locked = false; - break;
default: @@ -1241,8 +1240,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) /* Older 8610 DTs didn't have the fifo-depth property */ ssi_private->fifo_depth = 8;
- ssi_private->baudclk_locked = false; - dev_set_drvdata(&pdev->dev, ssi_private);
if (ssi_private->soc->imx) {
From: Sascha Hauer s.hauer@pengutronix.de
In i2s master mode the fsl_ssi driver depends on someone calling .set_tdm_slot correctly. In this mode though only a DC value of 2 is allowed, so set it in this case and no longer depend on .set_tdm_slot.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index e97e30f..9425299 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -719,6 +719,10 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBS_CFS: ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER; + write_ssi_mask(&ssi->stccr, CCSR_SSI_SxCCR_DC_MASK, + CCSR_SSI_SxCCR_DC(2)); + write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_DC_MASK, + CCSR_SSI_SxCCR_DC(2)); break; case SND_SOC_DAIFMT_CBM_CFM: ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE;
Reorder all variables in struct fsl_ssi_private to have groups that make sense together. The patch also updates the struct documentation.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/fsl_ssi.c | 59 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 9425299..f8a30689 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -141,35 +141,62 @@ struct fsl_ssi_soc_data { /** * fsl_ssi_private: per-SSI private data * - * @ssi: pointer to the SSI's registers - * @ssi_phys: physical address of the SSI registers + * @ssi: Pointer to the memory area * @irq: IRQ of this SSI - * @playback: the number of playback streams opened - * @capture: the number of capture streams opened - * @cpu_dai: the CPU DAI for this device - * @dev_attr: the sysfs device attribute structure - * @stats: SSI statistics + * @cpu_dai_drv: CPU DAI driver for this device + * + * @dai_fmt: DAI configuration this device is currently used with + * @i2s_mode: i2s and network mode configuration of the device. Is used to + * switch between normal and i2s/network mode + * mode depending on the number of channels + * @use_dma: DMA is used or FIQ with stream filter + * @use_dual_fifo: DMA with support for both FIFOs used + * @fifo_deph: Depth of the SSI FIFOs + * @rxtx_reg_val: Specific register settings for receive/transmit configuration + * + * @clk: SSI clock + * @baudclk: SSI baud clock for master mode + * @baudclk_locked: SSI baudclk is locked because it is in use + * + * @dma_params_tx: DMA transmit parameters + * @dma_params_rx: DMA receive parameters + * @ssi_phys: physical address of the SSI registers + * + * @fiq_params: FIQ stream filtering parameters + * + * @pdev: Pointer to pdev used for deprecated fsl-ssi sound card + * + * @dbg_stats: Debugging statistics + * + * @soc: SoC specifc data */ struct fsl_ssi_private { struct ccsr_ssi __iomem *ssi; - dma_addr_t ssi_phys; unsigned int irq; - unsigned int fifo_depth; struct snd_soc_dai_driver cpu_dai_drv; - struct platform_device *pdev; - unsigned int dai_fmt;
+ unsigned int dai_fmt; + u8 i2s_mode; bool use_dma; - bool baudclk_locked; bool use_dual_fifo; - u8 i2s_mode; - struct clk *baudclk; + unsigned int fifo_depth; + struct fsl_ssi_rxtx_reg_val rxtx_reg_val; + struct clk *clk; + struct clk *baudclk; + bool baudclk_locked; + + /* DMA params */ struct snd_dmaengine_dai_dma_data dma_params_tx; struct snd_dmaengine_dai_dma_data dma_params_rx; + dma_addr_t ssi_phys; + + /* params for non-dma FIQ stream filtered mode */ struct imx_pcm_fiq_params fiq_params; - /* Register values for rx/tx configuration */ - struct fsl_ssi_rxtx_reg_val rxtx_reg_val; + + /* Used when using fsl-ssi as sound-card. This is only used by ppc and + * should be replaced with simple-sound-card. */ + struct platform_device *pdev;
struct fsl_ssi_dbg dbg_stats;
This patch replaces the ssi specific functions write_ssi, read_ssi and write_ssi_mask by standard regmap function calls.
Signed-off-by: Markus Pargmann mpa@pengutronix.de --- sound/soc/fsl/Kconfig | 1 + sound/soc/fsl/fsl_ssi.c | 241 ++++++++++++++++++++++++++---------------------- sound/soc/fsl/fsl_ssi.h | 50 +++++----- 3 files changed, 158 insertions(+), 134 deletions(-)
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index 338a916..2d6281f 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -4,6 +4,7 @@ config SND_SOC_FSL_SAI select SND_SOC_GENERIC_DMAENGINE_PCM
config SND_SOC_FSL_SSI + select REGMAP_MMIO tristate
config SND_SOC_FSL_SPDIF diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index f8a30689..641787c 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -53,25 +53,6 @@ #include "fsl_ssi.h" #include "imx-pcm.h"
-#ifdef PPC -#define read_ssi(addr) in_be32(addr) -#define write_ssi(val, addr) out_be32(addr, val) -#define write_ssi_mask(addr, clear, set) clrsetbits_be32(addr, clear, set) -#else -#define read_ssi(addr) readl(addr) -#define write_ssi(val, addr) writel(val, addr) -/* - * FIXME: Proper locking should be added at write_ssi_mask caller level - * to ensure this register read/modify/write sequence is race free. - */ -static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set) -{ - u32 val = readl(addr); - val = (val & ~clear) | set; - writel(val, addr); -} -#endif - /** * FSLSSI_I2S_RATES: sample rates supported by the I2S * @@ -131,6 +112,13 @@ struct fsl_ssi_rxtx_reg_val { struct fsl_ssi_reg_val rx; struct fsl_ssi_reg_val tx; }; +static const struct regmap_config fsl_ssi_regconfig = { + .max_register = CCSR_SSI_SACCDIS, + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, + .val_format_endian = REGMAP_ENDIAN_NATIVE, +};
struct fsl_ssi_soc_data { bool imx; @@ -141,7 +129,7 @@ struct fsl_ssi_soc_data { /** * fsl_ssi_private: per-SSI private data * - * @ssi: Pointer to the memory area + * @reg: Pointer to the regmap registers * @irq: IRQ of this SSI * @cpu_dai_drv: CPU DAI driver for this device * @@ -171,7 +159,7 @@ struct fsl_ssi_soc_data { * @soc: SoC specifc data */ struct fsl_ssi_private { - struct ccsr_ssi __iomem *ssi; + struct regmap *regs; unsigned int irq; struct snd_soc_dai_driver cpu_dai_drv;
@@ -283,7 +271,7 @@ static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) { struct fsl_ssi_private *ssi_private = dev_id; - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; __be32 sisr; __be32 sisr2;
@@ -291,12 +279,12 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) were interrupted for. We mask it with the Interrupt Enable register so that we only check for events that we're interested in. */ - sisr = read_ssi(&ssi->sisr); + regmap_read(regs, CCSR_SSI_SISR, &sisr);
sisr2 = sisr & ssi_private->soc->sisr_write_mask; /* Clear the bits that we set */ if (sisr2) - write_ssi(sisr2, &ssi->sisr); + regmap_write(regs, CCSR_SSI_SISR, sisr2);
fsl_ssi_dbg_isr(&ssi_private->dbg_stats, sisr);
@@ -309,17 +297,26 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id) static void fsl_ssi_rxtx_config(struct fsl_ssi_private *ssi_private, bool enable) { - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; struct fsl_ssi_rxtx_reg_val *vals = &ssi_private->rxtx_reg_val;
if (enable) { - write_ssi_mask(&ssi->sier, 0, vals->rx.sier | vals->tx.sier); - write_ssi_mask(&ssi->srcr, 0, vals->rx.srcr | vals->tx.srcr); - write_ssi_mask(&ssi->stcr, 0, vals->rx.stcr | vals->tx.stcr); + regmap_update_bits(regs, CCSR_SSI_SIER, + vals->rx.sier | vals->tx.sier, + vals->rx.sier | vals->tx.sier); + regmap_update_bits(regs, CCSR_SSI_SRCR, + vals->rx.srcr | vals->tx.srcr, + vals->rx.srcr | vals->tx.srcr); + regmap_update_bits(regs, CCSR_SSI_STCR, + vals->rx.stcr | vals->tx.stcr, + vals->rx.stcr | vals->tx.stcr); } else { - write_ssi_mask(&ssi->srcr, vals->rx.srcr | vals->tx.srcr, 0); - write_ssi_mask(&ssi->stcr, vals->rx.stcr | vals->tx.stcr, 0); - write_ssi_mask(&ssi->sier, vals->rx.sier | vals->tx.sier, 0); + regmap_update_bits(regs, CCSR_SSI_SRCR, + vals->rx.srcr | vals->tx.srcr, 0); + regmap_update_bits(regs, CCSR_SSI_STCR, + vals->rx.stcr | vals->tx.stcr, 0); + regmap_update_bits(regs, CCSR_SSI_SIER, + vals->rx.sier | vals->tx.sier, 0); } }
@@ -350,13 +347,17 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi_private *ssi_private, static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, struct fsl_ssi_reg_val *vals) { - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; struct fsl_ssi_reg_val *avals; - u32 scr_val = read_ssi(&ssi->scr); - int nr_active_streams = !!(scr_val & CCSR_SSI_SCR_TE) + - !!(scr_val & CCSR_SSI_SCR_RE); + int nr_active_streams; + u32 scr_val; int keep_active;
+ regmap_read(regs, CCSR_SSI_SCR, &scr_val); + + nr_active_streams = !!(scr_val & CCSR_SSI_SCR_TE) + + !!(scr_val & CCSR_SSI_SCR_RE); + if (nr_active_streams - 1 > 0) keep_active = 1; else @@ -373,7 +374,7 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, if (!enable) { u32 scr = fsl_ssi_disable_val(vals->scr, avals->scr, keep_active); - write_ssi_mask(&ssi->scr, scr, 0); + regmap_update_bits(regs, CCSR_SSI_SCR, scr, 0); }
/* @@ -394,9 +395,9 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, * (online configuration) */ if (enable) { - write_ssi_mask(&ssi->sier, 0, vals->sier); - write_ssi_mask(&ssi->srcr, 0, vals->srcr); - write_ssi_mask(&ssi->stcr, 0, vals->stcr); + regmap_update_bits(regs, CCSR_SSI_SIER, vals->sier, vals->sier); + regmap_update_bits(regs, CCSR_SSI_SRCR, vals->srcr, vals->srcr); + regmap_update_bits(regs, CCSR_SSI_STCR, vals->stcr, vals->stcr); } else { u32 sier; u32 srcr; @@ -419,15 +420,15 @@ static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable, stcr = fsl_ssi_disable_val(vals->stcr, avals->stcr, keep_active);
- write_ssi_mask(&ssi->srcr, srcr, 0); - write_ssi_mask(&ssi->stcr, stcr, 0); - write_ssi_mask(&ssi->sier, sier, 0); + regmap_update_bits(regs, CCSR_SSI_SRCR, srcr, 0); + regmap_update_bits(regs, CCSR_SSI_STCR, stcr, 0); + regmap_update_bits(regs, CCSR_SSI_SIER, sier, 0); }
config_done: /* Enabling of subunits is done after configuration */ if (enable) - write_ssi_mask(&ssi->scr, 0, vals->scr); + regmap_update_bits(regs, CCSR_SSI_SCR, vals->scr, vals->scr); }
@@ -478,32 +479,33 @@ static void fsl_ssi_setup_reg_vals(struct fsl_ssi_private *ssi_private)
static void fsl_ssi_setup_ac97(struct fsl_ssi_private *ssi_private) { - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs;
/* * Setup the clock control register */ - write_ssi(CCSR_SSI_SxCCR_WL(17) | CCSR_SSI_SxCCR_DC(13), - &ssi->stccr); - write_ssi(CCSR_SSI_SxCCR_WL(17) | CCSR_SSI_SxCCR_DC(13), - &ssi->srccr); + regmap_write(regs, CCSR_SSI_STCCR, + CCSR_SSI_SxCCR_WL(17) | CCSR_SSI_SxCCR_DC(13)); + regmap_write(regs, CCSR_SSI_SRCCR, + CCSR_SSI_SxCCR_WL(17) | CCSR_SSI_SxCCR_DC(13));
/* * Enable AC97 mode and startup the SSI */ - write_ssi(CCSR_SSI_SACNT_AC97EN | CCSR_SSI_SACNT_FV, - &ssi->sacnt); - write_ssi(0xff, &ssi->saccdis); - write_ssi(0x300, &ssi->saccen); + regmap_write(regs, CCSR_SSI_SACNT, + CCSR_SSI_SACNT_AC97EN | CCSR_SSI_SACNT_FV); + regmap_write(regs, CCSR_SSI_SACCDIS, 0xff); + regmap_write(regs, CCSR_SSI_SACCEN, 0x300);
/* * Enable SSI, Transmit and Receive. AC97 has to communicate with the * codec before a stream is started. */ - write_ssi_mask(&ssi->scr, 0, CCSR_SSI_SCR_SSIEN | - CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE); + regmap_update_bits(regs, CCSR_SSI_SCR, + CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE, + CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE);
- write_ssi(CCSR_SSI_SOR_WAIT(3), &ssi->sor); + regmap_write(regs, CCSR_SSI_SOR, CCSR_SSI_SOR_WAIT(3)); }
/** @@ -567,7 +569,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai, unsigned int freq) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; unsigned long clkrate, baudrate, tmprate; @@ -634,9 +636,9 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, CCSR_SSI_SxCCR_PSR;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || synchronous) - write_ssi_mask(&ssi->stccr, mask, stccr); + regmap_update_bits(regs, CCSR_SSI_STCCR, mask, stccr); else - write_ssi_mask(&ssi->srccr, mask, stccr); + regmap_update_bits(regs, CCSR_SSI_SRCCR, mask, stccr);
if (!ssi_private->baudclk_locked) { ret = clk_set_rate(ssi_private->baudclk, baudrate); @@ -667,13 +669,17 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *cpu_dai) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; unsigned int channels = params_channels(hw_params); unsigned int sample_size = snd_pcm_format_width(params_format(hw_params)); u32 wl = CCSR_SSI_SxCCR_WL(sample_size); - int enabled = read_ssi(&ssi->scr) & CCSR_SSI_SCR_SSIEN; int ret; + u32 scr_val; + int enabled; + + regmap_read(regs, CCSR_SSI_SCR, &scr_val); + enabled = scr_val & CCSR_SSI_SCR_SSIEN;
/* * If we're in synchronous mode, and the SSI is already enabled, @@ -703,12 +709,14 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, /* In synchronous mode, the SSI uses STCCR for capture */ if ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) || ssi_private->cpu_dai_drv.symmetric_rates) - write_ssi_mask(&ssi->stccr, CCSR_SSI_SxCCR_WL_MASK, wl); + regmap_update_bits(regs, CCSR_SSI_STCCR, CCSR_SSI_SxCCR_WL_MASK, + wl); else - write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl); + regmap_update_bits(regs, CCSR_SSI_SRCCR, CCSR_SSI_SxCCR_WL_MASK, + wl);
if (!fsl_ssi_is_ac97(ssi_private)) - write_ssi_mask(&ssi->scr, + regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_NET | CCSR_SSI_SCR_I2S_MODE_MASK, channels == 1 ? 0 : ssi_private->i2s_mode);
@@ -718,7 +726,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, unsigned int fmt) { - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; u32 strcr = 0, stcr, srcr, scr, mask; u8 wm;
@@ -731,14 +739,17 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private,
fsl_ssi_setup_reg_vals(ssi_private);
- scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); + regmap_read(regs, CCSR_SSI_SCR, &scr); + scr &= ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); scr |= CCSR_SSI_SCR_SYNC_TX_FS;
mask = CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFDIR | CCSR_SSI_STCR_TXDIR | CCSR_SSI_STCR_TSCKP | CCSR_SSI_STCR_TFSI | CCSR_SSI_STCR_TFSL | CCSR_SSI_STCR_TEFS; - stcr = read_ssi(&ssi->stcr) & ~mask; - srcr = read_ssi(&ssi->srcr) & ~mask; + regmap_read(regs, CCSR_SSI_STCR, &stcr); + regmap_read(regs, CCSR_SSI_SRCR, &srcr); + stcr &= ~mask; + srcr &= ~mask;
ssi_private->i2s_mode = CCSR_SSI_SCR_NET; switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { @@ -746,10 +757,12 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBS_CFS: ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_MASTER; - write_ssi_mask(&ssi->stccr, CCSR_SSI_SxCCR_DC_MASK, - CCSR_SSI_SxCCR_DC(2)); - write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_DC_MASK, - CCSR_SSI_SxCCR_DC(2)); + regmap_update_bits(regs, CCSR_SSI_STCCR, + CCSR_SSI_SxCCR_DC_MASK, + CCSR_SSI_SxCCR_DC(2)); + regmap_update_bits(regs, CCSR_SSI_SRCCR, + CCSR_SSI_SxCCR_DC_MASK, + CCSR_SSI_SxCCR_DC(2)); break; case SND_SOC_DAIFMT_CBM_CFM: ssi_private->i2s_mode |= CCSR_SSI_SCR_I2S_MODE_SLAVE; @@ -828,9 +841,9 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, scr |= CCSR_SSI_SCR_SYN; }
- write_ssi(stcr, &ssi->stcr); - write_ssi(srcr, &ssi->srcr); - write_ssi(scr, &ssi->scr); + regmap_write(regs, CCSR_SSI_STCR, stcr); + regmap_write(regs, CCSR_SSI_SRCR, srcr); + regmap_write(regs, CCSR_SSI_SCR, scr);
/* * Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't @@ -848,16 +861,16 @@ static int _fsl_ssi_set_dai_fmt(struct fsl_ssi_private *ssi_private, else wm = ssi_private->fifo_depth;
- write_ssi(CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) | - CCSR_SSI_SFCSR_TFWM1(wm) | CCSR_SSI_SFCSR_RFWM1(wm), - &ssi->sfcsr); + regmap_write(regs, CCSR_SSI_SFCSR, + CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) | + CCSR_SSI_SFCSR_TFWM1(wm) | CCSR_SSI_SFCSR_RFWM1(wm));
if (ssi_private->use_dual_fifo) { - write_ssi_mask(&ssi->srcr, CCSR_SSI_SRCR_RFEN1, + regmap_update_bits(regs, CCSR_SSI_SRCR, CCSR_SSI_SRCR_RFEN1, CCSR_SSI_SRCR_RFEN1); - write_ssi_mask(&ssi->stcr, CCSR_SSI_STCR_TFEN1, + regmap_update_bits(regs, CCSR_SSI_STCR, CCSR_SSI_STCR_TFEN1, CCSR_SSI_STCR_TFEN1); - write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_TCH_EN, + regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_TCH_EN, CCSR_SSI_SCR_TCH_EN); }
@@ -887,31 +900,34 @@ static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask, u32 rx_mask, int slots, int slot_width) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs; u32 val;
/* The slot number should be >= 2 if using Network mode or I2S mode */ - val = read_ssi(&ssi->scr) & (CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_NET); + regmap_read(regs, CCSR_SSI_SCR, &val); + val &= CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_NET; if (val && slots < 2) { dev_err(cpu_dai->dev, "slot number should be >= 2 in I2S or NET\n"); return -EINVAL; }
- write_ssi_mask(&ssi->stccr, CCSR_SSI_SxCCR_DC_MASK, + regmap_update_bits(regs, CCSR_SSI_STCCR, CCSR_SSI_SxCCR_DC_MASK, CCSR_SSI_SxCCR_DC(slots)); - write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_DC_MASK, + regmap_update_bits(regs, CCSR_SSI_SRCCR, CCSR_SSI_SxCCR_DC_MASK, CCSR_SSI_SxCCR_DC(slots));
/* The register SxMSKs needs SSI to provide essential clock due to * hardware design. So we here temporarily enable SSI to set them. */ - val = read_ssi(&ssi->scr) & CCSR_SSI_SCR_SSIEN; - write_ssi_mask(&ssi->scr, 0, CCSR_SSI_SCR_SSIEN); + regmap_read(regs, CCSR_SSI_SCR, &val); + val &= CCSR_SSI_SCR_SSIEN; + regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN, + CCSR_SSI_SCR_SSIEN);
- write_ssi(tx_mask, &ssi->stmsk); - write_ssi(rx_mask, &ssi->srmsk); + regmap_write(regs, CCSR_SSI_STMSK, tx_mask); + regmap_write(regs, CCSR_SSI_SRMSK, rx_mask);
- write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, val); + regmap_update_bits(regs, CCSR_SSI_SCR, CCSR_SSI_SCR_SSIEN, val);
return 0; } @@ -930,7 +946,7 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); - struct ccsr_ssi __iomem *ssi = ssi_private->ssi; + struct regmap *regs = ssi_private->regs;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: @@ -955,9 +971,9 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
if (fsl_ssi_is_ac97(ssi_private)) { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - write_ssi(CCSR_SSI_SOR_TX_CLR, &ssi->sor); + regmap_write(regs, CCSR_SSI_SOR, CCSR_SSI_SOR_TX_CLR); else - write_ssi(CCSR_SSI_SOR_RX_CLR, &ssi->sor); + regmap_write(regs, CCSR_SSI_SOR, CCSR_SSI_SOR_RX_CLR); }
return 0; @@ -1031,7 +1047,7 @@ static struct fsl_ssi_private *fsl_ac97_data; static void fsl_ssi_ac97_write(struct snd_ac97 *ac97, unsigned short reg, unsigned short val) { - struct ccsr_ssi *ssi = fsl_ac97_data->ssi; + struct regmap *regs = fsl_ac97_data->regs; unsigned int lreg; unsigned int lval;
@@ -1040,12 +1056,12 @@ static void fsl_ssi_ac97_write(struct snd_ac97 *ac97, unsigned short reg,
lreg = reg << 12; - write_ssi(lreg, &ssi->sacadd); + regmap_write(regs, CCSR_SSI_SACADD, lreg);
lval = val << 4; - write_ssi(lval , &ssi->sacdat); + regmap_write(regs, CCSR_SSI_SACDAT, lval);
- write_ssi_mask(&ssi->sacnt, CCSR_SSI_SACNT_RDWR_MASK, + regmap_update_bits(regs, CCSR_SSI_SACNT, CCSR_SSI_SACNT_RDWR_MASK, CCSR_SSI_SACNT_WR); udelay(100); } @@ -1053,19 +1069,21 @@ static void fsl_ssi_ac97_write(struct snd_ac97 *ac97, unsigned short reg, static unsigned short fsl_ssi_ac97_read(struct snd_ac97 *ac97, unsigned short reg) { - struct ccsr_ssi *ssi = fsl_ac97_data->ssi; + struct regmap *regs = fsl_ac97_data->regs;
unsigned short val = -1; + u32 reg_val; unsigned int lreg;
lreg = (reg & 0x7f) << 12; - write_ssi(lreg, &ssi->sacadd); - write_ssi_mask(&ssi->sacnt, CCSR_SSI_SACNT_RDWR_MASK, + regmap_write(regs, CCSR_SSI_SACADD, lreg); + regmap_update_bits(regs, CCSR_SSI_SACNT, CCSR_SSI_SACNT_RDWR_MASK, CCSR_SSI_SACNT_RD);
udelay(100);
- val = (read_ssi(&ssi->sacdat) >> 4) & 0xffff; + regmap_read(regs, CCSR_SSI_SACDAT, ®_val); + val = (reg_val >> 4) & 0xffff;
return val; } @@ -1124,10 +1142,8 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, */ ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2; ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2; - ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + - offsetof(struct ccsr_ssi, stx0); - ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + - offsetof(struct ccsr_ssi, srx0); + ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0; + ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0;
ret = !of_property_read_u32_array(np, "dmas", dmas, 4); if (ssi_private->use_dma && !ret && dmas[2] == IMX_DMATYPE_SSI_DUAL) { @@ -1189,6 +1205,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) const char *p, *sprop; const uint32_t *iprop; struct resource res; + void __iomem *iomem; char name[64];
/* SSIs that are not connected on the board should have a @@ -1243,12 +1260,20 @@ static int fsl_ssi_probe(struct platform_device *pdev) dev_err(&pdev->dev, "could not determine device resources\n"); return ret; } - ssi_private->ssi = of_iomap(np, 0); - if (!ssi_private->ssi) { + ssi_private->ssi_phys = res.start; + + iomem = devm_ioremap(&pdev->dev, res.start, resource_size(&res)); + if (!iomem) { dev_err(&pdev->dev, "could not map device resources\n"); return -ENOMEM; } - ssi_private->ssi_phys = res.start; + + ssi_private->regs = devm_regmap_init_mmio(&pdev->dev, iomem, + &fsl_ssi_regconfig); + if (IS_ERR(ssi_private->regs)) { + dev_err(&pdev->dev, "Failed to init register map\n"); + return PTR_ERR(ssi_private->regs); + }
ssi_private->irq = irq_of_parse_and_map(np, 0); if (!ssi_private->irq) { @@ -1274,7 +1299,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, ssi_private);
if (ssi_private->soc->imx) { - ret = fsl_ssi_imx_probe(pdev, ssi_private, ssi_private->ssi); + ret = fsl_ssi_imx_probe(pdev, ssi_private, iomem); if (ret) goto error_irqmap; } diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h index 71c3e7e..5065105 100644 --- a/sound/soc/fsl/fsl_ssi.h +++ b/sound/soc/fsl/fsl_ssi.h @@ -12,32 +12,30 @@ #ifndef _MPC8610_I2S_H #define _MPC8610_I2S_H
-/* SSI Register Map */ -struct ccsr_ssi { - __be32 stx0; /* 0x.0000 - SSI Transmit Data Register 0 */ - __be32 stx1; /* 0x.0004 - SSI Transmit Data Register 1 */ - __be32 srx0; /* 0x.0008 - SSI Receive Data Register 0 */ - __be32 srx1; /* 0x.000C - SSI Receive Data Register 1 */ - __be32 scr; /* 0x.0010 - SSI Control Register */ - __be32 sisr; /* 0x.0014 - SSI Interrupt Status Register Mixed */ - __be32 sier; /* 0x.0018 - SSI Interrupt Enable Register */ - __be32 stcr; /* 0x.001C - SSI Transmit Configuration Register */ - __be32 srcr; /* 0x.0020 - SSI Receive Configuration Register */ - __be32 stccr; /* 0x.0024 - SSI Transmit Clock Control Register */ - __be32 srccr; /* 0x.0028 - SSI Receive Clock Control Register */ - __be32 sfcsr; /* 0x.002C - SSI FIFO Control/Status Register */ - __be32 str; /* 0x.0030 - SSI Test Register */ - __be32 sor; /* 0x.0034 - SSI Option Register */ - __be32 sacnt; /* 0x.0038 - SSI AC97 Control Register */ - __be32 sacadd; /* 0x.003C - SSI AC97 Command Address Register */ - __be32 sacdat; /* 0x.0040 - SSI AC97 Command Data Register */ - __be32 satag; /* 0x.0044 - SSI AC97 Tag Register */ - __be32 stmsk; /* 0x.0048 - SSI Transmit Time Slot Mask Register */ - __be32 srmsk; /* 0x.004C - SSI Receive Time Slot Mask Register */ - __be32 saccst; /* 0x.0050 - SSI AC97 Channel Status Register */ - __be32 saccen; /* 0x.0054 - SSI AC97 Channel Enable Register */ - __be32 saccdis; /* 0x.0058 - SSI AC97 Channel Disable Register */ -}; +/* SSI registers */ +#define CCSR_SSI_STX0 0x00 +#define CCSR_SSI_STX1 0x04 +#define CCSR_SSI_SRX0 0x08 +#define CCSR_SSI_SRX1 0x0c +#define CCSR_SSI_SCR 0x10 +#define CCSR_SSI_SISR 0x14 +#define CCSR_SSI_SIER 0x18 +#define CCSR_SSI_STCR 0x1c +#define CCSR_SSI_SRCR 0x20 +#define CCSR_SSI_STCCR 0x24 +#define CCSR_SSI_SRCCR 0x28 +#define CCSR_SSI_SFCSR 0x2c +#define CCSR_SSI_STR 0x30 +#define CCSR_SSI_SOR 0x34 +#define CCSR_SSI_SACNT 0x38 +#define CCSR_SSI_SACADD 0x3c +#define CCSR_SSI_SACDAT 0x40 +#define CCSR_SSI_SATAG 0x44 +#define CCSR_SSI_STMSK 0x48 +#define CCSR_SSI_SRMSK 0x4c +#define CCSR_SSI_SACCST 0x50 +#define CCSR_SSI_SACCEN 0x54 +#define CCSR_SSI_SACCDIS 0x58
#define CCSR_SSI_SCR_SYNC_TX_FS 0x00001000 #define CCSR_SSI_SCR_RFR_CLK_DIS 0x00000800
On Mon, Apr 14, 2014 at 03:35:30PM +0200, Markus Pargmann wrote:
This series is a cleanup of the fsl-ssi driver.
There's been some review comments on this series but only negative ones and I've not seen any testing reports either. Are the other patches OK for people?
There is no DT binding for regmap yet, as this should be solved in the regmap framework first. The current configuration should at least work on the same SoCs it was previously used. As soon as we have a common DT binding to set the endianess we replace it in this driver.
I'm really not convinced that this is a good idea to be frank - I would expect devices to still need to be involved to set defaults.
Hi,
On Thu, Apr 24, 2014 at 12:44:03PM +0100, Mark Brown wrote:
On Mon, Apr 14, 2014 at 03:35:30PM +0200, Markus Pargmann wrote:
This series is a cleanup of the fsl-ssi driver.
There's been some review comments on this series but only negative ones and I've not seen any testing reports either. Are the other patches OK for people?
There is no DT binding for regmap yet, as this should be solved in the regmap framework first. The current configuration should at least work on the same SoCs it was previously used. As soon as we have a common DT binding to set the endianess we replace it in this driver.
I'm really not convinced that this is a good idea to be frank - I would expect devices to still need to be involved to set defaults.
What do you think of some framework function like of_regmap_parse_endianess()? This would offer a common devicetree binding and the driver would still be involved.
Regards,
Markus
On Mon, Apr 28, 2014 at 10:54:53AM +0200, Markus Pargmann wrote:
What do you think of some framework function like of_regmap_parse_endianess()? This would offer a common devicetree binding and the driver would still be involved.
Something like that should be fine, Xiubo has been working on it.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Wednesday, April 30, 2014 12:22 AM To: Markus Pargmann Cc: Timur Tabi; Chen Guangyu-B42378; Estevam Fabio-R49496; Alexander Shiyan; Xiubo Li-B47053; alsa-devel@alsa-project.org; linux-arm- kernel@lists.infradead.org; kernel@pengutronix.de Subject: Re: [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
On Mon, Apr 28, 2014 at 10:54:53AM +0200, Markus Pargmann wrote:
What do you think of some framework function like of_regmap_parse_endianess()? This would offer a common devicetree binding and the driver would still be involved.
Something like that should be fine, Xiubo has been working on it.
Yes, the patch is still discussing.
Thanks,
BRs Xiubo
participants (5)
-
Li.Xiubo@freescale.com
-
Mark Brown
-
Markus Pargmann
-
Nicolin Chen
-
Timur Tabi