[PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Raghu Gandham]

The standard IO regions are already reserved by the platform code on most MIPS
devices(malta, cobalt, sni). The Commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
("Input: i8042-io - fix up region handling on MIPS") introduced a bug on these
MIPS platforms causing i8042 driver to fail when trying to reserve IO ports.
Prior to the above mentioned commit request_region is skipped on MIPS but
release_region is called.

This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444 and also
avoids calling release_region for MIPS.

Signed-off-by: Raghu Gandham <raghu.gandham@imgtec.com>
Reviewed-by: Steven J. Hill <Steven.Hill@imgtec.com>
---
 drivers/input/serio/i8042-io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
index a5eed2a..a09bb72 100644
--- a/drivers/input/serio/i8042-io.h
+++ b/drivers/input/serio/i8042-io.h
@@ -76,7 +76,7 @@ static inline int i8042_platform_init(void)
 	if (check_legacy_ioport(I8042_DATA_REG))
 		return -ENODEV;
 #endif
-#if !defined(__sh__) && !defined(__alpha__)
+#if !defined(__sh__) && !defined(__alpha__) && !defined(__mips__)
 	if (!request_region(I8042_DATA_REG, 16, "i8042"))
 		return -EBUSY;
 #endif
@@ -87,7 +87,7 @@ static inline int i8042_platform_init(void)
 
 static inline void i8042_platform_exit(void)
 {
-#if !defined(__sh__) && !defined(__alpha__)
+#if !defined(__sh__) && !defined(__alpha__) && !defined(__mips__)
 	release_region(I8042_DATA_REG, 16);
 #endif
 }
-- 
1.8.5.2

Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Dmitry Torokhov]

Hi Raghu,

On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> The standard IO regions are already reserved by the platform code on most MIPS
> devices(malta, cobalt, sni). The Commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
> ("Input: i8042-io - fix up region handling on MIPS") introduced a bug on these
> MIPS platforms causing i8042 driver to fail when trying to reserve IO ports.
> Prior to the above mentioned commit request_region is skipped on MIPS but
> release_region is called.
> 
> This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444 and also
> avoids calling release_region for MIPS.

The problem is that IO regions are reserved on _most_, but not _all_
devices. MIPS should figure out what they want to do with i8042
registers and be consistent on all devices.

I do not want to apply this patch because it will be breaking devices
using the other configuration.

Thanks.

-- 
Dmitry

RE: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Raghu Gandham]

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

Hi Dmitry, 

> 
> On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > The standard IO regions are already reserved by the platform code on
> > most MIPS devices(malta, cobalt, sni). The Commit
> > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > ("Input: i8042-io - fix up region handling on MIPS") introduced a bug
> > on these MIPS platforms causing i8042 driver to fail when trying to reserve
> IO ports.
> > Prior to the above mentioned commit request_region is skipped on MIPS
> > but release_region is called.
> >
> > This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
> and
> > also avoids calling release_region for MIPS.
> 
> The problem is that IO regions are reserved on _most_, but not _all_ devices.
> MIPS should figure out what they want to do with i8042 registers and be
> consistent on all devices.

Please examine the attached patch which went upstream in April of 2004.  Since then
it had been a convention not to call request_region routine in i8042 for MIPS. The
attached patch had a glitch that it guarded request_region in i8042-io.h but skipped 
guarding release_region in i8042-io.h. I believe that the issue Aaro saw was due to this 
glitch. Below is the error quoted in Aaro's commit message.

    [    2.112000] Trying to free nonexistent resource <0000000000000060-000000000000006f>

My patch reinstates the convention followed on MIPS devices along with fixing Aaro's issue.

Thanks,
Raghu

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 10823 bytes --]

commit 2b7058b32c3f1706a80d54b06ad238f3c0bd09e9
Author: Andrew Morton <akpm@osdl.org>
Date:   Wed Apr 21 23:37:12 2004 -0700

    [PATCH] Merge missing MIPS i8042 bits
    
    From: Ralf Baechle <ralf@linux-mips.org>
    
    - Add HPC3 PS/2 driver bits for SGI IP22 aka Indy
    - Add Mace PS/2 driver bits for SGI IP32 aka O2
    - Add R4030 PS/2 driver bits for Jazz family
    - Don't register I/O ports where we're using the I/O port memory window
       to access the i8042 registers

diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 10a54dd..cacd1f2 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -130,3 +130,13 @@ config SERIO_PCIPS2
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called pcips2.
+
+config SERIO_MACEPS2
+	tristate "SGI O2 MACE PS/2 controller"
+	depends on SGI_IP32 && SERIO
+	help
+	  Say Y here if you have SGI O2 workstation and want to use its
+	  PS/2 ports.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called maceps2.
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index 2eb86eb..a47dec2 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_SERIO_Q40KBD)	+= q40kbd.o
 obj-$(CONFIG_SERIO_98KBD)	+= 98kbd-io.o
 obj-$(CONFIG_SERIO_GSCPS2)	+= gscps2.o
 obj-$(CONFIG_SERIO_PCIPS2)	+= pcips2.o
+obj-$(CONFIG_SERIO_MACEPS2)	+= maceps2.o
diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
index 6b434f8..9b36485 100644
--- a/drivers/input/serio/i8042-io.h
+++ b/drivers/input/serio/i8042-io.h
@@ -69,7 +69,7 @@ static inline int i8042_platform_init(void)
  * On ix86 platforms touching the i8042 data register region can do really
  * bad things. Because of this the region is always reserved on ix86 boxes.
  */
-#if !defined(__i386__) && !defined(__sh__) && !defined(__alpha__) && !defined(__x86_64__)
+#if !defined(__i386__) && !defined(__sh__) && !defined(__alpha__) && !defined(__x86_64__) && !defined(__mips__)
 	if (!request_region(I8042_DATA_REG, 16, "i8042"))
 		return -1;
 #endif
