On Tue, Jan 18, 2011 at 12:45:06PM +0800, Guan Xuetao wrote: > > > > -----Original Message----- > > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] > > Sent: Tuesday, January 18, 2011 12:11 PM > > To: Guan Xuetao > > Cc: linux-input@xxxxxxxxxxxxxxx > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html