* [PATCH] i8042 driver for unicore32 architecture
@ 2011-01-16 15:45 Guan Xuetao
2011-01-17 17:39 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Guan Xuetao @ 2011-01-16 15:45 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input
This patch implements i8042 driver for unicore32 architecture.
Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
---
drivers/input/serio/i8042.h | 2 +
drivers/staging/puv3/i8042-ucio.h | 89 +++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/puv3/i8042-ucio.h
diff --git a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
index cbc1beb..711b41c 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 "../../staging/puv3/i8042-ucio.h"
#else
#include "i8042-io.h"
#endif
diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
new file mode 100644
index 0000000..c3221df
--- /dev/null
+++ b/drivers/staging/puv3/i8042-ucio.h
@@ -0,0 +1,89 @@
+/*
+ * linux/drivers/staging/puv3/i8042-ucio.h
+ *
+ * Code specific to PKUnity SoC and UniCore ISA
+ *
+ * Maintained by GUAN Xue-tao <gxt@mprc.pku.edu.cn>
+ * Copyright (C) 2001-2010 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_UCIO_H
+#define _I8042_UCIO_H
+
+#include <linux/clk.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)
+
+void puv3_ps2_init(void)
+{
+ struct clk *bclk32;
+
+ bclk32 = clk_get(NULL, "BUS32_CLK");
+ PS2_CNT = clk_get_rate(bclk32) / 200000; /* should > 5us */
+}
+
+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)
+{
+/*
+ * On some platforms touching the i8042 data register region can do really
+ * bad things. Because of this the region is always reserved on such boxes.
+ */
+ puv3_ps2_init();
+
+ 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_UCIO_H */
--
1.7.3.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] i8042 driver for unicore32 architecture
2011-01-16 15:45 [PATCH] i8042 driver for unicore32 architecture Guan Xuetao
@ 2011-01-17 17:39 ` Dmitry Torokhov
2011-01-18 3:31 ` Guan Xuetao
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2011-01-17 17:39 UTC (permalink / raw)
To: Guan Xuetao; +Cc: linux-input
Hi Guan,
On Sun, Jan 16, 2011 at 11:45:23PM +0800, Guan Xuetao wrote:
> This patch implements i8042 driver for unicore32 architecture.
>
> Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
> ---
> drivers/input/serio/i8042.h | 2 +
> drivers/staging/puv3/i8042-ucio.h | 89 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 91 insertions(+), 0 deletions(-)
> create mode 100644 drivers/staging/puv3/i8042-ucio.h
>
> diff --git a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
> index cbc1beb..711b41c 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 "../../staging/puv3/i8042-ucio.h"
I'd rather you put it directly into drivers/input/serio/
Also, maybe we should call it i8042-unicore32io.h
> #else
> #include "i8042-io.h"
> #endif
> diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
> new file mode 100644
> index 0000000..c3221df
> --- /dev/null
> +++ b/drivers/staging/puv3/i8042-ucio.h
> @@ -0,0 +1,89 @@
> +/*
> + * linux/drivers/staging/puv3/i8042-ucio.h
Please do not put filenames in the comments.
> + *
> + * Code specific to PKUnity SoC and UniCore ISA
> + *
> + * Maintained by GUAN Xue-tao <gxt@mprc.pku.edu.cn>
> + * Copyright (C) 2001-2010 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_UCIO_H
> +#define _I8042_UCIO_H
> +
> +#include <linux/clk.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)
This looks a bit iffy... any chance to simplify register definitions?
> +
> +void puv3_ps2_init(void)
Should it be static?
> +{
> + struct clk *bclk32;
> +
> + bclk32 = clk_get(NULL, "BUS32_CLK");
Error hanlding?
> + PS2_CNT = clk_get_rate(bclk32) / 200000; /* should > 5us */
> +}
> +
> +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)
> +{
> +/*
> + * On some platforms touching the i8042 data register region can do really
> + * bad things. Because of this the region is always reserved on such boxes.
> + */
The comment is in wrong place and is not applicable to your arch I
think.
> + puv3_ps2_init();
> +
> + 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_UCIO_H */
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] i8042 driver for unicore32 architecture
2011-01-17 17:39 ` Dmitry Torokhov
@ 2011-01-18 3:31 ` Guan Xuetao
2011-01-18 4:11 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Guan Xuetao @ 2011-01-18 3:31 UTC (permalink / raw)
To: 'Dmitry Torokhov'; +Cc: linux-input
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, January 18, 2011 1:40 AM
> To: Guan Xuetao
> Cc: linux-input@vger.kernel.org
> Subject: Re: [PATCH] i8042 driver for unicore32 architecture
>
> Hi Guan,
>
> On Sun, Jan 16, 2011 at 11:45:23PM +0800, Guan Xuetao wrote:
> > This patch implements i8042 driver for unicore32 architecture.
> >
> > Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
> > ---
> > drivers/input/serio/i8042.h | 2 +
> > drivers/staging/puv3/i8042-ucio.h | 89 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 91 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/staging/puv3/i8042-ucio.h
> >
> > diff --git a/drivers/input/serio/i8042.h b/drivers/input/serio/i8042.h
> > index cbc1beb..711b41c 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 "../../staging/puv3/i8042-ucio.h"
>
> I'd rather you put it directly into drivers/input/serio/
> Also, maybe we should call it i8042-unicore32io.h
Ok.
>
> > #else
> > #include "i8042-io.h"
> > #endif
> > diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
> > new file mode 100644
> > index 0000000..c3221df
> > --- /dev/null
> > +++ b/drivers/staging/puv3/i8042-ucio.h
> > @@ -0,0 +1,89 @@
> > +/*
> > + * linux/drivers/staging/puv3/i8042-ucio.h
>
> Please do not put filenames in the comments.
That's ok.
But why?
>
> > + *
> > + * Code specific to PKUnity SoC and UniCore ISA
> > + *
> > + * Maintained by GUAN Xue-tao <gxt@mprc.pku.edu.cn>
> > + * Copyright (C) 2001-2010 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_UCIO_H
> > +#define _I8042_UCIO_H
> > +
> > +#include <linux/clk.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)
>
> This looks a bit iffy... any chance to simplify register definitions?
All hardware registers on unicore32 use __REG macro, which acts well for
both left value and right value in assignment sentence. However, when __REG
is used for inb/outb, the registers need "&" prefix to be used as io addresses.
>
> > +
> > +void puv3_ps2_init(void)
>
> Should it be static?
Sorry for using not-static function in header file.
We have a counter register for ps2, which need initialization when machine startup, both cold startup
and wakeup from sleep. Its value is computed by using BUS32_CLK.
>
> > +{
> > + struct clk *bclk32;
> > +
> > + bclk32 = clk_get(NULL, "BUS32_CLK");
>
> Error hanlding?
Not error handling, see above explanation.
>
> > + PS2_CNT = clk_get_rate(bclk32) / 200000; /* should > 5us */
> > +}
> > +
> > +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)
> > +{
> > +/*
> > + * On some platforms touching the i8042 data register region can do really
> > + * bad things. Because of this the region is always reserved on such boxes.
> > + */
>
> The comment is in wrong place and is not applicable to your arch I
> think.
Ok, removed. Request_region and release_region are also removed.
>
> > + puv3_ps2_init();
> > +
> > + 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_UCIO_H */
>
> Thanks.
>
> --
> Dmitry
Thanks Dmitry.
Guan Xuetao
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i8042 driver for unicore32 architecture
2011-01-18 3:31 ` Guan Xuetao
@ 2011-01-18 4:11 ` Dmitry Torokhov
2011-01-18 4:45 ` Guan Xuetao
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2011-01-18 4:11 UTC (permalink / raw)
To: Guan Xuetao; +Cc: linux-input
On Tue, Jan 18, 2011 at 11:31:30AM +0800, Guan Xuetao wrote:
> > > diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
> > > new file mode 100644
> > > index 0000000..c3221df
> > > --- /dev/null
> > > +++ b/drivers/staging/puv3/i8042-ucio.h
> > > @@ -0,0 +1,89 @@
> > > +/*
> > > + * linux/drivers/staging/puv3/i8042-ucio.h
> >
> > Please do not put filenames in the comments.
> That's ok.
> But why?
Because:
1. It does not convey any additional information - your editor is sure
capable of displaying the filename associated with a buffer, and
2. It is one more thing that needs updating if you decide to move/rename
the file.
> > > +
> > > +static inline int i8042_platform_init(void)
> > > +{
> > > +/*
> > > + * On some platforms touching the i8042 data register region can do really
> > > + * bad things. Because of this the region is always reserved on such boxes.
> > > + */
> >
> > The comment is in wrong place and is not applicable to your arch I
> > think.
> Ok, removed. Request_region and release_region are also removed.
So are the regions reserved elsewhere? Might be a good idea to note this
fact.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] i8042 driver for unicore32 architecture
2011-01-18 4:11 ` Dmitry Torokhov
@ 2011-01-18 4:45 ` Guan Xuetao
2011-01-18 4:52 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Guan Xuetao @ 2011-01-18 4:45 UTC (permalink / raw)
To: 'Dmitry Torokhov'; +Cc: linux-input
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, January 18, 2011 12:11 PM
> To: Guan Xuetao
> Cc: linux-input@vger.kernel.org
> Subject: Re: [PATCH] i8042 driver for unicore32 architecture
>
> On Tue, Jan 18, 2011 at 11:31:30AM +0800, Guan Xuetao wrote:
> > > > diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
> > > > new file mode 100644
> > > > index 0000000..c3221df
> > > > --- /dev/null
> > > > +++ b/drivers/staging/puv3/i8042-ucio.h
> > > > @@ -0,0 +1,89 @@
> > > > +/*
> > > > + * linux/drivers/staging/puv3/i8042-ucio.h
> > >
> > > Please do not put filenames in the comments.
> > That's ok.
> > But why?
>
> Because:
>
> 1. It does not convey any additional information - your editor is sure
> capable of displaying the filename associated with a buffer, and
>
> 2. It is one more thing that needs updating if you decide to move/rename
> the file.
I see.
>
> > > > +
> > > > +static inline int i8042_platform_init(void)
> > > > +{
> > > > +/*
> > > > + * On some platforms touching the i8042 data register region can do really
> > > > + * bad things. Because of this the region is always reserved on such boxes.
> > > > + */
> > >
> > > The comment is in wrong place and is not applicable to your arch I
> > > think.
> > Ok, removed. Request_region and release_region are also removed.
>
> So are the regions reserved elsewhere? Might be a good idea to note this
> fact.
Not reserved. I think it unnecessary, since in unicore32, user application can't acess
io space directly.
>
> Thanks.
>
> --
> Dmitry
Thanks
Guan Xuetao
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] i8042 driver for unicore32 architecture
2011-01-18 4:45 ` Guan Xuetao
@ 2011-01-18 4:52 ` Dmitry Torokhov
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2011-01-18 4:52 UTC (permalink / raw)
To: Guan Xuetao; +Cc: linux-input
On Tue, Jan 18, 2011 at 12:45:06PM +0800, Guan Xuetao wrote:
>
>
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: Tuesday, January 18, 2011 12:11 PM
> > To: Guan Xuetao
> > Cc: linux-input@vger.kernel.org
> > Subject: Re: [PATCH] i8042 driver for unicore32 architecture
> >
> > On Tue, Jan 18, 2011 at 11:31:30AM +0800, Guan Xuetao wrote:
> > > > > diff --git a/drivers/staging/puv3/i8042-ucio.h b/drivers/staging/puv3/i8042-ucio.h
> > > > > new file mode 100644
> > > > > index 0000000..c3221df
> > > > > --- /dev/null
> > > > > +++ b/drivers/staging/puv3/i8042-ucio.h
> > > > > @@ -0,0 +1,89 @@
> > > > > +/*
> > > > > + * linux/drivers/staging/puv3/i8042-ucio.h
> > > >
> > > > Please do not put filenames in the comments.
> > > That's ok.
> > > But why?
> >
> > Because:
> >
> > 1. It does not convey any additional information - your editor is sure
> > capable of displaying the filename associated with a buffer, and
> >
> > 2. It is one more thing that needs updating if you decide to move/rename
> > the file.
> I see.
>
> >
> > > > > +
> > > > > +static inline int i8042_platform_init(void)
> > > > > +{
> > > > > +/*
> > > > > + * On some platforms touching the i8042 data register region can do really
> > > > > + * bad things. Because of this the region is always reserved on such boxes.
> > > > > + */
> > > >
> > > > The comment is in wrong place and is not applicable to your arch I
> > > > think.
> > > Ok, removed. Request_region and release_region are also removed.
> >
> > So are the regions reserved elsewhere? Might be a good idea to note this
> > fact.
> Not reserved. I think it unnecessary, since in unicore32, user application can't acess
> io space directly.
It is not for benefit of userspace applications, it is to ensure that 2
drivers do not try to access the same ports.
So please put request_region and release_region back ;)
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-18 4:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-16 15:45 [PATCH] i8042 driver for unicore32 architecture Guan Xuetao
2011-01-17 17:39 ` Dmitry Torokhov
2011-01-18 3:31 ` Guan Xuetao
2011-01-18 4:11 ` Dmitry Torokhov
2011-01-18 4:45 ` Guan Xuetao
2011-01-18 4:52 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).