diff --git a/drivers/input/serio/i8042-ip22io.h b/drivers/input/serio/i8042-ip22io.h
new file mode 100644
index 0000000..863b9c9
--- /dev/null
+++ b/drivers/input/serio/i8042-ip22io.h
@@ -0,0 +1,76 @@
+#ifndef _I8042_IP22_H
+#define _I8042_IP22_H
+
+#include <asm/sgi/ioc.h>
+#include <asm/sgi/ip22.h>
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+/*
+ * Names.
+ */
+
+#define I8042_KBD_PHYS_DESC "hpc3ps2/serio0"
+#define I8042_AUX_PHYS_DESC "hpc3ps2/serio1"
+#define I8042_MUX_PHYS_DESC "hpc3ps2/serio%d"
+
+/*
+ * IRQs.
+ */
+
+#define I8042_KBD_IRQ SGI_KEYBD_IRQ
+#define I8042_AUX_IRQ SGI_KEYBD_IRQ
+
+/*
+ * Register numbers.
+ */
+
+#define I8042_COMMAND_REG	((unsigned long)&sgioc->kbdmouse.command)
+#define I8042_STATUS_REG	((unsigned long)&sgioc->kbdmouse.command)
+#define I8042_DATA_REG		((unsigned long)&sgioc->kbdmouse.data)
+
+static inline int i8042_read_data(void)
+{
+	return sgioc->kbdmouse.data;
+}
+
+static inline int i8042_read_status(void)
+{
+	return sgioc->kbdmouse.command;
+}
+
+static inline void i8042_write_data(int val)
+{
+	sgioc->kbdmouse.data = val;
+}
+
+static inline void i8042_write_command(int val)
+{
+	sgioc->kbdmouse.command = val;
+}
+
+static inline int i8042_platform_init(void)
+{
+#if 0
+	/* XXX sgi_kh is a virtual address */
+	if (!request_mem_region(sgi_kh, sizeof(struct hpc_keyb), "i8042"))
+		return 1;
+#endif
+
+	i8042_reset = 1;
+
+	return 0;
+}
+
+static inline void i8042_platform_exit(void)
+{
+#if 0
+	release_mem_region(JAZZ_KEYBOARD_ADDRESS, sizeof(struct hpc_keyb));
+#endif
+}
+
+#endif /* _I8042_IP22_H */
diff --git a/drivers/input/serio/i8042-jazzio.h b/drivers/input/serio/i8042-jazzio.h
new file mode 100644
index 0000000..5c20ab1
--- /dev/null
+++ b/drivers/input/serio/i8042-jazzio.h
@@ -0,0 +1,69 @@
+#ifndef _I8042_JAZZ_H
+#define _I8042_JAZZ_H
+
+#include <asm/jazz.h>
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+/*
+ * Names.
+ */
+
+#define I8042_KBD_PHYS_DESC "R4030/serio0"
+#define I8042_AUX_PHYS_DESC "R4030/serio1"
+#define I8042_MUX_PHYS_DESC "R4030/serio%d"
+
+/*
+ * IRQs.
+ */
+
+#define I8042_KBD_IRQ JAZZ_KEYBOARD_IRQ
+#define I8042_AUX_IRQ JAZZ_MOUSE_IRQ
+
+#define I8042_COMMAND_REG	((unsigned long)&jazz_kh->command)
+#define I8042_STATUS_REG	((unsigned long)&jazz_kh->command)
+#define I8042_DATA_REG		((unsigned long)&jazz_kh->data)
+
+static inline int i8042_read_data(void)
+{
+	return jazz_kh->data;
+}
+
+static inline int i8042_read_status(void)
+{
+	return jazz_kh->command;
+}
+
+static inline void i8042_write_data(int val)
+{
+	jazz_kh->data = val;
+}
+
+static inline void i8042_write_command(int val)
+{
+	jazz_kh->command = val;
+}
+
+static inline int i8042_platform_init(void)
+{
+#if 0
+	/* XXX JAZZ_KEYBOARD_ADDRESS is a virtual address */
+	if (!request_mem_region(JAZZ_KEYBOARD_ADDRESS, 2, "i8042"))
+		return 1;
+#endif
+
+	return 0;
+}
+
+static inline void i8042_platform_exit(void)
+{
+#if 0
+	release_mem_region(JAZZ_KEYBOARD_ADDRESS, 2);
+#endif
+}
+
+#endif /* _I8042_JAZZ_H */
diff --git a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
index 3d59eb2..f0f6374 100644
--- a/drivers/input/serio/i8042.h
+++ b/drivers/input/serio/i8042.h
@@ -1,6 +1,8 @@
 #ifndef _I8042_H
 #define _I8042_H
 
+#include <linux/config.h>
+
 /*
  *  Copyright (c) 1999-2002 Vojtech Pavlik
  *
@@ -13,7 +15,11 @@
  * Arch-dependent inline functions and defines.
  */
 
-#if defined(CONFIG_PPC)
+#if defined(CONFIG_MIPS_JAZZ)
+#include "i8042-jazzio.h"
+#elif defined(CONFIG_SGI_IP22)
+#include "i8042-ip22io.h"
+#elif defined(CONFIG_PPC)
 #include "i8042-ppcio.h"
 #elif defined(CONFIG_SPARC32) || defined(CONFIG_SPARC64)
 #include "i8042-sparcio.h"
