alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
@ 2014-04-14 13:35 Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 01/18] ASoC: fsl-ssi: Fix register values when disabling Markus Pargmann
                   ` (18 more replies)
  0 siblings, 19 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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

-- 
1.9.1

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v3 01/18] ASoC: fsl-ssi: Fix register values when disabling
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 02/18] ASoC: fsl-ssi: Move debugging to seperate file Markus Pargmann
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 02/18] ASoC: fsl-ssi: Move debugging to seperate file
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 01/18] ASoC: fsl-ssi: Fix register values when disabling Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 03/18] ASoC: fsl-ssi: Use dev_name for DAI driver struct Markus Pargmann
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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);
+}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 03/18] ASoC: fsl-ssi: Use dev_name for DAI driver struct
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 01/18] ASoC: fsl-ssi: Fix register values when disabling Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 02/18] ASoC: fsl-ssi: Move debugging to seperate file Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 04/18] ASoC: fsl-ssi: Move imx-specific probe to seperate function Markus Pargmann
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 04/18] ASoC: fsl-ssi: Move imx-specific probe to seperate function
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (2 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 03/18] ASoC: fsl-ssi: Use dev_name for DAI driver struct Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 05/18] ASoC: fsl-ssi: Remove useless DMA code Markus Pargmann
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 05/18] ASoC: fsl-ssi: Remove useless DMA code
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (3 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 04/18] ASoC: fsl-ssi: Move imx-specific probe to seperate function Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 06/18] ASoC: fsl-ssi: Cleanup probe function Markus Pargmann
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 06/18] ASoC: fsl-ssi: Cleanup probe function
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (4 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 05/18] ASoC: fsl-ssi: Remove useless DMA code Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 07/18] ASoC: fsl-ssi: Remove unnecessary variables from ssi_private Markus Pargmann
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 07/18] ASoC: fsl-ssi: Remove unnecessary variables from ssi_private
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (5 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 06/18] ASoC: fsl-ssi: Cleanup probe function Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 08/18] ASoC: fsl-ssi: introduce SoC specific data Markus Pargmann
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 08/18] ASoC: fsl-ssi: introduce SoC specific data
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (6 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 07/18] ASoC: fsl-ssi: Remove unnecessary variables from ssi_private Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used Markus Pargmann
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (7 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 08/18] ASoC: fsl-ssi: introduce SoC specific data Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 15:28   ` Nicolin Chen
  2014-04-14 13:35 ` [PATCH v3 10/18] ASoC: fsl-ssi: make fsl,mode property optional Markus Pargmann
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 10/18] ASoC: fsl-ssi: make fsl,mode property optional
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (8 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 11/18] ASoC: fsl-ssi: Transmit enable synchronization Markus Pargmann
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 11/18] ASoC: fsl-ssi: Transmit enable synchronization
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (9 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 10/18] ASoC: fsl-ssi: make fsl,mode property optional Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 12/18] ASoC: fsl-ssi: Move fsl_ssi_set_dai_sysclk above fsl_ssi_hw_params Markus Pargmann
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 12/18] ASoC: fsl-ssi: Move fsl_ssi_set_dai_sysclk above fsl_ssi_hw_params
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (10 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 11/18] ASoC: fsl-ssi: Transmit enable synchronization Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 13/18] ASoC: fsl-ssi: set bitclock in master mode from hw_params Markus Pargmann
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 13/18] ASoC: fsl-ssi: set bitclock in master mode from hw_params
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (11 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 12/18] ASoC: fsl-ssi: Move fsl_ssi_set_dai_sysclk above fsl_ssi_hw_params Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 14/18] ASoC: fsl-ssi: remove unnecessary spinlock Markus Pargmann
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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,
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 14/18] ASoC: fsl-ssi: remove unnecessary spinlock
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (12 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 13/18] ASoC: fsl-ssi: set bitclock in master mode from hw_params Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 15/18] ASoC: fsl-ssi: Allow first stream to set the bitclock Markus Pargmann
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 15/18] ASoC: fsl-ssi: Allow first stream to set the bitclock
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (13 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 14/18] ASoC: fsl-ssi: remove unnecessary spinlock Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 16/18] ASoC: fsl-ssi: Set framerate divider correctly for i2s master mode Markus Pargmann
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 16/18] ASoC: fsl-ssi: Set framerate divider correctly for i2s master mode
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (14 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 15/18] ASoC: fsl-ssi: Allow first stream to set the bitclock Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 17/18] ASoC: fsl-ssi: reorder and document fsl_ssi_private Markus Pargmann
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel

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;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 17/18] ASoC: fsl-ssi: reorder and document fsl_ssi_private
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (15 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 16/18] ASoC: fsl-ssi: Set framerate divider correctly for i2s master mode Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-14 13:35 ` [PATCH v3 18/18] ASoC: fsl-ssi: Use regmap Markus Pargmann
  2014-04-24 11:44 ` [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Mark Brown
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v3 18/18] ASoC: fsl-ssi: Use regmap
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (16 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 17/18] ASoC: fsl-ssi: reorder and document fsl_ssi_private Markus Pargmann
@ 2014-04-14 13:35 ` Markus Pargmann
  2014-04-24 11:44 ` [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Mark Brown
  18 siblings, 0 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-14 13:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, Markus Pargmann,
	linux-arm-kernel

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, &reg_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
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-14 13:35 ` [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used Markus Pargmann
@ 2014-04-14 15:28   ` Nicolin Chen
  2014-04-16  7:27     ` Markus Pargmann
  0 siblings, 1 reply; 30+ messages in thread
From: Nicolin Chen @ 2014-04-14 15:28 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Timur Tabi, Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel

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->baudclk);
>  	clk_disable_unprepare(ssi_private->clk);
>  }
>  
> -- 
> 1.9.1
> 
> 
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-14 15:28   ` Nicolin Chen
@ 2014-04-16  7:27     ` Markus Pargmann
  2014-04-16  8:08       ` Nicolin Chen
  2014-04-17 13:46       ` Timur Tabi
  0 siblings, 2 replies; 30+ messages in thread
