linux-kernel - Re: [PATCH 1/5] i8042: intel-8042 DT documentation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20150203193202.GB20256@dtor-ws>
Date:	Tue, 3 Feb 2015 11:32:02 -0800
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Roman Volkov <v1ron@...os.org>, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <Pawel.Moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	Hans de Goede <hdegoede@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Wolfram Sang <wsa@...-dreams.de>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Roman Volkov <v1ron@...l.ru>,
	Tony Prisk <linux@...sktech.co.nz>
Subject: Re: [PATCH 1/5] i8042: intel-8042 DT documentation

On Tue, Feb 03, 2015 at 11:38:16AM +0000, Mark Rutland wrote:
> On Mon, Feb 02, 2015 at 09:48:46PM +0000, Roman Volkov wrote:
> > Documentation for 'intel,8042' DT compatible node.
> > 
> > Signed-off-by: Tony Prisk <linux@...sktech.co.nz>
> > Signed-off-by: Roman Volkov <v1ron@...os.org>
> > ---
> >  .../devicetree/bindings/input/intel-8042.txt       | 29 ++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/intel-8042.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/input/intel-8042.txt b/Documentation/devicetree/bindings/input/intel-8042.txt
> > new file mode 100644
> > index 0000000..2aea7ec
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/intel-8042.txt
> > @@ -0,0 +1,29 @@
> > +* Intel 8042 Keyboard Controller
> > +
> > +Required properties:
> > +- compatible: should be "intel,8042"
> > +- regs: memory for keyboard controller
> > +- interrupts: two interrupts should be specified (keyboard and aux)
> 
> Is it possible only one of these is wired up?

Yes, and we should support this case. The core of i8042 does.


> 
> It might be worth using interrupt-names.
> 
> > +- command-reg: offset in memory for command register
> > +- status-reg: offset in memory for status register
> > +- data-reg: offset in memory for data register
> > +
> > +Optional properties:
> > +- init-reset: Controller should be reset on init and cleanup
> 
> Why is this necessary? Can't we just always reset it?

We do not reset by default on x86 because BIOS takes care of this for us
and quite often firmware that emulates i8042 gets confused if we try to
reset it too. Non non-x86 we reset by default. I think we should do the
same for OF case  (reset) and not use this property.

> 
> > +
> > +Optional Linux-specific properties:
> > +- linux,kbd_phys_desc: defaults to i8042/serio0
> > +- linux,aux_phys_desc: defaults to i8042/serio1
> > +- linux,mux_phys_desc: defaults to i8042/serio%d
> 
> As a general note, s/_/-/ in property names please.
> 
> That said, I don't follow why we should have these at all. I don't
> understand what the description is intended to mean.
> 
> In general we want to avoid Linux-specific properties. If a DTB needs to
> know about the inernals of an OS it's likely to be fragile and broken
> over time.

Right, the desc were carried over from older days to keep dmesg
familiar. With OF it is new platforms so just settle on a generic
description and use it instead of allowing to specify through DT.

> 
> > +
> > +
> > +Example:
> > +	keyboard@...08800 {
> > +		compatible = "intel,8042";
> > +		reg = <0xd8008800 0x100>;
> > +		interrupts = <23 4>;
> 
> If this is intended to be two interrupts, please bracket them
> individually, e.g.
> 
> 	interrupts = <23>, <4>;
> 
> > +		command-reg = <0x04>;
> > +		status-reg = <0x04>;
> 
> Same address?
> 
> > +		data-reg = <0x00>;
> > +		mux-ports = <2>;
> 
> This wasn't documented above.

I think active MUX is purely x86 concept, I have never heard of it being
used anywhere else.

Thanks.

-- 
Dmitry
--
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/