diff --git a/drivers/input/serio/maceps2.c b/drivers/input/serio/maceps2.c
new file mode 100644
index 0000000..c7db1de
--- /dev/null
+++ b/drivers/input/serio/maceps2.c
@@ -0,0 +1,160 @@
+/*
+ * SGI O2 MACE PS2 controller driver for linux
+ *
+ * Copyright (C) 2002 Vivien Chappelier
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/serio.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <asm/io.h>
+#include <asm/irq.h>
+#include <asm/system.h>
+#include <asm/ip32/mace.h>
+#include <asm/ip32/ip32_ints.h>
+
+MODULE_AUTHOR("Vivien Chappelier <vivien.chappelier@linux-mips.org");
+MODULE_DESCRIPTION("SGI O2 MACE PS2 controller driver");
+MODULE_LICENSE("GPL");
+
+#define MACE_PS2_TIMEOUT 10000 /* in 50us unit */
+
+#define PS2_STATUS_CLOCK_SIGNAL  BIT(0) /* external clock signal */
+#define PS2_STATUS_CLOCK_INHIBIT BIT(1) /* clken output signal */
+#define PS2_STATUS_TX_INPROGRESS BIT(2) /* transmission in progress */
+#define PS2_STATUS_TX_EMPTY      BIT(3) /* empty transmit buffer */
+#define PS2_STATUS_RX_FULL       BIT(4) /* full receive buffer */
+#define PS2_STATUS_RX_INPROGRESS BIT(5) /* reception in progress */
+#define PS2_STATUS_ERROR_PARITY  BIT(6) /* parity error */
+#define PS2_STATUS_ERROR_FRAMING BIT(7) /* framing error */
+
+#define PS2_CONTROL_TX_CLOCK_DISABLE BIT(0) /* inhibit clock signal after TX */
+#define PS2_CONTROL_TX_ENABLE        BIT(1) /* transmit enable */
+#define PS2_CONTROL_TX_INT_ENABLE    BIT(2) /* enable transmit interrupt */
+#define PS2_CONTROL_RX_INT_ENABLE    BIT(3) /* enable receive interrupt */
+#define PS2_CONTROL_RX_CLOCK_ENABLE  BIT(4) /* pause reception if set to 0 */
+#define PS2_CONTROL_RESET            BIT(5) /* reset */
+
+
+struct maceps2_data {
+	struct mace_ps2port *port;
+	int irq;
+};
+
+static int maceps2_write(struct serio *dev, unsigned char val)
+{
+	struct mace_ps2port *port = ((struct maceps2_data *)dev->driver)->port;
+	unsigned int timeout = MACE_PS2_TIMEOUT;
+
+	do {
+		if (mace_read(port->status) & PS2_STATUS_TX_EMPTY) {
+			mace_write(val, port->tx);
+			return 0;
+		}
+		udelay(50);
+	} while (timeout--);
+
+	return -1;
+}
+
+static irqreturn_t maceps2_interrupt(int irq, void *dev_id,
+				     struct pt_regs *regs)
+{
+	struct serio *dev = dev_id;
+	struct mace_ps2port *port = ((struct maceps2_data *)dev->driver)->port;
+	unsigned int byte;
+
+	if (mace_read(port->status) & PS2_STATUS_RX_FULL) {
+		byte = mace_read(port->rx);
+		serio_interrupt(dev, byte & 0xff, 0, regs);
+        }
+
+	return IRQ_HANDLED;
+}
+
+static int maceps2_open(struct serio *dev)
+{
+	struct maceps2_data *data = (struct maceps2_data *)dev->driver;
+
+	if (request_irq(data->irq, maceps2_interrupt, 0, "PS/2 port", dev)) {
+		printk(KERN_ERR "Could not allocate PS/2 IRQ\n");
+		return -EBUSY;
+	}
+
+	/* Reset port */
+	mace_write(PS2_CONTROL_TX_CLOCK_DISABLE | PS2_CONTROL_RESET,
+		   data->port->control);
+	udelay(100);
+
+        /* Enable interrupts */
+	mace_write(PS2_CONTROL_RX_CLOCK_ENABLE | PS2_CONTROL_TX_ENABLE |
+		   PS2_CONTROL_RX_INT_ENABLE, data->port->control);
+
+	return 0;
+}
+
+static void maceps2_close(struct serio *dev)
+{
+	struct maceps2_data *data = (struct maceps2_data *)dev->driver;
+
+	mace_write(PS2_CONTROL_TX_CLOCK_DISABLE | PS2_CONTROL_RESET,
+		   data->port->control);
+	udelay(100);
+	free_irq(data->irq, dev);
+}
+
+static struct maceps2_data port0_data, port1_data;
+
+static struct serio maceps2_port0 =
+{
+	.type	= SERIO_8042,
+	.open	= maceps2_open,
+	.close	= maceps2_close,
+	.write	= maceps2_write,
+	.name	= "MACE PS/2 port0",
+	.phys	= "mace/serio0",
+	.driver = &port0_data,
+};
+
+static struct serio maceps2_port1 =
+{
+	.type	= SERIO_8042,
+	.open	= maceps2_open,
+	.close	= maceps2_close,
+	.write	= maceps2_write,
+	.name	= "MACE PS/2 port1",
+	.phys	= "mace/serio1",
+	.driver = &port1_data,
+};
+
+static int __init maceps2_init(void)
+{
+	port0_data.port = &mace->perif.ps2.keyb;
+	port0_data.irq  = MACEISA_KEYB_IRQ;
+	port1_data.port = &mace->perif.ps2.mouse;
+	port1_data.irq  = MACEISA_MOUSE_IRQ;
+	serio_register_port(&maceps2_port0);
+	serio_register_port(&maceps2_port1);
+
+	return 0;
+}
+
+static void __exit maceps2_exit(void)
+{
+	serio_unregister_port(&maceps2_port0);
+	serio_unregister_port(&maceps2_port1);
+}
+
+module_init(maceps2_init);
+module_exit(maceps2_exit);

Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Dmitry Torokhov]