From: Markus Pargmann @ 2014-04-16  7:27 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Timur Tabi, Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4448 bytes --]

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

-- 
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 |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-16  7:27     ` Markus Pargmann
@ 2014-04-16  8:08       ` Nicolin Chen
  2014-04-16  8:43         ` Markus Pargmann
  2014-04-16 17:42         ` Mark Brown
  2014-04-17 13:46       ` Timur Tabi
  1 sibling, 2 replies; 30+ messages in thread
From: Nicolin Chen @ 2014-04-16  8:08 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Timur Tabi, Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-16  8:43         ` Markus Pargmann
@ 2014-04-16  8:40           ` Nicolin Chen
  0 siblings, 0 replies; 30+ messages in thread
From: Nicolin Chen @ 2014-04-16  8:40 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Timur Tabi, Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel

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 |

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-16  8:08       ` Nicolin Chen
@ 2014-04-16  8:43         ` Markus Pargmann
  2014-04-16  8:40           ` Nicolin Chen
  2014-04-16 17:42         ` Mark Brown
  1 sibling, 1 reply; 30+ messages in thread
From: Markus Pargmann @ 2014-04-16  8:43 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Timur Tabi, Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4372 bytes --]

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

-- 
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 |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-16  8:08       ` Nicolin Chen
  2014-04-16  8:43         ` Markus Pargmann
@ 2014-04-16 17:42         ` Mark Brown
  1 sibling, 0 replies; 30+ messages in thread
From: Mark Brown @ 2014-04-16 17:42 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Sascha Hauer,
	Timur Tabi, Li.Xiubo, kernel, Markus Pargmann, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 553 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used
  2014-04-16  7:27     ` Markus Pargmann
  2014-04-16  8:08       ` Nicolin Chen
