On Thu, Mar 17, 2011 at 01:27:22PM -0700, Abhijeet Dharmapurikar wrote: > Grant Likely wrote: > >On Wed, Mar 16, 2011 at 07:23:58PM -0700, adharmap@xxxxxxxxxxxxxx wrote: > >>Add support for GPIO on Qualcomm PM8xxx PMIC chips. > >> > >>Signed-off-by: Abhijeet Dharmapurikar <adharmap@xxxxxxxxxxxxxx> > > > >Minor points below, but otherwise: > > > >Acked-by: Grant Likely <grant.likely@xxxxxxxxxxxx> > > Thanks Grant, can you review (and ack) drivers/mfd/pm8xxx-mpp.c? Now > that I think, mpp is basically a gpio device, so should I move it to > drivers/gpio? > > mpp is the 4th patch in this series and can be found here. > https://patchwork.kernel.org/patch/640891/ I've only looked briefly, but it appears to be fine. Yes, you should move it to drivers/gpio g. > > >>diff --git a/drivers/gpio/pm8xxx-gpio.c b/drivers/gpio/pm8xxx-gpio.c > >>new file mode 100644 > >>index 0000000..8995764 > >>--- /dev/null > >>+++ b/drivers/gpio/pm8xxx-gpio.c > >>@@ -0,0 +1,455 @@ > >>+/* Copyright (c) 2011, Code Aurora Forum. All rights reserved. > > > >Nit: first line should be '/*' and start the comments on the second > >line. It also helps for the first line of the comment block to be a > >single line description of what the driver actually is. (a > >description, not a filename). > > > >>+ * > >>+ * This program is free software; you can redistribute it and/or modify > >>+ * it under the terms of the GNU General Public License version 2 and > >>+ * only version 2 as published by the Free Software Foundation. > >>+ * > >>+ * This program is distributed in the hope that it will be useful, > >>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of > >>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >>+ * GNU General Public License for more details. > >>+ */ > >>+ > >>+/* > >>+ * Qualcomm PMIC8XXX GPIO driver > > > >Heh, this is the line I just was talking about should appear at the > >top of the file. :-) > > Ok I will make this change for all the files in the patchset. > > >>+ > >>+static int __init pm_gpio_init(void) > >>+{ > >>+ int rc = platform_driver_register(&pm_gpio_driver); > >>+ > >>+ return rc; > >>+} > > > >Or simply: > > > > return platform_driver_register(&pm_gpio_driver); > > yes will do. > > -- > Sent by an employee of the Qualcomm Innovation Center, Inc. The > Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html