linux-kernel - Re: [PATCH] Input: cros_ec_keyb

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] [day] [month] [year] [list]

Message-ID: <20160725205043.GI27415@dtor-ws>
Date:	Mon, 25 Jul 2016 13:50:43 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Brian Norris <briannorris@...omium.org>
Cc:	Brian Norris <computersforpeace@...il.com>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev

On Mon, Jul 25, 2016 at 01:46:05PM -0700, Brian Norris wrote:
> On Mon, Jul 25, 2016 at 01:31:51PM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 25, 2016 at 11:43:48AM -0700, Brian Norris wrote:
> > > From: Douglas Anderson <dianders@...omium.org>
> > > 
> > > In cros_ec_keyb we stored "dev" in "struct cros_ec_keyb", but this was
> > > the EC's dev pointer and not the keyboard's.  Let's clean this up to
> > > make it the keyboard's dev pointer.  This could be useful in future
> > > patches but also has the nice effect of changing a few printouts to
> > > include the name of the keyboard device instead of the EC device, which
> > > seems more technically correct.
> > 
> > What is the difference in the output? Could you show old vs new example?
> 
> For instance, on a veyron Chromebook, I see this on bootup with v4.7:
> 
> # dmesg | grep -e cros-ec -e keyb
> [    0.539153] cros-ec-spi spi0.0: Chrome EC device registered
> [    1.224505] cros-ec-spi spi0.0: valid_keys[00] = 0x14
> [    1.224509] cros-ec-spi spi0.0: valid_keys[01] = 0xff
> [    1.224514] cros-ec-spi spi0.0: valid_keys[02] = 0xff
> [    1.224518] cros-ec-spi spi0.0: valid_keys[03] = 0xff
> [    1.224521] cros-ec-spi spi0.0: valid_keys[04] = 0xff
> [    1.224525] cros-ec-spi spi0.0: valid_keys[05] = 0xf5
> [    1.224529] cros-ec-spi spi0.0: valid_keys[06] = 0xff
> [    1.224533] cros-ec-spi spi0.0: valid_keys[07] = 0xa4
> [    1.224536] cros-ec-spi spi0.0: valid_keys[08] = 0xff
> [    1.224540] cros-ec-spi spi0.0: valid_keys[09] = 0xfe
> [    1.224544] cros-ec-spi spi0.0: valid_keys[10] = 0x55
> [    1.224548] cros-ec-spi spi0.0: valid_keys[11] = 0xfa
> [    1.224551] cros-ec-spi spi0.0: valid_keys[12] = 0xca
> [    1.224632] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input0
> 
> and this, with this patch applied:
> 
> # dmesg | grep -e cros-ec -e keyb
> [    0.539437] cros-ec-spi spi0.0: Chrome EC device registered
> [    1.224648] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[00] = 0x14
> [    1.224653] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[01] = 0xff
> [    1.224657] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[02] = 0xff
> [    1.224662] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[03] = 0xff
> [    1.224666] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[04] = 0xff
> [    1.224670] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[05] = 0xf5
> [    1.224674] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[06] = 0xff
> [    1.224678] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[07] = 0xa4
> [    1.224682] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[08] = 0xff
> [    1.224686] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[09] = 0xfe
> [    1.224690] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[10] = 0x55
> [    1.224694] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[11] = 0xfa
> [    1.224698] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[12] = 0xca
> [    1.224779] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input
> 
> (both are done with an added '#define DEBUG', since that's easier than fiddling
> with dynamic debug prints)
> 
> I think it's important to see the "cros-ec-keyb" driver name, and the
> specific device name is a nice bonus.
> 
> Do you want to paste something like this into the commit description, or
> should I resend?

Yes, I munged the commit message a tiny bit, thank you.

-- 
Dmitry