On Wed, 08 May 2013, Fabio Baltieri wrote:
> 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.
Linus is on vacation at the moment, but I agree he should have the
final say on this. Better wait until he returns.
> > > Signed-off-by: Fabio Baltieri <fabio.balti...@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 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
