All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Wolfram Sang <wsa@the-dreams.de>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sekhar Nori <nsekhar@ti.com>,
	Kevin Hilman <khilman@deeprootsystems.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Murali Karicheri <m-karicheri2@ti.com>
Subject: Re: [2/5] i2c: davinci: query STP always when NACK is received
Date: Fri, 21 Nov 2014 17:33:37 +0200	[thread overview]
Message-ID: <546F5B51.1030006@ti.com> (raw)
In-Reply-To: <20141121131008.GQ27002@pengutronix.de>

Hi Uwe,

On 11/21/2014 03:10 PM, Uwe Kleine-König wrote:
> On Fri, Nov 21, 2014 at 02:48:57PM +0200, Grygorii Strashko wrote:
>> On 11/21/2014 12:19 AM, Uwe Kleine-König wrote:
>>>> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
>>>> index 9bbfb8f..2cef115 100644
>>>> --- a/drivers/i2c/busses/i2c-davinci.c
>>>> +++ b/drivers/i2c/busses/i2c-davinci.c
>>>> @@ -411,11 +411,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
>>>>    	if (dev->cmd_err & DAVINCI_I2C_STR_NACK) {
>>>>    		if (msg->flags & I2C_M_IGNORE_NAK)
>>>>    			return msg->len;
>>>> -		if (stop) {
>>>> -			w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>>>> -			w |= DAVINCI_I2C_MDR_STP;
>>>> -			davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
>>>> -		}
>>>> +		w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>>>> +		w |= DAVINCI_I2C_MDR_STP;
>>>> +		davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
>>> I think this is a good change, but I wonder if the handling of
>>> I2C_M_IGNORE_NAK is correct here. If the controller reports a NACK say
>>> for the 2nd byte of a 5-byte-message, the transfer supposed to
>>> continue, right? (Hmm, maybe the framework handle this and restarts the
>>> transfer with I2C_M_NOSTART but the davinci driver doesn't seem to
>>> handle this flag?)
>>
>> Have nothing to say about handling of I2C_M_IGNORE_NAK. I'm not going to
>> change current behavior - davinci driver will interrupt transfer of i2c_msg always
>>   in case of NACK and start transfer of the next i2c_msg (if exist).
>> In my opinion, Above question is out of scope of this patch.
> Yeah right, that's exactly what I thought.
> 
> Thinking again I wonder if with your change handling is correct when the
> sender wants to do a repeated start. That would need a more detailed
> look into the driver.

Davinci driver will always abort transfer with error -EREMOTEIO in case if
NACK received from I2C slave device. And the next omap_i2c_xfer() call may
be *not* targeted to the same I2C slave device.
^ if !I2C_M_IGNORE_NAK

This discussion is absolutely similar to https://lkml.org/lkml/2013/7/16/235
So, I'm just copy-pasting my answers from there ;)

regards,
-grygorii

WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Wolfram Sang <wsa@the-dreams.de>, <linux-i2c@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Sekhar Nori <nsekhar@ti.com>,
	Kevin Hilman <khilman@deeprootsystems.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Murali Karicheri <m-karicheri2@ti.com>
Subject: Re: [2/5] i2c: davinci: query STP always when NACK is received
Date: Fri, 21 Nov 2014 17:33:37 +0200	[thread overview]
Message-ID: <546F5B51.1030006@ti.com> (raw)
In-Reply-To: <20141121131008.GQ27002@pengutronix.de>

Hi Uwe,

On 11/21/2014 03:10 PM, Uwe Kleine-König wrote:
> On Fri, Nov 21, 2014 at 02:48:57PM +0200, Grygorii Strashko wrote:
>> On 11/21/2014 12:19 AM, Uwe Kleine-König wrote:
>>>> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
>>>> index 9bbfb8f..2cef115 100644
>>>> --- a/drivers/i2c/busses/i2c-davinci.c
>>>> +++ b/drivers/i2c/busses/i2c-davinci.c
>>>> @@ -411,11 +411,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
>>>>    	if (dev->cmd_err & DAVINCI_I2C_STR_NACK) {
>>>>    		if (msg->flags & I2C_M_IGNORE_NAK)
>>>>    			return msg->len;
>>>> -		if (stop) {
>>>> -			w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>>>> -			w |= DAVINCI_I2C_MDR_STP;
>>>> -			davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
>>>> -		}
>>>> +		w = davinci_i2c_read_reg(dev, DAVINCI_I2C_MDR_REG);
>>>> +		w |= DAVINCI_I2C_MDR_STP;
>>>> +		davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, w);
>>> I think this is a good change, but I wonder if the handling of
>>> I2C_M_IGNORE_NAK is correct here. If the controller reports a NACK say
>>> for the 2nd byte of a 5-byte-message, the transfer supposed to
>>> continue, right? (Hmm, maybe the framework handle this and restarts the
>>> transfer with I2C_M_NOSTART but the davinci driver doesn't seem to
>>> handle this flag?)
>>
>> Have nothing to say about handling of I2C_M_IGNORE_NAK. I'm not going to
>> change current behavior - davinci driver will interrupt transfer of i2c_msg always
>>   in case of NACK and start transfer of the next i2c_msg (if exist).
>> In my opinion, Above question is out of scope of this patch.
> Yeah right, that's exactly what I thought.
> 
> Thinking again I wonder if with your change handling is correct when the
> sender wants to do a repeated start. That would need a more detailed
> look into the driver.

Davinci driver will always abort transfer with error -EREMOTEIO in case if
NACK received from I2C slave device. And the next omap_i2c_xfer() call may
be *not* targeted to the same I2C slave device.
^ if !I2C_M_IGNORE_NAK

This discussion is absolutely similar to https://lkml.org/lkml/2013/7/16/235
So, I'm just copy-pasting my answers from there ;)

regards,
-grygorii

  reply	other threads:[~2014-11-21 15:33 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-20 10:03 [PATCH 0/5] i2c: davinci improvements and fixes Grygorii Strashko
2014-11-20 10:03 ` Grygorii Strashko
2014-11-20 10:03 ` [PATCH 1/5] i2c: i2c-davinci: switch to use platform_get_irq Grygorii Strashko
2014-11-20 10:03   ` Grygorii Strashko
2014-11-20 21:48   ` [1/5] " Uwe Kleine-König
     [not found]     ` <20141120214838.GA346-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-21 11:01       ` Grygorii Strashko
2014-11-21 11:01         ` Grygorii Strashko
     [not found]         ` <546F1B7D.1020209-l0cyMroinI0@public.gmane.org>
2014-11-21 14:03           ` Rob Herring
2014-11-21 14:03             ` Rob Herring
2014-11-21 14:59             ` Grygorii Strashko
     [not found] ` <1416477788-5544-1-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-11-20 10:03   ` [PATCH 2/5] i2c: davinci: query STP always when NACK is received Grygorii Strashko
2014-11-20 10:03     ` Grygorii Strashko
     [not found]     ` <1416477788-5544-3-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-11-20 22:19       ` [2/5] " Uwe Kleine-König
2014-11-20 22:19         ` Uwe Kleine-König
     [not found]         ` <20141120221953.GI27002-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-21 12:48           ` Grygorii Strashko
2014-11-21 12:48             ` Grygorii Strashko
     [not found]             ` <546F34B9.1000206-l0cyMroinI0@public.gmane.org>
2014-11-21 13:10               ` Uwe Kleine-König
2014-11-21 13:10                 ` Uwe Kleine-König
2014-11-21 15:33                 ` Grygorii Strashko [this message]
2014-11-21 15:33                   ` Grygorii Strashko
     [not found]                   ` <546F5B51.1030006-l0cyMroinI0@public.gmane.org>
2014-11-23 20:33                     ` Uwe Kleine-König
2014-11-23 20:33                       ` Uwe Kleine-König
     [not found]                       ` <20141123203314.GF4431-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-24 13:34                         ` Grygorii Strashko
2014-11-24 13:34                           ` Grygorii Strashko
     [not found]                           ` <547333EB.5050403-l0cyMroinI0@public.gmane.org>
2014-11-24 20:02                             ` Uwe Kleine-König
2014-11-24 20:02                               ` Uwe Kleine-König
2014-11-20 10:03 ` [PATCH 3/5] i2c: recovery: change input parameter to i2c_adapter for prepare/unprepare_recovery Grygorii Strashko
2014-11-20 10:03   ` Grygorii Strashko
     [not found]   ` <1416477788-5544-4-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-11-21 18:49     ` [3/5] " Uwe Kleine-König
2014-11-21 18:49       ` Uwe Kleine-König
2014-11-20 10:03 ` [PATCH 4/5] i2c: davinci: use bus recovery infrastructure Grygorii Strashko
2014-11-20 10:03   ` Grygorii Strashko
     [not found]   ` <1416477788-5544-5-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-11-21 19:07     ` [4/5] " Uwe Kleine-König
2014-11-21 19:07       ` Uwe Kleine-König
     [not found]       ` <20141121190744.GB4431-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-21 19:33         ` Grygorii Strashko
2014-11-21 19:33           ` Grygorii Strashko
     [not found]           ` <546F9382.40102-l0cyMroinI0@public.gmane.org>
2014-11-23 20:36             ` Uwe Kleine-König
2014-11-23 20:36               ` Uwe Kleine-König
2014-11-24 13:26               ` Grygorii Strashko
2014-11-24 13:26                 ` Grygorii Strashko
     [not found]                 ` <547331F2.3000006-l0cyMroinI0@public.gmane.org>
2014-11-24 20:07                   ` Uwe Kleine-König
2014-11-24 20:07                     ` Uwe Kleine-König
2014-11-20 10:03 ` [PATCH 5/5] i2c: davinci: use ICPFUNC to toggle I2C as gpio for bus recovery Grygorii Strashko
2014-11-20 10:03   ` Grygorii Strashko
     [not found]   ` <1416477788-5544-6-git-send-email-grygorii.strashko-l0cyMroinI0@public.gmane.org>
2014-11-23 17:04     ` [5/5] " Uwe Kleine-König
2014-11-23 17:04       ` Uwe Kleine-König
     [not found]       ` <20141123170400.GC4431-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-24 13:15         ` Grygorii Strashko
2014-11-24 13:15           ` Grygorii Strashko
     [not found]           ` <54732F8E.4080105-l0cyMroinI0@public.gmane.org>
2014-11-24 18:13             ` Mike Looijmans
2014-11-24 18:13               ` Mike Looijmans
     [not found]               ` <54737532.1050808-KQxf8TFvCoAi8rCdYzckzA@public.gmane.org>
2014-11-24 19:22                 ` Grygorii Strashko
2014-11-24 19:22                   ` Grygorii Strashko
2014-11-24 19:45             ` Uwe Kleine-König
2014-11-24 19:45               ` Uwe Kleine-König
     [not found]               ` <20141124194533.GU4431-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2014-11-25 13:04                 ` Grygorii Strashko
2014-11-25 13:04                   ` Grygorii Strashko

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=546F5B51.1030006@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=khilman@deeprootsystems.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m-karicheri2@ti.com \
    --cc=nsekhar@ti.com \
    --cc=ssantosh@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=wsa@the-dreams.de \
    /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.