[alsa-devel] [PATCH 0/6] More code cleanup for ASoC ux500
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(-)
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 {
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
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.
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 } };
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.
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;
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; };
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
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;
participants (3)
-
Fabio Baltieri -
Linus Walleij -
Mark Brown