All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@deeprootsystems.com>
To: Ben Dooks <ben-linux@fluff.org>
Cc: linux-i2c@vger.kernel.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH] I2C: OMAP3: PM: (re)init for every transfer to support off-mode
Date: Wed, 30 Sep 2009 15:06:09 -0700	[thread overview]
Message-ID: <636c5030909301506p7da5f016m5148ce6cd279e704@mail.gmail.com> (raw)
In-Reply-To: <4A7760A4.3080109@deeprootsystems.com>

On Mon, Aug 3, 2009 at 3:11 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Ben Dooks wrote:
>>
>> On Tue, Jul 21, 2009 at 04:09:03PM -0700, Kevin Hilman wrote:
>>>
>>> From: Rajendra Nayak <rnayak@ti.com>
>>>
>>> Because of OMAP off-mode, powerdomain can go off when I2C is idle.
>>> Save enough state, and do a re-init for each transfer.
>>>
>>> Additional save/restore state added by Jagadeesh Bhaskar Pakaravoor
>>> (SYSC_REG) and Aaro Koskinen (wakeup sources.)
>>>
>>> Also, The OMAP3430 TRM states:
>>>
>>> "During active mode (I2Ci.I2C_CON[15] I2C_EN bit is set to 1), make no
>>> changes to the I2Ci.I2C_SCLL and I2Ci.I2C_SCLH registers.  Changes may
>>> result in unpredictable behavior."
>>>
>>> Hence, the I2C_EN bit should be clearer when modifying these
>>> registers. Please note that clearing the entire I2C_CON register to
>>> disable the I2C module is safe, because the I2C_CON register is
>>> re-configured for each transfer.
>>
>> should this be applied as a bugfix, or kept for next merge window?
>
> next merge window is fine.
>

Ben,

It doesn't look like this made it in during the 2.6.32 merge window.
Can you queue it for the -rc series?

Kevin

