| Message ID | E2EE47005FA75F44B80E1019FDD2EBBB6E38B06D@BADAG02.ba.imgtec.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);