On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> Hi Dmitry, 
> 
> > 
> > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > The standard IO regions are already reserved by the platform code on
> > > most MIPS devices(malta, cobalt, sni). The Commit
> > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > ("Input: i8042-io - fix up region handling on MIPS") introduced a bug
> > > on these MIPS platforms causing i8042 driver to fail when trying to reserve
> > IO ports.
> > > Prior to the above mentioned commit request_region is skipped on MIPS
> > > but release_region is called.
> > >
> > > This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > and
> > > also avoids calling release_region for MIPS.
> > 
> > The problem is that IO regions are reserved on _most_, but not _all_ devices.
> > MIPS should figure out what they want to do with i8042 registers and be
> > consistent on all devices.
> 
> Please examine the attached patch which went upstream in April of 2004.  Since then
> it had been a convention not to call request_region routine in i8042 for MIPS. The
> attached patch had a glitch that it guarded request_region in i8042-io.h but skipped 
> guarding release_region in i8042-io.h. I believe that the issue Aaro saw was due to this 
> glitch. Below is the error quoted in Aaro's commit message.
> 
>     [    2.112000] Trying to free nonexistent resource <0000000000000060-000000000000006f>
> 
> My patch reinstates the convention followed on MIPS devices along with fixing Aaro's issue.

I assume that Aaro did test his patch and on his box request_region()
succeeds. That would indicate that various MIPS sub-arches still not
settled on the topic.

Aaro?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Aaro Koskinen]

Hi,

On Sun, Jan 26, 2014 at 10:56:38PM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> > > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > > The standard IO regions are already reserved by the platform code on
> > > > most MIPS devices(malta, cobalt, sni). The Commit
> > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > ("Input: i8042-io - fix up region handling on MIPS") introduced a bug
> > > > on these MIPS platforms causing i8042 driver to fail when trying to
> > > > reserve IO ports.
> > > > Prior to the above mentioned commit request_region is skipped on MIPS
> > > > but release_region is called.
> > > >
> > > > This patch reverts commit 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > and also avoids calling release_region for MIPS.
> > > 
> > > The problem is that IO regions are reserved on _most_, but not _all_
> > > devices.
> > > MIPS should figure out what they want to do with i8042 registers and be
> > > consistent on all devices.
> > 
> > Please examine the attached patch which went upstream in April of 2004.
> > Since then it had been a convention not to call request_region routine in
> > i8042 for MIPS. The attached patch had a glitch that it guarded
> > request_region in i8042-io.h but skipped guarding release_region in
> > i8042-io.h. I believe that the issue Aaro saw was due to this 
> > glitch. Below is the error quoted in Aaro's commit message.
> > 
> >     [    2.112000] Trying to free nonexistent resource <0000000000000060-000000000000006f>
> > 
> > My patch reinstates the convention followed on MIPS devices along with
> > fixing Aaro's issue.
> 
> I assume that Aaro did test his patch and on his box request_region()
> succeeds. That would indicate that various MIPS sub-arches still not
> settled on the topic.

