[PATCH 11/12] unicore32 machine related files: ps2 driver

[PATCH 11/12] unicore32 machine related files: ps2 driver

[Guan Xuetao]

Message-Id: <bca6d64c27c47036b940df5085b899a38fa15a82.1297842537.git.gxt@mprc.pku.edu.cn>
In-Reply-To: <cover.1297842537.git.gxt@mprc.pku.edu.cn>
References: <cover.1297842537.git.gxt@mprc.pku.edu.cn>
From: GuanXuetao <gxt@mprc.pku.edu.cn>
Date: Sat, 15 Jan 2011 18:28:19 +0800

This patch implements arch-specific ps2 driver.

By reviewed with Dmitry Torokhov:
     1. move i8042-ucio.h to drivers/input/serio/i8042-unicore32io.h
     2. move puv3_ps2_init() to arch/unicore32/kernel/puv3-core.c
     3. remove unused comments.

Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
Acked-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/serio/i8042-unicore32io.h |   70 +++++++++++++++++++++++++++++++
 drivers/input/serio/i8042.h             |    2 +
 2 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/serio/i8042-unicore32io.h

diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
new file mode 100644
index 0000000..6a7e8b3
--- /dev/null
+++ b/drivers/input/serio/i8042-unicore32io.h
@@ -0,0 +1,70 @@
+/*
+ * Code specific to PKUnity SoC and UniCore ISA
+ *
+ *	Maintained by GUAN Xue-tao <gxt@mprc.pku.edu.cn>
+ *	Copyright (C) 2001-2011 Guan Xuetao
+ *
+ * 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.
+ */
+#ifndef _I8042_UNICORE32_H
+#define _I8042_UNICORE32_H
+
+#include <mach/hardware.h>
+
+/*
+ * Names.
+ */
+#define I8042_KBD_PHYS_DESC "isa0060/serio0"
+#define I8042_AUX_PHYS_DESC "isa0060/serio1"
+#define I8042_MUX_PHYS_DESC "isa0060/serio%d"
+
+/*
+ * IRQs.
+ */
+#define I8042_KBD_IRQ           IRQ_PS2_KBD
+#define I8042_AUX_IRQ           IRQ_PS2_AUX
+
+/*
+ * Register numbers.
+ */
+#define I8042_COMMAND_REG	((unsigned long)&PS2_COMMAND)
+#define I8042_STATUS_REG	((unsigned long)&PS2_STATUS)
+#define I8042_DATA_REG		((unsigned long)&PS2_DATA)
+
+static inline int i8042_read_data(void)
+{
+	return inb(I8042_DATA_REG);
+}
+
+static inline int i8042_read_status(void)
+{
+	return inb(I8042_STATUS_REG);
+}
+
+static inline void i8042_write_data(int val)
+{
+	outb(val, I8042_DATA_REG);
+}
+
+static inline void i8042_write_command(int val)
+{
+	outb(val, I8042_COMMAND_REG);
+}
+
+static inline int i8042_platform_init(void)
+{
+	if (!request_region(I8042_DATA_REG, 16, "i8042"))
+		return -EBUSY;
+
+	i8042_reset = 1;
+	return 0;
+}
+
+static inline void i8042_platform_exit(void)
+{
+	release_region(I8042_DATA_REG, 16);
+}
+
+#endif /* _I8042_UNICORE32_H */
diff --git a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
index cbc1beb..e0fe4af 100644
--- a/drivers/input/serio/i8042.h
+++ b/drivers/input/serio/i8042.h
@@ -26,6 +26,8 @@
 #include "i8042-sparcio.h"
 #elif defined(CONFIG_X86) || defined(CONFIG_IA64)
 #include "i8042-x86ia64io.h"
+#elif defined(CONFIG_UNICORE32)
+#include "i8042-unicore32io.h"
 #else
 #include "i8042-io.h"
 #endif
-- 
1.6.2.2

Re: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Arnd Bergmann]

On Wednesday 16 February 2011, Guan Xuetao wrote:
> +/*
> + * Register numbers.
> + */
> +#define I8042_COMMAND_REG      ((unsigned long)&PS2_COMMAND)
> +#define I8042_STATUS_REG       ((unsigned long)&PS2_STATUS)
> +#define I8042_DATA_REG         ((unsigned long)&PS2_DATA)
> +
> +static inline int i8042_read_data(void)
> +{
> +       return inb(I8042_DATA_REG);
> +}
> +
> +static inline int i8042_read_status(void)
> +{
> +       return inb(I8042_STATUS_REG);
> +}
> +

This is not a correct way to use inb()/outb(), as far as I can tell:
PS2_COMMAND is an mmio pointer (or should be, see my other message).

inb() however is only defined on PCI/ISA PIO port numbers, which
are in the range between 0 and 65535, and typically get mapped
into the memory from the PCI driver.

	Arnd

RE: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Guan Xuetao]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Friday, February 18, 2011 1:03 AM
> To: Guan Xuetao
> Cc: linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; 'Greg KH'
> Subject: Re: [PATCH 11/12] unicore32 machine related files: ps2 driver
> 
> On Wednesday 16 February 2011, Guan Xuetao wrote:
> > +/*
> > + * Register numbers.
> > + */
> > +#define I8042_COMMAND_REG      ((unsigned long)&PS2_COMMAND)
> > +#define I8042_STATUS_REG       ((unsigned long)&PS2_STATUS)
> > +#define I8042_DATA_REG         ((unsigned long)&PS2_DATA)
> > +
> > +static inline int i8042_read_data(void)
> > +{
> > +       return inb(I8042_DATA_REG);
> > +}
> > +
> > +static inline int i8042_read_status(void)
> > +{
> > +       return inb(I8042_STATUS_REG);
> > +}
> > +
> 
> This is not a correct way to use inb()/outb(), as far as I can tell:
> PS2_COMMAND is an mmio pointer (or should be, see my other message).
> 
> inb() however is only defined on PCI/ISA PIO port numbers, which
> are in the range between 0 and 65535, and typically get mapped
> into the memory from the PCI driver.
Thanks.
Please see my patch following, and cc to Dmitry Torokhov.

From: GuanXuetao <gxt@mprc.pku.edu.cn>
Date: Fri, 18 Feb 2011 18:38:33 +0800
Subject: [PATCH] unicore32: adjust i8042-unicore32io codes
   replace inb/outb with readb/writeb in i8042-unicore32io.h
   and correct typecasting of register and region macros
   -- by advice with Arnd Bergmann

Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
---
 drivers/input/serio/i8042-unicore32io.h |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
index 6a7e8b3..2cdd872 100644
--- a/drivers/input/serio/i8042-unicore32io.h
+++ b/drivers/input/serio/i8042-unicore32io.h
@@ -29,33 +29,36 @@
 /*
  * Register numbers.
  */
-#define I8042_COMMAND_REG	((unsigned long)&PS2_COMMAND)
-#define I8042_STATUS_REG	((unsigned long)&PS2_STATUS)
-#define I8042_DATA_REG		((unsigned long)&PS2_DATA)
+#define I8042_COMMAND_REG	((volatile void __iomem *)&PS2_COMMAND)
+#define I8042_STATUS_REG	((volatile void __iomem *)&PS2_STATUS)
+#define I8042_DATA_REG		((volatile void __iomem *)&PS2_DATA)
+
+#define I8042_REGION_START	(resource_size_t)(&PS2_DATA)
+#define I8042_REGION_SIZE	(resource_size_t)(16)
 
 static inline int i8042_read_data(void)
 {
-	return inb(I8042_DATA_REG);
+	return readb(I8042_DATA_REG);
 }
 
 static inline int i8042_read_status(void)
 {
-	return inb(I8042_STATUS_REG);
+	return readb(I8042_STATUS_REG);
 }
 
 static inline void i8042_write_data(int val)
 {
-	outb(val, I8042_DATA_REG);
+	writeb(val, I8042_DATA_REG);
 }
 
 static inline void i8042_write_command(int val)
 {
-	outb(val, I8042_COMMAND_REG);
+	writeb(val, I8042_COMMAND_REG);
 }
 
 static inline int i8042_platform_init(void)
 {
-	if (!request_region(I8042_DATA_REG, 16, "i8042"))
+	if (!request_region(I8042_REGION_START, I8042_REGION_SIZE, "i8042"))
 		return -EBUSY;
 
 	i8042_reset = 1;
@@ -64,7 +67,7 @@ static inline int i8042_platform_init(void)
 
 static inline void i8042_platform_exit(void)
 {
-	release_region(I8042_DATA_REG, 16);
+	release_region(I8042_REGION_START, I8042_REGION_SIZE);
 }
 
 #endif /* _I8042_UNICORE32_H */
-- 
1.6.2.2

> 
> 	Arnd

Thanks & Regards.

Guan Xuetao

Re: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Arnd Bergmann]

