[PATCH 0/6] More code cleanup for ASoC ux500

[PATCH 0/6] More code cleanup for ASoC ux500

[Fabio Baltieri]

Hi Mark,

this series contains more various code cleanup patches that I put
together while working on other stuff this drivers.

The first patch is actually a resend, as Linus Walleij agreed to drop
that code in the original thread (https://lkml.org/lkml/2013/5/17/487).

The others are just a bunch of sparse fixes and generic code cleanup and
dead code removal.

Thanks,
Fabio


Fabio Baltieri (6):
  ASoC: ux500: Drop pinctrl sleep support
  ASoC: ab8500-codec: Move codec ops on a separate structure
  ASoC: ux500: Drop dangling struct i2s_controller
  ASoC: ux500: Drop unused code from msp headers
  ASoC: ux500: Add missing mop500_ab8500.h include
  ASoC: ux500: Drop redundant msp id enumerations

 sound/soc/codecs/ab8500-codec.c | 19 ++++-------
 sound/soc/ux500/mop500_ab8500.c |  1 +
 sound/soc/ux500/ux500_msp_dai.h |  2 --
 sound/soc/ux500/ux500_msp_i2s.c | 75 ++---------------------------------------
 sound/soc/ux500/ux500_msp_i2s.h | 59 ++------------------------------
 5 files changed, 12 insertions(+), 144 deletions(-)

-- 
1.8.2

[PATCH 1/6] ASoC: ux500: Drop pinctrl sleep support

[Fabio Baltieri]

Drop pinctrl default/sleep state switching code, as it was breaking the
capture interface by putting the I2S pins in hi-z mode regardless of its
usage status, and not giving any real benefit.

Pinctrl default mode configuration is already managed automatically by a
specific pinctrl hog.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/ux500/ux500_msp_i2s.c | 56 ++---------------------------------------
 sound/soc/ux500/ux500_msp_i2s.h |  6 -----
 2 files changed, 2 insertions(+), 60 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index f2db6c9..b029b2d 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -15,7 +15,6 @@
 
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/pinctrl/consumer.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/io.h>
@@ -26,9 +25,6 @@
 
 #include "ux500_msp_i2s.h"
 
-/* MSP1/3 Tx/Rx usage protection */
-static DEFINE_SPINLOCK(msp_rxtx_lock);
-
  /* Protocol desciptors */
 static const struct msp_protdesc prot_descs[] = {
 	{ /* I2S */
@@ -356,24 +352,8 @@ static int configure_multichannel(struct ux500_msp *msp,
 
 static int enable_msp(struct ux500_msp *msp, struct ux500_msp_config *config)
 {
-	int status = 0, retval = 0;
+	int status = 0;
 	u32 reg_val_DMACR, reg_val_GCR;
-	unsigned long flags;
-
-	/* Check msp state whether in RUN or CONFIGURED Mode */
-	if (msp->msp_state == MSP_STATE_IDLE) {
-		spin_lock_irqsave(&msp_rxtx_lock, flags);
-		if (msp->pinctrl_rxtx_ref == 0 &&
-			!(IS_ERR(msp->pinctrl_p) || IS_ERR(msp->pinctrl_def))) {
-			retval = pinctrl_select_state(msp->pinctrl_p,
-						msp->pinctrl_def);
-			if (retval)
-				pr_err("could not set MSP defstate\n");
-		}
-		if (!retval)
-			msp->pinctrl_rxtx_ref++;
-		spin_unlock_irqrestore(&msp_rxtx_lock, flags);
-	}
 
 	/* Configure msp with protocol dependent settings */
 	configure_protocol(msp, config);
@@ -630,8 +610,7 @@ int ux500_msp_i2s_trigger(struct ux500_msp *msp, int cmd, int direction)
 
 int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 {
-	int status = 0, retval = 0;
-	unsigned long flags;
+	int status = 0;
 
 	dev_dbg(msp->dev, "%s: Enter (dir = 0x%01x).\n", __func__, dir);
 
@@ -643,18 +622,6 @@ int ux500_msp_i2s_close(struct ux500_msp *msp, unsigned int dir)
 			       (~(FRAME_GEN_ENABLE | SRG_ENABLE))),
 			      msp->registers + MSP_GCR);
 
-		spin_lock_irqsave(&msp_rxtx_lock, flags);
-		WARN_ON(!msp->pinctrl_rxtx_ref);
-		msp->pinctrl_rxtx_ref--;
-		if (msp->pinctrl_rxtx_ref == 0 &&
-			!(IS_ERR(msp->pinctrl_p) || IS_ERR(msp->pinctrl_sleep))) {
-			retval = pinctrl_select_state(msp->pinctrl_p,
-						msp->pinctrl_sleep);
-			if (retval)
-				pr_err("could not set MSP sleepstate\n");
-		}
-		spin_unlock_irqrestore(&msp_rxtx_lock, flags);
-
 		writel(0, msp->registers + MSP_GCR);
 		writel(0, msp->registers + MSP_TCF);
 		writel(0, msp->registers + MSP_RCF);
@@ -743,25 +710,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 	dev_dbg(&pdev->dev, "I2S device-name: '%s'\n", i2s_cont->name);
 	msp->i2s_cont = i2s_cont;
 
-	msp->pinctrl_p = pinctrl_get(msp->dev);
-	if (IS_ERR(msp->pinctrl_p))
-		dev_err(&pdev->dev, "could not get MSP pinctrl\n");
-	else {
-		msp->pinctrl_def = pinctrl_lookup_state(msp->pinctrl_p,
-						PINCTRL_STATE_DEFAULT);
-		if (IS_ERR(msp->pinctrl_def)) {
-			dev_err(&pdev->dev,
-				"could not get MSP defstate (%li)\n",
-				PTR_ERR(msp->pinctrl_def));
-		}
-		msp->pinctrl_sleep = pinctrl_lookup_state(msp->pinctrl_p,
-						PINCTRL_STATE_SLEEP);
-		if (IS_ERR(msp->pinctrl_sleep))
-			dev_err(&pdev->dev,
-				"could not get MSP idlestate (%li)\n",
-				PTR_ERR(msp->pinctrl_def));
-	}
-
 	return 0;
 }
 
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index e5cd105..8ce014e 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -528,12 +528,6 @@ struct ux500_msp {
 	int loopback_enable;
 	u32 backup_regs[MAX_MSP_BACKUP_REGS];
 	unsigned int f_bitclk;
-	/* Pin modes */
-	struct pinctrl *pinctrl_p;
-	struct pinctrl_state *pinctrl_def;
-	struct pinctrl_state *pinctrl_sleep;
-	/* Reference Count */
-	int pinctrl_rxtx_ref;
 };
 
 struct ux500_msp_dma_params {
-- 
1.8.2

Re: [PATCH 1/6] ASoC: ux500: Drop pinctrl sleep support

[Linus Walleij]

On Fri, May 24, 2013 at 12:39 PM, Fabio Baltieri
<fabio.baltieri@linaro.org> wrote:

> Drop pinctrl default/sleep state switching code, as it was breaking the
> capture interface by putting the I2S pins in hi-z mode regardless of its
> usage status, and not giving any real benefit.
>
> Pinctrl default mode configuration is already managed automatically by a
> specific pinctrl hog.
>
> Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 1/6] ASoC: ux500: Drop pinctrl sleep support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Fri, May 24, 2013 at 12:39:15PM +0200, Fabio Baltieri wrote:
> Drop pinctrl default/sleep state switching code, as it was breaking the
> capture interface by putting the I2S pins in hi-z mode regardless of its
> usage status, and not giving any real benefit.

Applied all, thanks, but please do look into fixing the other issues
with these functions.

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

[PATCH 2/6] ASoC: ab8500-codec: Move codec ops on a separate structure

[Fabio Baltieri]

Define ab8500 codec operations structure on its own rather than inline
with snd_soc_dai_drivers to clean up the code and make the style
coherent with other codec drivers.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/codecs/ab8500-codec.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/ab8500-codec.c b/sound/soc/codecs/ab8500-codec.c
index 4ca45b9..b8ba0ad 100644
--- a/sound/soc/codecs/ab8500-codec.c
+++ b/sound/soc/codecs/ab8500-codec.c
@@ -2380,6 +2380,11 @@ static int ab8500_codec_set_dai_tdm_slot(struct snd_soc_dai *dai,
 	return 0;
 }
 
+static const struct snd_soc_dai_ops ab8500_codec_ops = {
+	.set_fmt = ab8500_codec_set_dai_fmt,
+	.set_tdm_slot = ab8500_codec_set_dai_tdm_slot,
+};
+
 static struct snd_soc_dai_driver ab8500_codec_dai[] = {
 	{
 		.name = "ab8500-codec-dai.0",
@@ -2391,12 +2396,7 @@ static struct snd_soc_dai_driver ab8500_codec_dai[] = {
 			.rates = AB8500_SUPPORTED_RATE,
 			.formats = AB8500_SUPPORTED_FMT,
 		},
-		.ops = (struct snd_soc_dai_ops[]) {
-			{
-				.set_tdm_slot = ab8500_codec_set_dai_tdm_slot,
-				.set_fmt = ab8500_codec_set_dai_fmt,
-			}
-		},
+		.ops = &ab8500_codec_ops,
 		.symmetric_rates = 1
 	},
 	{
@@ -2409,12 +2409,7 @@ static struct snd_soc_dai_driver ab8500_codec_dai[] = {
 			.rates = AB8500_SUPPORTED_RATE,
 			.formats = AB8500_SUPPORTED_FMT,
 		},
-		.ops = (struct snd_soc_dai_ops[]) {
-			{
-				.set_tdm_slot = ab8500_codec_set_dai_tdm_slot,
-				.set_fmt = ab8500_codec_set_dai_fmt,
-			}
-		},
+		.ops = &ab8500_codec_ops,
 		.symmetric_rates = 1
 	}
 };
-- 
1.8.2

Re: [PATCH 2/6] ASoC: ab8500-codec: Move codec ops on a separate structure

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

On Fri, May 24, 2013 at 12:39:16PM +0200, Fabio Baltieri wrote:
> Define ab8500 codec operations structure on its own rather than inline
> with snd_soc_dai_drivers to clean up the code and make the style
> coherent with other codec drivers.

Applied this and the rest, thanks.

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

[PATCH 3/6] ASoC: ux500: Drop dangling struct i2s_controller

[Fabio Baltieri]

Drop struct i2s_controller from the ux500 ASoC driver as right now it is
instantiated but not used anywhere.  Also drop a mismatched
device_unregister in the process.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/ux500/ux500_msp_i2s.c | 19 -------------------
 sound/soc/ux500/ux500_msp_i2s.h | 12 ------------
 2 files changed, 31 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.c b/sound/soc/ux500/ux500_msp_i2s.c
index b029b2d..cba0e86 100644
--- a/sound/soc/ux500/ux500_msp_i2s.c
+++ b/sound/soc/ux500/ux500_msp_i2s.c
@@ -649,7 +649,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 			struct msp_i2s_platform_data *platform_data)
 {
 	struct resource *res = NULL;
-	struct i2s_controller *i2s_cont;
 	struct device_node *np = pdev->dev.of_node;
 	struct ux500_msp *msp;
 
@@ -694,22 +693,6 @@ int ux500_msp_i2s_init_msp(struct platform_device *pdev,
 	msp->msp_state = MSP_STATE_IDLE;
 	msp->loopback_enable = 0;
 
-	/* I2S-controller is allocated and added in I2S controller class. */
-	i2s_cont = devm_kzalloc(&pdev->dev, sizeof(*i2s_cont), GFP_KERNEL);
-	if (!i2s_cont) {
-		dev_err(&pdev->dev,
-			"%s: ERROR: Failed to allocate I2S-controller!\n",
-			__func__);
-		return -ENOMEM;
-	}
-	i2s_cont->dev.parent = &pdev->dev;
-	i2s_cont->data = (void *)msp;
-	i2s_cont->id = (s16)msp->id;
-	snprintf(i2s_cont->name, sizeof(i2s_cont->name), "ux500-msp-i2s.%04x",
-		msp->id);
-	dev_dbg(&pdev->dev, "I2S device-name: '%s'\n", i2s_cont->name);
-	msp->i2s_cont = i2s_cont;
-
 	return 0;
 }
 
@@ -717,8 +700,6 @@ void ux500_msp_i2s_cleanup_msp(struct platform_device *pdev,
 			struct ux500_msp *msp)
 {
 	dev_dbg(msp->dev, "%s: Enter (id = %d).\n", __func__, msp->id);
-
-	device_unregister(&msp->i2s_cont->dev);
 }
 
 MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index 8ce014e..ccfcc32 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -469,17 +469,6 @@ struct i2s_message {
 	size_t period_len;
 };
 
-struct i2s_controller {
-	struct module *owner;
-	unsigned int id;
-	unsigned int class;
-	const struct i2s_algorithm *algo; /* the algorithm to access the bus */
-	void *data;
-	struct mutex bus_lock;
-	struct device dev; /* the controller device */
-	char name[48];
-};
-
 struct ux500_msp_config {
 	unsigned int f_inputclk;
 	unsigned int rx_clk_sel;
@@ -515,7 +504,6 @@ struct ux500_msp {
 	enum enum_i2s_controller id;
 	void __iomem *registers;
 	struct device *dev;
-	struct i2s_controller *i2s_cont;
 	struct stedma40_chan_cfg *dma_cfg_rx;
 	struct stedma40_chan_cfg *dma_cfg_tx;
 	struct dma_chan *tx_pipeid;
-- 
1.8.2

[PATCH 4/6] ASoC: ux500: Drop unused code from msp headers

[Fabio Baltieri]

Drop unused fields and structures from ux500_msp_i2s header file, as
those looks like leftover from a previous implementation of the driver.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/ux500/ux500_msp_dai.h |  2 --
 sound/soc/ux500/ux500_msp_i2s.h | 31 -------------------------------
 2 files changed, 33 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_dai.h b/sound/soc/ux500/ux500_msp_dai.h
index f531043..c721282 100644
--- a/sound/soc/ux500/ux500_msp_dai.h
+++ b/sound/soc/ux500/ux500_msp_dai.h
@@ -58,8 +58,6 @@ struct ux500_msp_i2s_drvdata {
 	unsigned int rx_mask;
 	int slots;
 	int slot_width;
-	u8 configured;
-	int data_delay;
 
 	/* Clocks */
 	unsigned int master_clk;
diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index ccfcc32..d5e4176 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -341,11 +341,6 @@ enum msp_compress_mode {
 	MSP_COMPRESS_MODE_A_LAW = 3
 };
 
-enum msp_spi_burst_mode {
-	MSP_SPI_BURST_MODE_DISABLE = 0,
-	MSP_SPI_BURST_MODE_ENABLE = 1
-};
-
 enum msp_expand_mode {
 	MSP_EXPAND_MODE_LINEAR = 0,
 	MSP_EXPAND_MODE_LINEAR_SIGNED = 1,
@@ -454,21 +449,6 @@ struct msp_protdesc {
 	u32 clocks_per_frame;
 };
 
-struct i2s_message {
-	enum i2s_direction_t i2s_direction;
-	void *txdata;
-	void *rxdata;
-	size_t txbytes;
-	size_t rxbytes;
-	int dma_flag;
-	int tx_offset;
-	int rx_offset;
-	bool cyclic_dma;
-	dma_addr_t buf_addr;
-	size_t buf_len;
-	size_t period_len;
-};
-
 struct ux500_msp_config {
 	unsigned int f_inputclk;
 	unsigned int rx_clk_sel;
@@ -480,8 +460,6 @@ struct ux500_msp_config {
 	unsigned int tx_fsync_sel;
 	unsigned int rx_fifo_config;
 	unsigned int tx_fifo_config;
-	unsigned int spi_clk_mode;
-	unsigned int spi_burst_mode;
 	unsigned int loopback_enable;
 	unsigned int tx_data_enable;
 	unsigned int default_protdesc;
@@ -491,13 +469,9 @@ struct ux500_msp_config {
 	unsigned int direction;
 	unsigned int protocol;
 	unsigned int frame_freq;
-	unsigned int frame_size;
 	enum msp_data_size data_size;
 	unsigned int def_elem_len;
 	unsigned int iodelay;
-	void (*handler) (void *data);
-	void *tx_callback_data;
-	void *rx_callback_data;
 };
 
 struct ux500_msp {
@@ -506,15 +480,10 @@ struct ux500_msp {
 	struct device *dev;
 	struct stedma40_chan_cfg *dma_cfg_rx;
 	struct stedma40_chan_cfg *dma_cfg_tx;
-	struct dma_chan *tx_pipeid;
-	struct dma_chan *rx_pipeid;
 	enum msp_state msp_state;
-	int (*transfer) (struct ux500_msp *msp, struct i2s_message *message);
-	struct timer_list notify_timer;
 	int def_elem_len;
 	unsigned int dir_busy;
 	int loopback_enable;
-	u32 backup_regs[MAX_MSP_BACKUP_REGS];
 	unsigned int f_bitclk;
 };
 
-- 
1.8.2

[PATCH 5/6] ASoC: ux500: Add missing mop500_ab8500.h include

[Fabio Baltieri]

Add a missing include that was resulting in some sparse warning for
non-static structure without forward declaration.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/ux500/mop500_ab8500.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/ux500/mop500_ab8500.c b/sound/soc/ux500/mop500_ab8500.c
index 884a362..5e0f146 100644
--- a/sound/soc/ux500/mop500_ab8500.c
+++ b/sound/soc/ux500/mop500_ab8500.c
@@ -24,6 +24,7 @@
 
 #include "ux500_pcm.h"
 #include "ux500_msp_dai.h"
+#include "mop500_ab8500.h"
 #include "../codecs/ab8500-codec.h"
 
 #define TX_SLOT_MONO	0x0008
-- 
1.8.2

[PATCH 6/6] ASoC: ux500: Drop redundant msp id enumerations

[Fabio Baltieri]

Ux500 has two equivalent enum for device id, one in platform_data and
one in a local header.  Fix this by dropping the local one.

Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
---
 sound/soc/ux500/ux500_msp_i2s.h | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/sound/soc/ux500/ux500_msp_i2s.h b/sound/soc/ux500/ux500_msp_i2s.h
index d5e4176..189a375 100644
--- a/sound/soc/ux500/ux500_msp_i2s.h
+++ b/sound/soc/ux500/ux500_msp_i2s.h
@@ -16,6 +16,7 @@
 #define UX500_MSP_I2S_H
 
 #include <linux/platform_device.h>
+#include <linux/platform_data/asoc-ux500-msp.h>
 
 #define MSP_INPUT_FREQ_APB 48000000
 
@@ -365,13 +366,6 @@ enum msp_protocol {
  */
 #define MAX_MSP_BACKUP_REGS 36
 
-enum enum_i2s_controller {
-	MSP_0_I2S_CONTROLLER = 0,
-	MSP_1_I2S_CONTROLLER,
-	MSP_2_I2S_CONTROLLER,
-	MSP_3_I2S_CONTROLLER,
-};
-
 enum i2s_direction_t {
 	MSP_DIR_TX = 0x01,
 	MSP_DIR_RX = 0x02,
@@ -475,7 +469,7 @@ struct ux500_msp_config {
 };
 
 struct ux500_msp {
-	enum enum_i2s_controller id;
+	enum msp_i2s_id id;
 	void __iomem *registers;
 	struct device *dev;
 	struct stedma40_chan_cfg *dma_cfg_rx;
-- 
1.8.2