All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Cc: Yao Jin <yao.jin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Aubrey Li <aubrey.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 1/4] i2c: designware: No need to disable already disabled controller
Date: Mon, 2 Jun 2014 20:34:08 +0300	[thread overview]
Message-ID: <20140602173408.GB1730@lahna.fi.intel.com> (raw)
In-Reply-To: <20140602161234.GG2654@katana>

On Mon, Jun 02, 2014 at 06:12:34PM +0200, Wolfram Sang wrote:
> On Thu, May 15, 2014 at 05:37:21PM +0300, Mika Westerberg wrote:
> > If the controller is already in desired state (enabled/disabled) there is
> > no point in setting its state again.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> 
> Does it have a side-effect when setting then enable bit again? Otherwise
> it will exit the loop immediately on the first try. Not too bad IMO
> given the additional code saved.

AFAICT there shouldn't be any side effect. So the $subject patch just
saves one register write in the best case. You are right, maybe it's not
worth adding 3 extra lines of code just for that :)

> 
> >  drivers/i2c/busses/i2c-designware-core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> > index b58ecf19e767..b0792675b970 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.c
> > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > @@ -258,6 +258,10 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
> >  {
> >  	int timeout = 100;
> >  
> > +	/* In case the controller is already in desired state */
> > +	if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
> > +		return;
> > +
> >  	do {
> >  		dw_writel(dev, enable, DW_IC_ENABLE);
> >  		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
> > -- 
> > 2.0.0.rc2
> > 

WARNING: multiple messages have this Message-ID (diff)
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Wolfram Sang <wsa@the-dreams.de>
Cc: Yao Jin <yao.jin@intel.com>, Aubrey Li <aubrey.li@intel.com>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] i2c: designware: No need to disable already disabled controller
Date: Mon, 2 Jun 2014 20:34:08 +0300	[thread overview]
Message-ID: <20140602173408.GB1730@lahna.fi.intel.com> (raw)
In-Reply-To: <20140602161234.GG2654@katana>

On Mon, Jun 02, 2014 at 06:12:34PM +0200, Wolfram Sang wrote:
> On Thu, May 15, 2014 at 05:37:21PM +0300, Mika Westerberg wrote:
> > If the controller is already in desired state (enabled/disabled) there is
> > no point in setting its state again.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> 
> Does it have a side-effect when setting then enable bit again? Otherwise
> it will exit the loop immediately on the first try. Not too bad IMO
> given the additional code saved.

AFAICT there shouldn't be any side effect. So the $subject patch just
saves one register write in the best case. You are right, maybe it's not
worth adding 3 extra lines of code just for that :)

> 
> >  drivers/i2c/busses/i2c-designware-core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> > index b58ecf19e767..b0792675b970 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.c
> > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > @@ -258,6 +258,10 @@ static void __i2c_dw_enable(struct dw_i2c_dev *dev, bool enable)
> >  {
> >  	int timeout = 100;
> >  
> > +	/* In case the controller is already in desired state */
> > +	if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
> > +		return;
> > +
> >  	do {
> >  		dw_writel(dev, enable, DW_IC_ENABLE);
> >  		if ((dw_readl(dev, DW_IC_ENABLE_STATUS) & 1) == enable)
> > -- 
> > 2.0.0.rc2
> > 



  reply	other threads:[~2014-06-02 17:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-15 14:37 [PATCH 0/4] i2c: designware: Fixes for Asus T100 and Haswell PCI IDs Mika Westerberg
     [not found] ` <1400164644-3222-1-git-send-email-mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2014-05-15 14:37   ` [PATCH 1/4] i2c: designware: No need to disable already disabled controller Mika Westerberg
2014-05-15 14:37     ` Mika Westerberg
2014-06-02 16:12     ` Wolfram Sang
2014-06-02 17:34       ` Mika Westerberg [this message]
2014-06-02 17:34         ` Mika Westerberg
2014-06-02 17:36         ` Wolfram Sang
2014-05-15 14:37   ` [PATCH 3/4] i2c: designware: Add runtime PM hooks Mika Westerberg
2014-05-15 14:37     ` Mika Westerberg
2014-06-02 16:16   ` [PATCH 0/4] i2c: designware: Fixes for Asus T100 and Haswell PCI IDs Wolfram Sang
2014-06-02 16:16     ` Wolfram Sang
2014-05-15 14:37 ` [PATCH 2/4] i2c: designware: Disable device on system suspend Mika Westerberg
2014-05-15 14:37 ` [PATCH 4/4] i2c: designware-pci: Add Haswell PCI IDs Mika Westerberg
2014-06-06 22:44   ` [4/4] " Scot Doyle

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=20140602173408.GB1730@lahna.fi.intel.com \
    --to=mika.westerberg-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=aubrey.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    --cc=yao.jin-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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.