Re: [PATCH 1/5] i8042: intel-8042 DT documentation
В Tue, 3 Feb 2015 11:32:02 -0800
Dmitry Torokhov пишет:
> On Tue, Feb 03, 2015 at 11:38:16AM +, Mark Rutland wrote:
> > On Mon, Feb 02, 2015 at 09:48:46PM +, Roman Volkov wrote:
> > > Documentation for 'intel,8042' DT compatible node.
> > >
> > > Signed-off-by: Tony Prisk
> > > Signed-off-by: Roman Volkov
> > > ---
> > > .../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 000..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.
>
Do we need to just read these IRQ numbers and leave them negative if
absent? Will it be acceptable? This would look like:
i8042_kbd_irq = platform_get_irq_byname(pdev, "kbd");
Testing shows it prints "Invalid argument" error -22 when an IRQ is
absent and we are not using nokbd/noaux module options.
>
> >
> > 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@d8008800 {
> > > + 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.
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] i8042: intel-8042 DT documentation
On Tue, Feb 03, 2015 at 11:38:16AM +, Mark Rutland wrote:
> On Mon, Feb 02, 2015 at 09:48:46PM +, Roman Volkov wrote:
> > Documentation for 'intel,8042' DT compatible node.
> >
> > Signed-off-by: Tony Prisk
> > Signed-off-by: Roman Volkov
> > ---
> > .../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 000..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@d8008800 {
> > + 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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] i8042: intel-8042 DT documentation
On Mon, Feb 02, 2015 at 09:48:46PM +, Roman Volkov wrote:
> Documentation for 'intel,8042' DT compatible node.
>
> Signed-off-by: Tony Prisk
> Signed-off-by: Roman Volkov
> ---
> .../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 000..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?
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?
> +
> +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.
> +
> +
> +Example:
> + keyboard@d8008800 {
> + 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.
Thanks,
Mark.
> + };
> --
> 2.2.2
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] i8042: intel-8042 DT documentation
В Tue, 3 Feb 2015 00:48:46 +0300
Roman Volkov пишет:
> Documentation for 'intel,8042' DT compatible node.
>
> Signed-off-by: Tony Prisk
> Signed-off-by: Roman Volkov
> ---
> .../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 000..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)
> +- 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
> +
> +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
> +
> +
> +Example:
> + keyboard@d8008800 {
> + compatible = "intel,8042";
> + reg = <0xd8008800 0x100>;
> + interrupts = <23 4>;
> + command-reg = <0x04>;
> + status-reg = <0x04>;
> + data-reg = <0x00>;
> + mux-ports = <2>;
> + };
Hi,
This patch set is to enable the Open Firmware Device Tree support for
the i8042 controller. Yes, some ARM SoC boards are still using i8042. As
an example, it is the vt8500 architecture.
I've tested this on my wm8505 machine in both configurations: as a
module and built-in. Also a modification of this driver is available
at https://github.com/linux-wmt/linux-vtwm. This should not affect x86
and similar architectures without the DT enabled in the config.
Regards,
Roman.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] i8042: intel-8042 DT documentation
Documentation for 'intel,8042' DT compatible node.
Signed-off-by: Tony Prisk
Signed-off-by: Roman Volkov
---
.../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 000..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)
+- 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
+
+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
+
+
+Example:
+ keyboard@d8008800 {
+ compatible = "intel,8042";
+ reg = <0xd8008800 0x100>;
+ interrupts = <23 4>;
+ command-reg = <0x04>;
+ status-reg = <0x04>;
+ data-reg = <0x00>;
+ mux-ports = <2>;
+ };
--
2.2.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