@ 2014-04-17 13:46       ` Timur Tabi
  1 sibling, 0 replies; 30+ messages in thread
From: Timur Tabi @ 2014-04-17 13:46 UTC (permalink / raw)
  To: Markus Pargmann, Nicolin Chen
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Mark Brown,
	Li.Xiubo, kernel, Sascha Hauer, linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
  2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
                   ` (17 preceding siblings ...)
  2014-04-14 13:35 ` [PATCH v3 18/18] ASoC: fsl-ssi: Use regmap Markus Pargmann
@ 2014-04-24 11:44 ` Mark Brown
  2014-04-28  8:54   ` Markus Pargmann
  18 siblings, 1 reply; 30+ messages in thread
From: Mark Brown @ 2014-04-24 11:44 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
  2014-04-24 11:44 ` [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Mark Brown
@ 2014-04-28  8:54   ` Markus Pargmann
  2014-04-29 16:22     ` Mark Brown
  0 siblings, 1 reply; 30+ messages in thread
From: Markus Pargmann @ 2014-04-28  8:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1306 bytes --]

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


-- 
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 |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
  2014-04-28  8:54   ` Markus Pargmann
@ 2014-04-29 16:22     ` Mark Brown
  2014-04-30  2:01       ` Li.Xiubo
  0 siblings, 1 reply; 30+ messages in thread
From: Mark Brown @ 2014-04-29 16:22 UTC (permalink / raw)
  To: Markus Pargmann
  Cc: Fabio Estevam, alsa-devel, Alexander Shiyan, Timur Tabi,
	Li.Xiubo, kernel, Nicolin Chen, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 303 bytes --]

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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup
  2014-04-29 16:22     ` Mark Brown
@ 2014-04-30  2:01       ` Li.Xiubo
  0 siblings, 0 replies; 30+ messages in thread
From: Li.Xiubo @ 2014-04-30  2:01 UTC (permalink / raw)
  To: Mark Brown, Markus Pargmann
  Cc: Fabio.Estevam, alsa-devel, Alexander Shiyan, Timur Tabi, kernel,
	guangyu.chen, linux-arm-kernel




> -----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

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2014-04-30  2:01 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-14 13:35 [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 01/18] ASoC: fsl-ssi: Fix register values when disabling Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 02/18] ASoC: fsl-ssi: Move debugging to seperate file Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 03/18] ASoC: fsl-ssi: Use dev_name for DAI driver struct Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 04/18] ASoC: fsl-ssi: Move imx-specific probe to seperate function Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 05/18] ASoC: fsl-ssi: Remove useless DMA code Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 06/18] ASoC: fsl-ssi: Cleanup probe function Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 07/18] ASoC: fsl-ssi: Remove unnecessary variables from ssi_private Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 08/18] ASoC: fsl-ssi: introduce SoC specific data Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 09/18] ASoC: fsl-ssi: Only enable baudclk when used Markus Pargmann
2014-04-14 15:28   ` Nicolin Chen
2014-04-16  7:27     ` Markus Pargmann
2014-04-16  8:08       ` Nicolin Chen
2014-04-16  8:43         ` Markus Pargmann
2014-04-16  8:40           ` Nicolin Chen
2014-04-16 17:42         ` Mark Brown
2014-04-17 13:46       ` Timur Tabi
2014-04-14 13:35 ` [PATCH v3 10/18] ASoC: fsl-ssi: make fsl,mode property optional Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 11/18] ASoC: fsl-ssi: Transmit enable synchronization Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 12/18] ASoC: fsl-ssi: Move fsl_ssi_set_dai_sysclk above fsl_ssi_hw_params Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 13/18] ASoC: fsl-ssi: set bitclock in master mode from hw_params Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 14/18] ASoC: fsl-ssi: remove unnecessary spinlock Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 15/18] ASoC: fsl-ssi: Allow first stream to set the bitclock Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 16/18] ASoC: fsl-ssi: Set framerate divider correctly for i2s master mode Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 17/18] ASoC: fsl-ssi: reorder and document fsl_ssi_private Markus Pargmann
2014-04-14 13:35 ` [PATCH v3 18/18] ASoC: fsl-ssi: Use regmap Markus Pargmann
2014-04-24 11:44 ` [PATCH v3 00/18] ASoC: fsl-ssi: Driver cleanup Mark Brown
2014-04-28  8:54   ` Markus Pargmann
2014-04-29 16:22     ` Mark Brown
2014-04-30  2:01       ` Li.Xiubo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).