From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752006Ab1KHGTz (ORCPT ); Tue, 8 Nov 2011 01:19:55 -0500 Received: from cantor2.suse.de ([195.135.220.15]:60669 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751234Ab1KHGTx (ORCPT ); Tue, 8 Nov 2011 01:19:53 -0500 Date: Tue, 08 Nov 2011 07:19:51 +0100 Message-ID: From: Takashi Iwai To: =?UTF-8?B?w4lyaWM=?= Piel Cc: Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [PATCH] lis3lv02d: Avoid zero-division In-Reply-To: <4EB8A8B6.1010205@tremplin-utc.net> References: <4EB8A8B6.1010205@tremplin-utc.net> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Tue, 08 Nov 2011 04:57:42 +0100, Éric Piel wrote: > > Op 03-11-11 13:09, Takashi Iwai schreef: > > In some weird situation, HP DriveGuard chip can't read ODR value > > correctly, and it results in a zero-division Oops in lis3lv02d driver. > > This patch fixes the Oops by checking the value appopriately, and skips > > if any weird value is read. > Hi Takashi, > Actually, a similar patch already just landed in linus' tree: > 1510dd5954 (lis3lv02d: avoid divide by zero due to unchecked) > > However, in the patch applied, the device is disabled (until next > reboot) while in yours, the sleep is just skipped. Does it work again > after the read of odr fails? If so, maybe I could improve the current > version by, after the odr read fails, sleeping a long and safe time and > then trying to read the odr again. Then if it fails again, we give up, > otherwise the device can be used again. I guess it's possible to use the device afterward. The possible reason is either the chip is set to an invalid mode or ACPI isn't set up properly. But this path usually means that ACPI does work more or less since you could read WHOAMI. > Do you have such a device yourself? Could you let me know if after a > failing read of the odr, the device keeps working? I have a machine but I'm not quite sure how to reproduce this error. It happened casually during the installation of a new system, so it's not so trivial to switch the module during it... thanks, Takashi > > Cheers, > Éric > > > > > Cc: > > Signed-off-by: Takashi Iwai > > --- > > drivers/misc/lis3lv02d/lis3lv02d.c | 14 +++++++++++--- > > 1 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/misc/lis3lv02d/lis3lv02d.c b/drivers/misc/lis3lv02d/lis3lv02d.c > > index 8b51cd6..c4eb2f3 100644 > > --- a/drivers/misc/lis3lv02d/lis3lv02d.c > > +++ b/drivers/misc/lis3lv02d/lis3lv02d.c > > @@ -228,6 +228,14 @@ static int lis3lv02d_set_odr(int rate) > > return -EINVAL; > > } > > > > +static void lis3lv02d_power_delay(struct lis3lv02d *lis3) > > +{ > > + int odr = lis3lv02d_get_odr(); > > + if (odr<= 0) > > + return; > > + msleep(lis3->pwron_delay / odr); > > +} > > + > > static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3]) > > { > > u8 ctlreg, reg; > > @@ -266,7 +274,7 @@ static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3]) > > > > lis3->read(lis3, ctlreg,®); > > lis3->write(lis3, ctlreg, (reg | selftest)); > > - msleep(lis3->pwron_delay / lis3lv02d_get_odr()); > > + lis3lv02d_power_delay(lis3); > > > > /* Read directly to avoid axis remap */ > > x = lis3->read_data(lis3, OUTX); > > @@ -275,7 +283,7 @@ static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3]) > > > > /* back to normal settings */ > > lis3->write(lis3, ctlreg, reg); > > - msleep(lis3->pwron_delay / lis3lv02d_get_odr()); > > + lis3lv02d_power_delay(lis3); > > > > results[0] = x - lis3->read_data(lis3, OUTX); > > results[1] = y - lis3->read_data(lis3, OUTY); > > @@ -385,7 +393,7 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3) > > } > > > > /* LIS3 power on delay is quite long */ > > - msleep(lis3->pwron_delay / lis3lv02d_get_odr()); > > + lis3lv02d_power_delay(lis3); > > > > if (lis3->reg_ctrl) > > lis3_context_restore(lis3); >