Re: [PATCH 1/2] platform: x86: dell-smo8800: Dell Latitude freefall driver (ACPI SMO8800/SMO8810)

[Pali RohÃr]

On Tuesday 20 May 2014 09:00:05 Valdis.Kletnieks@xxxxxx wrote:
> On Sat, 03 May 2014 12:47:56 +0200, Pali RohÃr said:
> > This acpi driver provide supports for freefall sensors
> > SMO8800/SMO8810 which can be found on Dell Latitude
> > laptops. Driver register /dev/freefall misc device which
> > has same interface as driver hp_accel freefall driver. So
> > any existing applications for HP freefall sensor
> > /dev/freefall will work for with this new driver for Dell
> > Latitude laptops too.
> 
> So just for grins, I tossed this patchset on top of
> next-20140519, and it actually works on my Dell Latitude
> E6530. Tested with a controlled drop of 3-4 inches, and I get
> the distinctive sound of a head park, and the following in
> /var/log/messages:
> 
> May 20 02:46:22 turing-police kernel: [18839.158926] smo8800
> SMO8810:00: detected free fall May 20 02:46:22 turing-police
> FREE FALL[1337]: Parked /dev/sda disk head May 20 02:46:24
> turing-police FREE FALL[1337]: Unparked /dev/sda disk head
> 
> I did a quick read-through of the code, looks OK.  checkpatch
> complains about missing blank lines after declarations in 3-4
> places. Sparse and gcc have no complaints. We've certainly
> merged much uglier code.  And it certainly makes the world a
> better place for us Dell users.
> 

Hm? Which errors? Here is output from checkpacth:

total: 0 errors, 0 warnings, 276 lines checked

0001-platform-x86-dell-smo8800-Dell-Latitude-freefall-dri.patch 
has no obvious style problems and is ready for submission.

total: 0 errors, 0 warnings, 181 lines checked

0002-Documentation-Rename-hpfall.c-freefall.c-fixes-for-D.patch 
has no obvious style problems and is ready for submission.

> So it's OK by me if Pali adds this to the signoffs after
> fixing the checkpatch and the one serious nit below....
> 
> Tested-By: Valdis Kletnieks <valdis.kletnieks@xxxxxx>
> Acked-By: Valdis Kletnieks <valdis.kletnieks@xxxxxx>
> 
> One nit:
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -127,6 +127,16 @@ config DELL_WMI_AIO
> > 
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called dell-wmi-aio.
> > 
> > +config DELL_SMO8800
> > +	tristate "Dell Latitude freefall driver (ACPI
> > SMO8800/SMO8810)" +	depends ON ACPI
> 
> Lowercase ON.  Not sure how this escaped, Kbuild barfs on it.

I will send new version with some changes from Sonal.

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.