Sharp power management: split into sharpsl-dependend and generic parts

Hi!

This splits sharpsl_pm.c into sharpsl_pm.c and sharp_pm.c. sharpsl_pm.c contains stuff that is shared between spitz and corgi, sharp_pm.c contains more widely usable code. I'd like something like this to be eventually merged... [Of course, I'll cleanup #ifdef COLLIE's, I did not realize some were still pending.]

Comments?

@@ -33,6 +33,7 @@ static void corgi_charger_init(void)
 	pxa_gpio_mode(CORGI_GPIO_CHRG_ON | GPIO_OUT);
 	pxa_gpio_mode(CORGI_GPIO_CHRG_UKN | GPIO_OUT);
 	pxa_gpio_mode(CORGI_GPIO_KEY_INT | GPIO_IN);
+	sharpsl_arch_pm_init();
 }
 
 static void corgi_charge_led(int val)

@@ -179,6 +180,10 @@ static int corgi_acin_status(void)
 
 static struct sharpsl_charger_machinfo corgi_pm_machinfo = {
 	.init            = corgi_charger_init,
+	.remove		 = sharpsl_arch_pm_remove,
+	.read_main_battery= sharpsl_read_main_battery,
+	.read_temp	  = sharpsl_read_temp,
+	.read_acin	  = sharpsl_read_acin,
 	.gpio_batlock    = CORGI_GPIO_BAT_COVER,
 	.gpio_acin       = CORGI_GPIO_AC_IN,
 	.gpio_batfull    = CORGI_GPIO_CHRG_FULL,

@@ -2,6 +2,8 @@
  * SharpSL SSP Driver
  */
 
+#include <linux/interrupt.h>
+
 struct corgissp_machinfo {
 	int port;
 	int cs_lcdcon;

@@ -51,8 +53,12 @@ struct sharpsl_charger_machinfo {
 	void (*measure_temp)(int);
 	void (*presuspend)(void);
 	void (*postsuspend)(void);
+	int (*read_main_battery)(void);
+	int (*read_temp)(void);
+	int (*read_acin)(void);
 	unsigned long (*charger_wakeup)(void);
 	int (*should_wakeup)(unsigned int resume_on_alarm);
+	int (*remove)(void);
 	int bat_levels;
 	struct battery_thresh *bat_levels_noac;
 	struct battery_thresh *bat_levels_acin;

@@ -119,3 +125,14 @@ extern struct battery_thresh spitz_batte
 #define STATUS_BATT_LOCKED()  READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batlock)
 #define STATUS_CHRG_FULL()  READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_batfull)
 #define STATUS_FATAL()      READ_GPIO_BIT(sharpsl_pm.machinfo->gpio_fatal)
+
+extern irqreturn_t sharpsl_ac_isr(int irq, void *dev_id, struct pt_regs *fp);
+extern irqreturn_t sharpsl_fatal_isr(int irq, void *dev_id, struct pt_regs *fp);
+extern irqreturn_t sharpsl_chrg_full_isr(int irq, void *dev_id, struct pt_regs *fp);
+
+extern int sharpsl_read_main_battery(void);
+extern int sharpsl_read_temp(void);
+extern int sharpsl_read_acin(void);
+
+extern int sharpsl_arch_pm_init(void);
+extern int sharpsl_arch_pm_remove(void);

@@ -33,6 +33,7 @@ static void spitz_charger_init(void)
 {
 	pxa_gpio_mode(SPITZ_GPIO_KEY_INT | GPIO_IN);
 	pxa_gpio_mode(SPITZ_GPIO_SYNC | GPIO_IN);
+	sharpsl_arch_pm_init();
 }
 
 static void spitz_charge_led(int val)

@@ -182,6 +183,10 @@ static int spitz_acin_status(void)
 
 struct sharpsl_charger_machinfo spitz_pm_machinfo = {
 	.init             = spitz_charger_init,
+	.remove		  = sharpsl_arch_pm_remove,
+	.read_main_battery= sharpsl_read_main_battery,
+	.read_temp	  = sharpsl_read_temp,
+	.read_acin	  = sharpsl_read_acin,
 	.gpio_batlock     = SPITZ_GPIO_BAT_COVER,
 	.gpio_acin        = SPITZ_GPIO_AC_IN,
 	.gpio_batfull     = SPITZ_GPIO_CHRG_FULL,

@@ -11,8 +11,8 @@
 obj-$(CONFIG_ARCH_LUBBOCK) += lubbock.o
 obj-$(CONFIG_MACH_MAINSTONE) += mainstone.o
 obj-$(CONFIG_ARCH_PXA_IDP) += idp.o
-obj-$(CONFIG_PXA_SHARP_C7xx)	+= corgi.o corgi_ssp.o corgi_lcd.o sharpsl_pm.o corgi_pm.o
-obj-$(CONFIG_PXA_SHARP_Cxx00)	+= spitz.o corgi_ssp.o corgi_lcd.o sharpsl_pm.o spitz_pm.o
+obj-$(CONFIG_PXA_SHARP_C7xx)	+= corgi.o corgi_ssp.o corgi_lcd.o sharp_pm.o sharpsl_pm.o corgi_pm.o
+obj-$(CONFIG_PXA_SHARP_Cxx00)	+= spitz.o corgi_ssp.o corgi_lcd.o sharp_pm.o sharpsl_pm.o spitz_pm.o
 obj-$(CONFIG_MACH_AKITA)	+= akita-ioexp.o
 obj-$(CONFIG_MACH_POODLE)	+= poodle.o
 obj-$(CONFIG_MACH_TOSA)         += tosa.o

Thanks, Sharp!

The patch makes it very difficult to see what you've changed. The other ifdef COLLIEs look fairly straight forward to clean up and once that's done I'm ok with the principle of it.

I don't think the filenames are quite right as collie is still a Sharp SL series device. How about leaving the common code in sharpsl_pm.c and adding sharpsl_pm_pxa.c for the corgi/spitz common code? That should make the diffs easier to read. Eventually, sharpsl_pm.c should probably end up in arm/common but for now, I'd like to see easy to read diffs.

Thinking further about the structure, I have another idea which might be worth thinking about. If the common sharpsl_pm code was moved to arm/common, the files mach-sa1100/sharpsl_pm.c and mach-pxa/sharpsl_pm.c could provide the mach dependent functions Only one of the two would ever get compiled into a kernel so the files can provide the same function namespace and avoid any ugly ifdefs or tables of function pointers.

Out of interest, does the code work on collie yet?

Richard

Hi!

Yep, sorry about that. Best way to see what I've done is to apply the patch, then diff between old sharpsl_pm.c and new sharp_pm.c.

Okay, yes, that's a plan. Works for me.

I'd say we want to keep those function pointers. We may want to support single kernel running on multiple machines. Its probably not going to go as far as multiboot on pxa and sa1100, but you have function pointers there, already, and it seems cleaner to have single mechanism for handling indirection.

Somehow... it is able to read battery status from time to time, and it detects ac plug/unplug. I do not yet dare to enable real charging.

Pavel

Thanks, Sharp!

As discussed, I've made a version of this available as:

http://www.rpsys.net/openzaurus/patches/sharpsl_pm_move-r0.patch

This:

* moves the common code into arm/common
* adds a sharpsl_pm.h header file (in asm/hardware)
* adds appropriate Kconfig entries for this
* removes all the "nasty" macros
* refactors temperature measurement slightly
* removes most of the led code. I have some led patches which apply after the patch above to implement that differently
* combined the status and measurement code into a single machine dependent function.

I know that the pxa_pm_prepare/enter/finish functions are going to be a problem on collie but will await the rest of Pavel's findings before trying to work out the best way to handle it.

I've asked the person working on tosa (SL-C6000) to investigate what changes he needs to the code to enable tosa to work within this framework as well.

Richard

Hi!

Looks good to me, thanks.
Pavel

Thanks, Sharp!