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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130215130417.GC21335@the-dreams.de>
Date:	Fri, 15 Feb 2013 14:04:17 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Shubhrajyoti Datta <omaplinuxkernel@...il.com>
Cc:	Laxman Dewangan <ldewangan@...dia.com>, swarren@...dia.com,
	khali@...ux-fr.org, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH] i2c: tegra: remove warning dump if timeout happen in
 transfer

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@...dia.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@...dia.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/