request_region() succeeds on Loongson and OCTEON.

On OCTEONs without PCI, request_region() will fail which is correct as
there is no I/O space.

I wasn't aware of that 2004 patch (it pre-dates GIT history of mainline
Linux). Why the regions are already reserved by the platform code?

A.

RE: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Raghu Gandham]

Hi Aaro,

> 
> On Sun, Jan 26, 2014 at 10:56:38PM -0800, Dmitry Torokhov wrote:
> > On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> > > > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > > > The standard IO regions are already reserved by the platform
> > > > > code on most MIPS devices(malta, cobalt, sni). The Commit
> > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > ("Input: i8042-io - fix up region handling on MIPS") introduced
> > > > > a bug on these MIPS platforms causing i8042 driver to fail when
> > > > > trying to reserve IO ports.
> > > > > Prior to the above mentioned commit request_region is skipped on
> > > > > MIPS but release_region is called.
> > > > >
> > > > > This patch reverts commit
> > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > and also avoids calling release_region for MIPS.
> > > >
> > > > The problem is that IO regions are reserved on _most_, but not
> > > > _all_ devices.
> > > > MIPS should figure out what they want to do with i8042 registers
> > > > and be consistent on all devices.
> > >
> > > Please examine the attached patch which went upstream in April of 2004.
> > > Since then it had been a convention not to call request_region
> > > routine in
> > > i8042 for MIPS. The attached patch had a glitch that it guarded
> > > request_region in i8042-io.h but skipped guarding release_region in
> > > i8042-io.h. I believe that the issue Aaro saw was due to this
> > > glitch. Below is the error quoted in Aaro's commit message.
> > >
> > >     [    2.112000] Trying to free nonexistent resource <0000000000000060-
> 000000000000006f>
> > >
> > > My patch reinstates the convention followed on MIPS devices along
> > > with fixing Aaro's issue.
> >
> > I assume that Aaro did test his patch and on his box request_region()
> > succeeds. That would indicate that various MIPS sub-arches still not
> > settled on the topic.
> 
> request_region() succeeds on Loongson and OCTEON.

This would mean that before your patch in oct of 2012, Loongson and Octeon 
were not reserving the IO space for i8042.

> 
> On OCTEONs without PCI, request_region() will fail which is correct as there
> is no I/O space.
> 
> I wasn't aware of that 2004 patch (it pre-dates GIT history of mainline Linux).
> Why the regions are already reserved by the platform code?

The only information I have is the comment before request_region in i8042-io.h that
touching data register on some platforms is flaky.  If your patch was primarily aimed at
addressing the error message from release_region, the current patch I uploaded should
also take care of it too. 

Thanks,
Raghu

Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Ralf Baechle]

