2013-05-24 10:39:26

by Fabio Baltieri

[permalink] [raw]
Subject: [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(-)

--
1.8.2


2013-05-24 10:39:39

by Fabio Baltieri

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

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 <[email protected]>
---
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

2013-05-24 10:39:48

by Fabio Baltieri

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

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 <[email protected]>
---
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

2013-05-24 10:39:42

by Fabio Baltieri

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

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 <[email protected]>
---
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

2013-05-24 10:40:06

by Fabio Baltieri

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

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

Signed-off-by: Fabio Baltieri <[email protected]>
---
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

2013-05-24 10:39:36

by Fabio Baltieri

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

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 <[email protected]>
---
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

2013-05-24 10:40:59

by Fabio Baltieri

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

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 <[email protected]>
---
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

2013-05-24 11:11:11

by Linus Walleij

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

On Fri, May 24, 2013 at 12:39 PM, Fabio Baltieri
<[email protected]> 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 <[email protected]>

Acked-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2013-05-26 20:37:45

by Mark Brown

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

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.


Attachments:
(No filename) (354.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-05-26 20:38:04

by Mark Brown

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

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.


Attachments:
(No filename) (277.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments