Re: [PATCHv8 01/16] power: add omap prm driver skeleton

2011-09-21 Thread Paul Walmsley
Hi

a few comments.

First, as Govindraj mentioned, this should be cc'ed to 
linux-arm-ker...@lists.infradead.org.  It's surprising that the linux-arm 
mailing list is still running...

 On Fri, 16 Sep 2011, Tero Kristo wrote:

 This driver will eventually support OMAP soc PRM module features, e.g. PRCM
 chain interrupts.

This driver should be a separate series and should be posted to 
linux-ker...@vger.kernel.org also, since it will presumably need to be 
acked or merged by someone else who is responsible for the appropriate 
drivers directory.  Since it should probably go into drivers/mfd (see 
below), it should also be sent to the MFD tree maintainer, Samuel Ortiz 
sa...@linux.intel.com.

 Signed-off-by: Tero Kristo t-kri...@ti.com
 Cc: Paul Walmsley p...@pwsan.com
 Cc: Kevin Hilman khil...@ti.com
 ---
  drivers/power/Kconfig|7 
  drivers/power/Makefile   |1 +
  drivers/power/omap_prm.c |   83 
 ++

A couple of comments here:

This driver is eventually going to need to create the VC and VP 
subdevices, so it makes sense to me to place the main portion of the 
driver into drivers/mfd, rather than drivers/power, to avoid churn.

Also, the driver should probably be renamed to something like 
'omap2430_prm.c' or something like that.  It looks to me like there are at 
least three different hardware PRM IP block designs:

- OMAP2420 (integrated into the PRCM)
- OMAP2430/3430/3630/3517 (standalone PRM)
- OMAP4430/4460, possibly also the 816x (standalone PRM with very 
  different register layout)

The idea being that the hwmods for these would all have different names 
and would therefore be associated with different drivers/mfd drivers.