On Tue, Jan 28, 2014 at 06:25:39AM +0000, Raghu Gandham wrote:
> Date:   Tue, 28 Jan 2014 06:25:39 +0000
> From: Raghu Gandham <Raghu.Gandham@imgtec.com>
> To: Aaro Koskinen <aaro.koskinen@iki.fi>, Dmitry Torokhov
>  <dmitry.torokhov@gmail.com>
> CC: "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
>  "linux-mips@linux-mips.org" <linux-mips@linux-mips.org>,
>  "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
> Subject: RE: [PATCH] Input: i8042-io - Exclude mips platforms when
>  allocating/deallocating IO regions.
> Content-Type: text/plain; charset="us-ascii"
> 
> Hi Aaro,
> 
> > 
> > On Sun, Jan 26, 2014 at 10:56:38PM -0800, Dmitry Torokhov wrote:
> > > On Mon, Jan 27, 2014 at 12:32:36AM +0000, Raghu Gandham wrote:
> > > > > On Sat, Jan 25, 2014 at 11:01:54AM -0800, Raghu Gandham wrote:
> > > > > > The standard IO regions are already reserved by the platform
> > > > > > code on most MIPS devices(malta, cobalt, sni). The Commit
> > > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > > ("Input: i8042-io - fix up region handling on MIPS") introduced
> > > > > > a bug on these MIPS platforms causing i8042 driver to fail when
> > > > > > trying to reserve IO ports.
> > > > > > Prior to the above mentioned commit request_region is skipped on
> > > > > > MIPS but release_region is called.
> > > > > >
> > > > > > This patch reverts commit
> > > > > > 197a1e96c8be5b6005145af3a4c0e45e2d651444
> > > > > > and also avoids calling release_region for MIPS.
> > > > >
> > > > > The problem is that IO regions are reserved on _most_, but not
> > > > > _all_ devices.
> > > > > MIPS should figure out what they want to do with i8042 registers
> > > > > and be consistent on all devices.
> > > >
> > > > Please examine the attached patch which went upstream in April of 2004.
> > > > Since then it had been a convention not to call request_region
> > > > routine in
> > > > i8042 for MIPS. The attached patch had a glitch that it guarded
> > > > request_region in i8042-io.h but skipped guarding release_region in
> > > > i8042-io.h. I believe that the issue Aaro saw was due to this
> > > > glitch. Below is the error quoted in Aaro's commit message.
> > > >
> > > >     [    2.112000] Trying to free nonexistent resource <0000000000000060-
> > 000000000000006f>
> > > >
> > > > My patch reinstates the convention followed on MIPS devices along
> > > > with fixing Aaro's issue.
> > >
> > > I assume that Aaro did test his patch and on his box request_region()
> > > succeeds. That would indicate that various MIPS sub-arches still not
> > > settled on the topic.
> > 
> > request_region() succeeds on Loongson and OCTEON.
> 
> This would mean that before your patch in oct of 2012, Loongson and Octeon 
> were not reserving the IO space for i8042.
> 
> > 
> > On OCTEONs without PCI, request_region() will fail which is correct as there
> > is no I/O space.
> > 
> > I wasn't aware of that 2004 patch (it pre-dates GIT history of mainline Linux).
> > Why the regions are already reserved by the platform code?
> 
> The only information I have is the comment before request_region in i8042-io.h that
> touching data register on some platforms is flaky.  If your patch was primarily aimed at
> addressing the error message from release_region, the current patch I uploaded should
> also take care of it too. 

I think the patch (http://patchwork.linux-mips.org/patch/6419/) should be
applied.  The argumentation for reserving ports in the platform code are
the same as on x86 - touch the registers and bad things may happen.  This
is because a fair number of older MIPS platforms were based on x86 chipsets
or at least are using very similar designs.

The fact that on certain platforms such as Loongson, some Octeon systems
and others the request_region() call to reserve the keyboard call succeeds
is by accident not design.

I wish i8042.c was a real platform driver, not using platform_create_bundle.
That would leave a natural place in the arch/platform code to deal with
I/O port allocation and platform_device creation, as necessary for a
platform.

  Ralf

Re: [PATCH] Input: i8042-io - Exclude mips platforms when allocating/deallocating IO regions.

[Aaro Koskinen]

Hi,

On Wed, Mar 19, 2014 at 09:49:31PM +0100, Ralf Baechle wrote:
> I think the patch (http://patchwork.linux-mips.org/patch/6419/) should be
> applied.  The argumentation for reserving ports in the platform code are
> the same as on x86 - touch the registers and bad things may happen.  This
> is because a fair number of older MIPS platforms were based on x86 chipsets
> or at least are using very similar designs.

If you drop the request_region() from the driver, it will try to probe
anyway regardless what the platform code codes. So bad things could happen,
no?

Currently we can prevent i8042 driver from probing I/O space on PCI-less
Octeons (for example), because we define empty I/O space so request_region()
by driver will fail. So we can actually prevent bad things from happening.
I would call this good design, not an accident.

Maybe I'm missing something? Anyway, I don't have strong feelings whether
this patch is applied or not. My computers will keep on working on either
case.

A.