>>> Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
>>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>>> Cc: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
>>> Cc: Aaro Koskinen <aaro.koskinen@nokia.com>
>>> Cc: Jon Hunter <jon-hunter@ti.com>
>>> Cc: Hu Tao <taohu@motorola.com>
>>> Cc: Xiaolong Chen <A21785@motorola.com>
>>> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
>>> ---
>>> This patch has been tested extensively as part of the OMAP 'PM branch'
>>>
>>>  drivers/i2c/busses/i2c-omap.c |   64
>>> ++++++++++++++++++++++++++--------------
>>>  1 files changed, 41 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-omap.c
>>> b/drivers/i2c/busses/i2c-omap.c
>>> index ad8d201..bb8ae50 100644
>>> --- a/drivers/i2c/busses/i2c-omap.c
>>> +++ b/drivers/i2c/busses/i2c-omap.c
>>> @@ -178,6 +178,12 @@ struct omap_i2c_dev {
>>>        unsigned                b_hw:1;         /* bad h/w fixes */
>>>        unsigned                idle:1;
>>>        u16                     iestate;        /* Saved interrupt
>>> register */
>>> +       u16                     pscstate;
>>> +       u16                     scllstate;
>>> +       u16                     sclhstate;
>>> +       u16                     bufstate;
>>> +       u16                     syscstate;
>>> +       u16                     westate;
>>>  };
>>>   static inline void omap_i2c_write_reg(struct omap_i2c_dev *i2c_dev,
>>> @@ -230,9 +236,18 @@ static void omap_i2c_unidle(struct omap_i2c_dev
>>> *dev)
>>>          clk_enable(dev->iclk);
>>>        clk_enable(dev->fclk);
>>> +       if (cpu_is_omap34xx()) {
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG,
>>> dev->scllstate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG,
>>> dev->sclhstate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, dev->bufstate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
>>> dev->syscstate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate);
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
>>> OMAP_I2C_CON_EN);
>>> +       }
>>>        dev->idle = 0;
>>> -       if (dev->iestate)
>>> -               omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>>> +       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>>>  }
>>>   static void omap_i2c_idle(struct omap_i2c_dev *dev)
>>> @@ -258,7 +273,7 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
>>>   static int omap_i2c_init(struct omap_i2c_dev *dev)
>>>  {
>>> -       u16 psc = 0, scll = 0, sclh = 0;
>>> +       u16 psc = 0, scll = 0, sclh = 0, buf = 0;
>>>        u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
>>>        unsigned long fclk_rate = 12000000;
>>>        unsigned long timeout;
>>> @@ -287,24 +302,22 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>>>                                           SYSC_AUTOIDLE_MASK);
>>>                  } else if (dev->rev >= OMAP_I2C_REV_ON_3430) {
>>> -                       u32 v;
>>> -
>>> -                       v = SYSC_AUTOIDLE_MASK;
>>> -                       v |= SYSC_ENAWAKEUP_MASK;
>>> -                       v |= (SYSC_IDLEMODE_SMART <<
>>> +                       dev->syscstate = SYSC_AUTOIDLE_MASK;
>>> +                       dev->syscstate |= SYSC_ENAWAKEUP_MASK;
>>> +                       dev->syscstate |= (SYSC_IDLEMODE_SMART <<
>>>                              __ffs(SYSC_SIDLEMODE_MASK));
>>> -                       v |= (SYSC_CLOCKACTIVITY_FCLK <<
>>> +                       dev->syscstate |= (SYSC_CLOCKACTIVITY_FCLK <<
>>>                              __ffs(SYSC_CLOCKACTIVITY_MASK));
>>>  -                       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, v);
>>> +                       omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
>>> +                                                       dev->syscstate);
>>>                        /*
>>>                         * Enabling all wakup sources to stop I2C freezing
>>> on
>>>                         * WFI instruction.
>>>                         * REVISIT: Some wkup sources might not be needed.
>>>                         */
>>> -                       omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
>>> -                                                       OMAP_I2C_WE_ALL);
>>> -
>>> +                       dev->westate = OMAP_I2C_WE_ALL;
>>> +                       omap_i2c_write_reg(dev, OMAP_I2C_WE_REG,
>>> dev->westate);
>>>                }
>>>        }
>>>        omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
>>> @@ -394,23 +407,28 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>>>        omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG, scll);
>>>        omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG, sclh);
>>>  -       if (dev->fifo_size)
>>> -               /* Note: setup required fifo size - 1 */
>>> -               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
>>> -                                       (dev->fifo_size - 1) << 8 | /*
>>> RTRSH */
>>> -                                       OMAP_I2C_BUF_RXFIF_CLR |
>>> -                                       (dev->fifo_size - 1) | /* XTRSH
>>> */
>>> -                                       OMAP_I2C_BUF_TXFIF_CLR);
>>> +       if (dev->fifo_size) {
>>> +               /* Note: setup required fifo size - 1. RTRSH and XTRSH */
>>> +               buf = (dev->fifo_size - 1) << 8 | OMAP_I2C_BUF_RXFIF_CLR
>>> |
>>> +                       (dev->fifo_size - 1) | OMAP_I2C_BUF_TXFIF_CLR;
>>> +               omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
>>> +       }
>>>          /* Take the I2C module out of reset: */
>>>        omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
>>>          /* Enable interrupts */
>>> -       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG,
>>> -                       (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
>>> +       dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
>>>                        OMAP_I2C_IE_ARDY | OMAP_I2C_IE_NACK |
>>>                        OMAP_I2C_IE_AL)  | ((dev->fifo_size) ?
>>> -                               (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) :
>>> 0));
>>> +                               (OMAP_I2C_IE_RDR | OMAP_I2C_IE_XDR) : 0);
>>> +       omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
>>> +       if (cpu_is_omap34xx()) {
>>> +               dev->pscstate = psc;
>>> +               dev->scllstate = scll;
>>> +               dev->sclhstate = sclh;
>>> +               dev->bufstate = buf;
>>> +       }
>>>        return 0;
>>>  }
>>>
>>> --
>>> 1.6.3.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2009-09-30 22:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-21 23:09 [PATCH] I2C: OMAP3: PM: (re)init for every transfer to support off-mode Kevin Hilman
2009-07-29 23:43 ` Ben Dooks
     [not found]   ` <20090729234308.GA8850-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
2009-08-03 22:11     ` Kevin Hilman
2009-09-30 22:06       ` Kevin Hilman [this message]
     [not found]         ` <636c5030909301506p7da5f016m5148ce6cd279e704-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-11-12 19:29           ` Kevin Hilman
2009-11-23 16:59 Kevin Hilman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=636c5030909301506p7da5f016m5148ce6cd279e704@mail.gmail.com \
    --to=khilman@deeprootsystems.com \
    --cc=ben-linux@fluff.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.