Message-Id: <bca6d64c27c47036b940df5085b899a38fa15a82.1297842537.git.gxt@mprc.pku.edu.cn>
In-Reply-To: <[email protected]>
References: <[email protected]>
From: GuanXuetao <[email protected]>
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 <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
---
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 <[email protected]>
+ * 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
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
> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Friday, February 18, 2011 1:03 AM
> To: Guan Xuetao
> Cc: [email protected]; [email protected]; '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 <[email protected]>
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 <[email protected]>
---
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
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
> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Friday, February 18, 2011 6:33 PM
> To: Guan Xuetao
> Cc: [email protected]; [email protected]; [email protected]; '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
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
> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Wednesday, February 23, 2011 1:12 AM
> To: Guan Xuetao
> Cc: [email protected]; [email protected]; [email protected]; '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