Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1577936 > unrolled thread

Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update is disabled

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update  is disabled Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-02-09 21:30 +0100
    Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update  is disabled "De Marchi, Lucas" <lucas.demarchi@intel.com> - 2017-02-09 22:40 +0100
      Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update  is disabled Jarkko Nikula <jarkko.nikula@linux.intel.com> - 2017-02-10 11:50 +0100
        RE: [PATCH] i2c: designware: Fix regression when dynamic TAR update  is disabled "Shah, Nehal-bakulchandra" <Nehal-bakulchandra.Shah@amd.com> - 2017-02-13 05:20 +0100

#1577936 — Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update is disabled

On Fri, 2017-02-10 at 01:20 +0530, Shah Nehal-Bakulchandra wrote:
> The following commit causes a regression when dynamic TAR update is
> disabled:
> 
>      commit 63d0f0a6952a1a02bc4f116b7da7c7887e46efa3 ("i2c:
> designware:
>      detect when dynamic tar update is possible")

Please, leave just 12 characters, it still enough.

> In such case, the DW_IC_CON_10BITADDR_MASTER is R/W, and is changed
> by the logic that's trying to detect  dynamic TAR update.The original
> value of DW_IC_CON_10BITADDR_MASTER bit should be restored.
> 

+ Fixes tag?

> Signed-off-by: Shah Nehal-Bakulchandra <Nehal-
> bakulchandra.Shah@amd.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  drivers/i2c/busses/i2c-designware-core.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.c
> b/drivers/i2c/busses/i2c-designware-core.c
> index 6d81c56..0c57166 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -987,6 +987,11 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>  	    (reg & DW_IC_CON_10BITADDR_MASTER)) {
>  		dev->dynamic_tar_update_enabled = true;
>  		dev_dbg(dev->dev, "Dynamic TAR update enabled");
> +	} else {
> +		/* If test is failed then restore the original value
> */

> +		dev->dynamic_tar_update_enabled = false;

It's default.

> +		dev_dbg(dev->dev, "Dynamic TAR update disable restore
> the value");

I think this is useless. Either you have enabled message, or have
nothing.

> +		dw_writel(dev, reg, DW_IC_CON);
>  	}
>  
>  	i2c_dw_release_lock(dev);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [next] | [standalone]

#1577972

On Thu, 2017-02-09 at 22:07 +0200, Andy Shevchenko wrote:
> On Fri, 2017-02-10 at 01:20 +0530, Shah Nehal-Bakulchandra wrote:
> > The following commit causes a regression when dynamic TAR update is
> > disabled:
> > 
> >      commit 63d0f0a6952a1a02bc4f116b7da7c7887e46efa3 ("i2c:
> > designware:
> >      detect when dynamic tar update is possible")
> 
> Please, leave just 12 characters, it still enough.
> 
> > In such case, the DW_IC_CON_10BITADDR_MASTER is R/W, and is changed
> > by the logic that's trying to detect  dynamic TAR update.The original
> > value of DW_IC_CON_10BITADDR_MASTER bit should be restored.


You are right, thanks for the fix.  This may also explains why
0317e6c (i2c: designware: do not disable adapter after transfer) caused problems
and ended up being reverted.  Could you try that on your hardware?

The dynamic tar update detection was only done as preparation work to allow not
disabling the adapter, which is reverted.  We may also just revert this commit
instead of fixing the logic.


thanks
Lucas De Marchi

[toc] | [prev] | [next] | [standalone]

#1578375

On 10.02.2017 08:38, Suravee Suthikulpanit wrote:
> At this points, my understanding is there are probably two options here:
>
> 1) Keep the commit 63d0f0a6952a (i2c: designware: detect when dynamic
> tar update
>    is possible) and apply V2 of this patch in 4.10. We might need to
> back-port the change
>    to v4.9 stable as well.
>
> 2) Revert the 63d0f0a6952a (i2c: designware: detect when dynamic tar
> update is possible) in 4.10,
>    and also in v4.9 stable as well.
>
I'm fine with both options. Maybe revert as commit 0317e6c0f1dc ("i2c: 
designware: do not disable adapter after transfer") was also reverted 
and this revert doesn't seem to cause conflicts.

-- 
Jarkko

[toc] | [prev] | [next] | [standalone]

#1579399

It is better to revert the 63d0f0a6952a.  

-----Original Message-----
From: Jarkko Nikula [mailto:jarkko.nikula@linux.intel.com] 
Sent: Friday, February 10, 2017 4:19 PM
To: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; mika.westerberg@linux.intel.com; andriy.shevchenko@linux.intel.com; Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
Cc: wsa@the-dreams.de; linux-kernel@vger.kernel.org; linux-i2c@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
Subject: Re: [PATCH] i2c: designware: Fix regression when dynamic TAR update is disabled

On 10.02.2017 08:38, Suravee Suthikulpanit wrote:
> At this points, my understanding is there are probably two options here:
>
> 1) Keep the commit 63d0f0a6952a (i2c: designware: detect when dynamic 
> tar update
>    is possible) and apply V2 of this patch in 4.10. We might need to 
> back-port the change
>    to v4.9 stable as well.
>
> 2) Revert the 63d0f0a6952a (i2c: designware: detect when dynamic tar 
> update is possible) in 4.10,
>    and also in v4.9 stable as well.
>
I'm fine with both options. Maybe revert as commit 0317e6c0f1dc ("i2c: 
designware: do not disable adapter after transfer") was also reverted and this revert doesn't seem to cause conflicts.

--
Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web