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

From: Lee Jones
Date: Wed May 08 2013 - 04:07:30 EST


On Wed, 08 May 2013, 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.
>
> Pinctrl default mode configuration is already managed automatically by a
> specific pinctrl hog.

I'm sure we should support pinctrl though shouldn't we?

Is there no way of fixing the implementation instead of ripping it out?

> Signed-off-by: Fabio Baltieri <fabio.baltieri@xxxxxxxxxx>
> ---
> 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 964cfd6..8512c78 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>
> @@ -28,9 +27,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 */
> @@ -358,24 +354,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);
> @@ -632,8 +612,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);
>
> @@ -645,18 +624,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);
> @@ -745,25 +712,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 1311c0d..1ce336f 100644
> --- a/sound/soc/ux500/ux500_msp_i2s.h
> +++ b/sound/soc/ux500/ux500_msp_i2s.h
> @@ -530,12 +530,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 {

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/