| Message ID | 1360845813-11334-1-git-send-email-ldewangan@nvidia.com |
|---|---|
| State | Accepted |
| Headers | show |
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 -- 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
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@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. 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 -- 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
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@nvidia.com> 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 -- 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
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@nvidia.com> > > 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? -- 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
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@nvidia.com> > > > > 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! -- 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
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);
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(-)