Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753413Ab3EHIUb (ORCPT ); Wed, 8 May 2013 04:20:31 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:64799 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752537Ab3EHIU2 (ORCPT ); Wed, 8 May 2013 04:20:28 -0400 Date: Wed, 8 May 2013 10:20:17 +0200 From: Fabio Baltieri To: Lee Jones Cc: Mark Brown , Liam Girdwood , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Linus Walleij , Ola Lilja Subject: Re: [PATCH 3/6] ASoC: ux500: Drop pinctrl sleep support Message-ID: <20130508082017.GA1526@balto.lan> References: <1367997261-32048-1-git-send-email-fabio.baltieri@linaro.org> <1367997261-32048-4-git-send-email-fabio.baltieri@linaro.org> <20130508080708.GH3102@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130508080708.GH3102@gmail.com> X-Operating-System: Linux balto 3.9.0-rc8-00030-g4cbbd1d x86_64 GNU/Linux User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5743 Lines: 160 On Wed, May 08, 2013 at 09:07:08AM +0100, Lee Jones wrote: > 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? Yes, but requesting the default pinctrl configuration should be enough, and as those pins are shared with multiple device ids, a "hog" configuration should be the cleanest. Actually I asked Linus an opinion before doing this, so maybe he can ack this patch or suggest a better way of doing this, such as declaring the same pins for multiple device ids, but I'm not sure that would work as expected. Thanks, Fabio > > Signed-off-by: Fabio Baltieri > > --- > > 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 > > #include > > -#include > > #include > > #include > > #include > > @@ -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 { -- Fabio Baltieri -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/