Looking at your patch 6/16, it looks like there's some code that can be 
shared between OMAP3/4 for interrupt handling, so maybe that can go into a 
shared file, like omap_prm_common.c.  The omap2430_prm.c or omap4430_prm.c 
files could pass the number of registers and IRQ register offset arrays to 
the common code in platform_data.  drivers/mfd/wm8350-{core,irq}.c is what 
I'm looking at here as a reasonable approach to compare to; see 
wm8350_irq_init().

  3 files changed, 91 insertions(+), 0 deletions(-)
  create mode 100644 drivers/power/omap_prm.c
 
 diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
 index 57de051..e735a95 100644
 --- a/drivers/power/Kconfig
 +++ b/drivers/power/Kconfig
 @@ -249,4 +249,11 @@ config CHARGER_MAX8998
 Say Y to enable support for the battery charger control sysfs and
 platform data of MAX8998/LP3974 PMICs.
  
 +config OMAP_PRM
 + bool OMAP Power and Reset Management driver
 + depends on (ARCH_OMAP)  PM
 + help
 +   Say Y to enable support for the OMAP Power and Reset Management
 +   driver.
 +
  endif # POWER_SUPPLY
 diff --git a/drivers/power/Makefile b/drivers/power/Makefile
 index b4af13d..8df93c2 100644
 --- a/drivers/power/Makefile
 +++ b/drivers/power/Makefile
 @@ -13,6 +13,7 @@ obj-$(CONFIG_WM831X_BACKUP) += wm831x_backup.o
  obj-$(CONFIG_WM831X_POWER)   += wm831x_power.o
  obj-$(CONFIG_WM8350_POWER)   += wm8350_power.o
  obj-$(CONFIG_TEST_POWER) += test_power.o
 +obj-$(CONFIG_OMAP_PRM)   += omap_prm.o
  
  obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
  obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
 diff --git a/drivers/power/omap_prm.c b/drivers/power/omap_prm.c
 new file mode 100644
 index 000..dfc0920
 --- /dev/null
 +++ b/drivers/power/omap_prm.c
 @@ -0,0 +1,83 @@
 +/*
 + * OMAP Power and Reset Management (PRM) driver
 + *
 + * Copyright (C) 2011 Texas Instruments, Inc.
 + *
 + * Author: Tero Kristo t-kri...@ti.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#include linux/kernel.h
 +#include linux/ctype.h
 +#include linux/module.h
 +#include linux/io.h
 +#include linux/slab.h
 +#include linux/init.h
 +#include linux/err.h
 +#include linux/platform_device.h
 +
 +#define DRIVER_NAME omap-prm
 +
 +struct omap_prm_device {
 + struct platform_device  pdev;
 +};

Hmm.  This doesn't look right.  The platform_device should be created as 
part of an omap_device_build() call in code that resides in 
arch/arm/mach-omap2/ - see for example arch/arm/mach-omap2/hwspinlock.c

 +
 +static struct omap_prm_device prm_dev = {
 + .pdev   = {
 + .name   = DRIVER_NAME,
 + .id = -1,
 + },
 +};
 +
 +static int __init omap_prm_probe(struct platform_device *pdev)
 +{
 + return 0;
 +}
 +
 +static int __devexit omap_prm_remove(struct platform_device *pdev)
 +{
 + return 0;
 +}
 +
 +static struct platform_driver prm_driver = {
 + .remove = __exit_p(omap_prm_remove),

This should define a probe function that the LDM core 

[PATCHv8 01/16] power: add omap prm driver skeleton

2011-09-16 Thread Tero Kristo
This driver will eventually support OMAP soc PRM module features, e.g. PRCM
chain interrupts.

Signed-off-by: Tero Kristo t-kri...@ti.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Kevin Hilman khil...@ti.com
---
 drivers/power/Kconfig|7 
 drivers/power/Makefile   |1 +
 drivers/power/omap_prm.c |   83 ++
 3 files changed, 91 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/omap_prm.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 57de051..e735a95 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -249,4 +249,11 @@ config CHARGER_MAX8998
  Say Y to enable support for the battery charger control sysfs and
  platform data of MAX8998/LP3974 PMICs.
 
+config OMAP_PRM
+   bool OMAP Power and Reset Management driver
+   depends on (ARCH_OMAP)  PM
+   help
+ Say Y to enable support for the OMAP Power and Reset Management
+ driver.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b4af13d..8df93c2 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_WM831X_BACKUP)   += wm831x_backup.o
 obj-$(CONFIG_WM831X_POWER) += wm831x_power.o
 obj-$(CONFIG_WM8350_POWER) += wm8350_power.o
 obj-$(CONFIG_TEST_POWER)   += test_power.o
+obj-$(CONFIG_OMAP_PRM) += omap_prm.o
 
 obj-$(CONFIG_BATTERY_DS2760)   += ds2760_battery.o
 obj-$(CONFIG_BATTERY_DS2780)   += ds2780_battery.o
diff --git a/drivers/power/omap_prm.c b/drivers/power/omap_prm.c
new file mode 100644
index 000..dfc0920
--- /dev/null
+++ b/drivers/power/omap_prm.c
@@ -0,0 +1,83 @@
+/*
+ * OMAP Power and Reset Management (PRM) driver
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ *
+ * Author: Tero Kristo t-kri...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/kernel.h
+#include linux/ctype.h
+#include linux/module.h
+#include linux/io.h
+#include linux/slab.h
+#include linux/init.h
+#include linux/err.h
+#include linux/platform_device.h
+
+#define DRIVER_NAME omap-prm
+
+struct omap_prm_device {
+   struct platform_device  pdev;
+};
+
+static struct omap_prm_device prm_dev = {
+   .pdev   = {
+   .name   = DRIVER_NAME,
+   .id = -1,
+   },
+};
+
+static int __init omap_prm_probe(struct platform_device *pdev)
+{
+   return 0;
+}
+
+static int __devexit omap_prm_remove(struct platform_device *pdev)
+{
+   return 0;
+}
+
+static struct platform_driver prm_driver = {
+   .remove = __exit_p(omap_prm_remove),
+   .driver = {
+   .name   = DRIVER_NAME,
+   },
+};
+
+static int __init omap_prm_init(void)
+{
+   int ret;
+
+   ret = platform_device_register(prm_dev.pdev);
+
+   if (ret) {
+   printk(KERN_ERR Unable to register PRM device: %d\n, ret);
+   return ret;
+   }
+
+   ret = platform_driver_probe(prm_driver, omap_prm_probe);
+
+   if (ret)
+   printk(KERN_ERR PRM driver probe failed: %d\n, ret);
+
+   return ret;
+}
+subsys_initcall(omap_prm_init);
+
+static void __exit omap_prm_exit(void)
+{
+   platform_device_unregister(prm_dev.pdev);
+   platform_driver_unregister(prm_driver);
+}
+module_exit(omap_prm_exit);
+
+MODULE_ALIAS(platform:DRIVER_NAME);
+MODULE_AUTHOR(Tero Kristo t-kri...@ti.com);
+MODULE_DESCRIPTION(OMAP PRM driver);
+MODULE_LICENSE(GPL);
-- 
1.7.4.1


Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. 
Kotipaikka: Helsinki
 

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html