[PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Laxman Dewangan]

If timeout error occurs in the i2c transfer then it was dumping warning
of call stack.

Remove the warning dump as there is may be possibility that some slave
devices are busy and not responding the i2c communication.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
The patch is generated based on discussion happen between Stephena and
Wolfram on the patch:
 i2c: add bcm2835 driver

 drivers/i2c/busses/i2c-tegra.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index ae2e027..36704e3 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -587,7 +587,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
 	tegra_i2c_mask_irq(i2c_dev, int_mask);
 
-	if (WARN_ON(ret == 0)) {
+	if (ret == 0) {
 		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
 
 		tegra_i2c_init(i2c_dev);
-- 
1.7.1.1

[PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Laxman Dewangan]

If timeout error occurs in the i2c transfer then it was dumping warning
of call stack.

Remove the warning dump as there is may be possibility that some slave
devices are busy and not responding the i2c communication.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
The patch is generated based on discussion happen between Stephena and
Wolfram on the patch:
 i2c: add bcm2835 driver

resending patch as Wolfram's email id has been changed.

 drivers/i2c/busses/i2c-tegra.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index ae2e027..36704e3 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -587,7 +587,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
 	tegra_i2c_mask_irq(i2c_dev, int_mask);
 
-	if (WARN_ON(ret == 0)) {
+	if (ret == 0) {
 		dev_err(i2c_dev->dev, "i2c transfer timed out\n");
 
 		tegra_i2c_init(i2c_dev);
-- 
1.7.1.1

Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Shubhrajyoti Datta]

On Thu, Feb 14, 2013 at 6:13 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> If timeout error occurs in the i2c transfer then it was dumping warning
> of call stack.
>
> Remove the warning dump as there is may be possibility that some slave
> devices are busy and not responding the i2c communication.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> The patch is generated based on discussion happen between Stephena and
> Wolfram on the patch:
>  i2c: add bcm2835 driver
>
> resending patch as Wolfram's email id has been changed.
>
>  drivers/i2c/busses/i2c-tegra.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index ae2e027..36704e3 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -587,7 +587,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
>         ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
>         tegra_i2c_mask_irq(i2c_dev, int_mask);
>
> -       if (WARN_ON(ret == 0)) {
> +       if (ret == 0) {

I think WARN_ON has a unlikely.

If you could do a profiling and have the unlikely.

BTW thats not an objection to the patch though.

>                 dev_err(i2c_dev->dev, "i2c transfer timed out\n");
>
>                 tegra_i2c_init(i2c_dev);
> --
> 1.7.1.1
>
> --
> 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

Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Wolfram Sang]

On Fri, Feb 15, 2013 at 06:31:07PM +0530, Shubhrajyoti Datta wrote:
> On Thu, Feb 14, 2013 at 6:13 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > If timeout error occurs in the i2c transfer then it was dumping warning
> > of call stack.
> >
> > Remove the warning dump as there is may be possibility that some slave
> > devices are busy and not responding the i2c communication.
> >
> > Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > The patch is generated based on discussion happen between Stephena and
> > Wolfram on the patch:
> >  i2c: add bcm2835 driver
> >
> > resending patch as Wolfram's email id has been changed.
> >
> >  drivers/i2c/busses/i2c-tegra.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> > index ae2e027..36704e3 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -587,7 +587,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
> >         ret = wait_for_completion_timeout(&i2c_dev->msg_complete, TEGRA_I2C_TIMEOUT);
> >         tegra_i2c_mask_irq(i2c_dev, int_mask);
> >
> > -       if (WARN_ON(ret == 0)) {
> > +       if (ret == 0) {
> 
> I think WARN_ON has a unlikely.
> 
> If you could do a profiling and have the unlikely.
> 
> BTW thats not an objection to the patch though.

The thing is: Timeouts can be expected on an I2C bus. Devices can be
busy, that's fine. So, no need for a WARN. I'm even thinking of asking
to remove the dev_err here, or at least change it into dev_warn, but
haven't made up my mind yet.

Regards,

   Wolfram

Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Wolfram Sang]

On Thu, Feb 14, 2013 at 06:13:33PM +0530, Laxman Dewangan wrote:
> If timeout error occurs in the i2c transfer then it was dumping warning
> of call stack.
> 
> Remove the warning dump as there is may be possibility that some slave
> devices are busy and not responding the i2c communication.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

I'd like to have the patch extended. Currently, i2c-tegra is the only
I2C user of BUG and BUG_ON. Could you maybe extend the patch to handle
those situations more gracefully while we are at it? From a glimpse,
these situations don't need a complete halt of the kernel?

Thanks,

   Wolfram

Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Stephen Warren]

On 02/15/2013 12:18 PM, Wolfram Sang wrote:
> On Thu, Feb 14, 2013 at 06:13:33PM +0530, Laxman Dewangan wrote:
>> If timeout error occurs in the i2c transfer then it was dumping warning
>> of call stack.
>>
>> Remove the warning dump as there is may be possibility that some slave
>> devices are busy and not responding the i2c communication.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> I'd like to have the patch extended. Currently, i2c-tegra is the only
> I2C user of BUG and BUG_ON. Could you maybe extend the patch to handle
> those situations more gracefully while we are at it? From a glimpse,
> these situations don't need a complete halt of the kernel?

While that's probably useful, surely that's a separate patch?

Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in transfer

[Wolfram Sang]

On Fri, Feb 15, 2013 at 12:40:47PM -0700, Stephen Warren wrote:
> On 02/15/2013 12:18 PM, Wolfram Sang wrote:
> > On Thu, Feb 14, 2013 at 06:13:33PM +0530, Laxman Dewangan wrote:
> >> If timeout error occurs in the i2c transfer then it was dumping warning
> >> of call stack.
> >>
> >> Remove the warning dump as there is may be possibility that some slave
> >> devices are busy and not responding the i2c communication.
> >>
> >> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > I'd like to have the patch extended. Currently, i2c-tegra is the only
> > I2C user of BUG and BUG_ON. Could you maybe extend the patch to handle
> > those situations more gracefully while we are at it? From a glimpse,
> > these situations don't need a complete halt of the kernel?
> 
> While that's probably useful, surely that's a separate patch?

Yes, can be argued. Patch applied to for-next, thanks!