On Friday 18 February 2011 11:28:45 Guan Xuetao wrote:
>   * Register numbers.
>   */
> -#define I8042_COMMAND_REG      ((unsigned long)&PS2_COMMAND)
> -#define I8042_STATUS_REG       ((unsigned long)&PS2_STATUS)
> -#define I8042_DATA_REG         ((unsigned long)&PS2_DATA)
> +#define I8042_COMMAND_REG      ((volatile void __iomem *)&PS2_COMMAND)
> +#define I8042_STATUS_REG       ((volatile void __iomem *)&PS2_STATUS)
> +#define I8042_DATA_REG         ((volatile void __iomem *)&PS2_DATA)
> +
> +#define I8042_REGION_START     (resource_size_t)(&PS2_DATA)
> +#define I8042_REGION_SIZE      (resource_size_t)(16)

It would be better to remove the cast and make the PS2_COMMAND
etc macros have the correct type. Aside from this, the change
looks good.

	Arnd

RE: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Guan Xuetao]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Friday, February 18, 2011 6:33 PM
> To: Guan Xuetao
> Cc: dmitry.torokhov@gmail.com; linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; 'Greg KH'
> Subject: Re: [PATCH 11/12] unicore32 machine related files: ps2 driver
> 
> On Friday 18 February 2011 11:28:45 Guan Xuetao wrote:
> >   * Register numbers.
> >   */
> > -#define I8042_COMMAND_REG      ((unsigned long)&PS2_COMMAND)
> > -#define I8042_STATUS_REG       ((unsigned long)&PS2_STATUS)
> > -#define I8042_DATA_REG         ((unsigned long)&PS2_DATA)
> > +#define I8042_COMMAND_REG      ((volatile void __iomem *)&PS2_COMMAND)
> > +#define I8042_STATUS_REG       ((volatile void __iomem *)&PS2_STATUS)
> > +#define I8042_DATA_REG         ((volatile void __iomem *)&PS2_DATA)
> > +
> > +#define I8042_REGION_START     (resource_size_t)(&PS2_DATA)
> > +#define I8042_REGION_SIZE      (resource_size_t)(16)
> 
> It would be better to remove the cast and make the PS2_COMMAND
> etc macros have the correct type. Aside from this, the change
> looks good.
With the patch for __REG in unicore32, following patch could be applied:
drivers/input/serio/i8042-unicore32io.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
index 2cdd872..9350843 100644
--- a/drivers/input/serio/i8042-unicore32io.h
+++ b/drivers/input/serio/i8042-unicore32io.h
@@ -29,11 +29,11 @@
 /*
  * Register numbers.
  */
-#define I8042_COMMAND_REG	((volatile void __iomem *)&PS2_COMMAND)
-#define I8042_STATUS_REG	((volatile void __iomem *)&PS2_STATUS)
-#define I8042_DATA_REG		((volatile void __iomem *)&PS2_DATA)
+#define I8042_COMMAND_REG	PS2_COMMAND
+#define I8042_STATUS_REG	PS2_STATUS
+#define I8042_DATA_REG		PS2_DATA
 
-#define I8042_REGION_START	(resource_size_t)(&PS2_DATA)
+#define I8042_REGION_START	(resource_size_t)(PS2_DATA)
 #define I8042_REGION_SIZE	(resource_size_t)(16)
 
 static inline int i8042_read_data(void)

> 
> 	Arnd
Thanks & Regards.

Guan Xuetao

Re: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Arnd Bergmann]

On Tuesday 22 February 2011, Guan Xuetao wrote:
> diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
> index 2cdd872..9350843 100644
> --- a/drivers/input/serio/i8042-unicore32io.h
> +++ b/drivers/input/serio/i8042-unicore32io.h
> @@ -29,11 +29,11 @@
>  /*
>   * Register numbers.
>   */
> -#define I8042_COMMAND_REG      ((volatile void __iomem *)&PS2_COMMAND)
> -#define I8042_STATUS_REG       ((volatile void __iomem *)&PS2_STATUS)
> -#define I8042_DATA_REG         ((volatile void __iomem *)&PS2_DATA)
> +#define I8042_COMMAND_REG      PS2_COMMAND
> +#define I8042_STATUS_REG       PS2_STATUS
> +#define I8042_DATA_REG         PS2_DATA

I wouldn't bother defining two sets of macros then. Either use PS2_COMMAND
in the driver, or define I8042_COMMAND_REG in the place where you currently
define PS2_COMMAND.

Aside from this, it looks good.

	Arnd

RE: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Guan Xuetao]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, February 23, 2011 1:12 AM
> To: Guan Xuetao
> Cc: dmitry.torokhov@gmail.com; linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; 'Greg KH'
> Subject: Re: [PATCH 11/12] unicore32 machine related files: ps2 driver
> 
> On Tuesday 22 February 2011, Guan Xuetao wrote:
> > diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
> > index 2cdd872..9350843 100644
> > --- a/drivers/input/serio/i8042-unicore32io.h
> > +++ b/drivers/input/serio/i8042-unicore32io.h
> > @@ -29,11 +29,11 @@
> >  /*
> >   * Register numbers.
> >   */
> > -#define I8042_COMMAND_REG      ((volatile void __iomem *)&PS2_COMMAND)
> > -#define I8042_STATUS_REG       ((volatile void __iomem *)&PS2_STATUS)
> > -#define I8042_DATA_REG         ((volatile void __iomem *)&PS2_DATA)
> > +#define I8042_COMMAND_REG      PS2_COMMAND
> > +#define I8042_STATUS_REG       PS2_STATUS
> > +#define I8042_DATA_REG         PS2_DATA
> 
> I wouldn't bother defining two sets of macros then. Either use PS2_COMMAND
> in the driver, or define I8042_COMMAND_REG in the place where you currently
> define PS2_COMMAND.
All I8042_*_REGs are explicitly defined in i8042-*io.h files.
And I want to reserve the original register names in unicore32 namespace.
> 
> Aside from this, it looks good.
> 
> 	Arnd
Thanks & Regards.
Guan Xuetao

RE: [PATCH 11/12] unicore32 machine related files: ps2 driver

[Guan Xuetao]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, February 23, 2011 1:12 AM
> To: Guan Xuetao
> Cc: dmitry.torokhov@gmail.com; linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; 'Greg KH'
> Subject: Re: [PATCH 11/12] unicore32 machine related files: ps2 driver
> 
> On Tuesday 22 February 2011, Guan Xuetao wrote:
> > diff --git a/drivers/input/serio/i8042-unicore32io.h b/drivers/input/serio/i8042-unicore32io.h
> > index 2cdd872..9350843 100644
> > --- a/drivers/input/serio/i8042-unicore32io.h
> > +++ b/drivers/input/serio/i8042-unicore32io.h
> > @@ -29,11 +29,11 @@
> >  /*
> >   * Register numbers.
> >   */
> > -#define I8042_COMMAND_REG      ((volatile void __iomem *)&PS2_COMMAND)
> > -#define I8042_STATUS_REG       ((volatile void __iomem *)&PS2_STATUS)
> > -#define I8042_DATA_REG         ((volatile void __iomem *)&PS2_DATA)
> > +#define I8042_COMMAND_REG      PS2_COMMAND
> > +#define I8042_STATUS_REG       PS2_STATUS
> > +#define I8042_DATA_REG         PS2_DATA
> 
> I wouldn't bother defining two sets of macros then. Either use PS2_COMMAND
> in the driver, or define I8042_COMMAND_REG in the place where you currently
> define PS2_COMMAND.
All I8042_*_REGs are explicitly defined in i8042-*io.h files.
And I want to reserve the original register names in unicore32 namespace.
> 
> Aside from this, it looks good.
> 
> 	Arnd
Thanks & Regards.
Guan Xuetao