[PATCH] make neo1973_pm_gps.c suck less
Vladimir Koutny
vlado at moko.ksp.sk
Mon Jan 19 10:05:25 CET 2009
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Werner,
Werner Almesberger wrote:
> Here's a bunch of changes to neo1973_pm_gps.c. The thing I set out to
> change was just the initial power state, but there's so much wrong in
> there, I couldn't resist slaying some evil on the way:
any reason my 2 patches (or at least the first one which fixes a power-on
resume bug) didn't get included yet? (see your/om-kernel mailbox around Jan5;
probably won't apply directly after your patch but are rather trivial anyway)
Vladimir
>
> - update the copyright year
> - show slightly more confidence in operator precedence
> - simplify #ifdef rat's nest a little
> - rename "pwron" sysfs file to "power_on", to conform to all the rest
> - wrap lines longer than 80 columns
> - use PTR_ERR, not cast
> - reduce redundancy and increase readability in code patching
> gta01_gps_sysfs_entries
> - on GTA02 and if booted by u-boot, GPS is powered on. Power it off,
> like on GTA01 or with Qi.
>
> It's still nasty, but slightly less painful to look at now. Tested on
> GTA02 and tested that it still compiles for GTA01.
>
> The name change from "pwron" to "power_on" may be controversial. If
> this causes undue misery for distribution makers, please holler.
>
> Signed-off-by: Werner Almesberger <werner at openmoko.org>
>
> ---
>
> Index: ktrack/arch/arm/plat-s3c24xx/neo1973_pm_gps.c
> ===================================================================
> --- ktrack.orig/arch/arm/plat-s3c24xx/neo1973_pm_gps.c 2009-01-17 12:12:44.000000000 -0200
> +++ ktrack/arch/arm/plat-s3c24xx/neo1973_pm_gps.c 2009-01-17 12:28:54.000000000 -0200
> @@ -1,7 +1,7 @@
> /*
> * GPS Power Management code for the FIC Neo1973 GSM Phone
> *
> - * (C) 2007 by Openmoko Inc.
> + * (C) 2007-2009 by Openmoko Inc.
> * Author: Harald Welte <laforge at openmoko.org>
> * All rights reserved.
> *
> @@ -29,6 +29,7 @@
>
> /* For GTA02 */
> #include <mach/gta02.h>
> +#include <linux/mfd/pcf50633/core.h>
>
> #include <linux/regulator/consumer.h>
>
> @@ -294,10 +295,10 @@
> /* don't let RX from unpowered GPS float */
> s3c2410_gpio_pullup(S3C2410_GPH5, 1);
> }
> - if ((on) && (!neo1973_gps.power_was_on))
> + if (on && !neo1973_gps.power_was_on)
> regulator_enable(neo1973_gps.regulator);
>
> - if ((!on) && (neo1973_gps.power_was_on))
> + if (!on && neo1973_gps.power_was_on)
> regulator_disable(neo1973_gps.regulator);
> }
>
> @@ -325,10 +326,7 @@
> {
> int ret = 0;
>
> - if (!strcmp(attr->attr.name, "pwron"))
> -#ifdef CONFIG_MACH_NEO1973_GTA01
> - {
> -#endif
> + if (!strcmp(attr->attr.name, "power_on")) {
> ret = gps_pwron_get();
> #ifdef CONFIG_MACH_NEO1973_GTA01
> } else if (!strcmp(attr->attr.name, "power_avdd_3v")) {
> @@ -344,8 +342,8 @@
> } else if (!strcmp(attr->attr.name, "power_core_1v5") ||
> !strcmp(attr->attr.name, "power_vdd_core_1v5")) {
> ret = gps_power_1v5_get();
> - }
> #endif
> + }
> if (ret)
> return strlcpy(buf, "1\n", 3);
> else
> @@ -358,10 +356,7 @@
> {
> unsigned long on = simple_strtoul(buf, NULL, 10);
>
> - if (!strcmp(attr->attr.name, "pwron"))
> -#ifdef CONFIG_MACH_NEO1973_GTA01
> -{
> -#endif
> + if (!strcmp(attr->attr.name, "power_on")) {
> gps_pwron_set(on);
> #ifdef CONFIG_MACH_NEO1973_GTA01
> } else if (!strcmp(attr->attr.name, "power_avdd_3v")) {
> @@ -377,8 +372,8 @@
> } else if (!strcmp(attr->attr.name, "power_core_1v5") ||
> !strcmp(attr->attr.name, "power_vdd_core_1v5")) {
> gps_power_1v5_set(on);
> - }
> #endif
> + }
> return count;
> }
>
> @@ -543,11 +538,11 @@
> #define gta01_pm_gps_resume NULL
> #endif
>
> -static DEVICE_ATTR(pwron, 0644, power_gps_read, power_gps_write);
> +static DEVICE_ATTR(power_on, 0644, power_gps_read, power_gps_write);
>
>
> static struct attribute *gta01_gps_sysfs_entries[] = {
> - &dev_attr_pwron.attr,
> + &dev_attr_power_on.attr,
> #ifdef CONFIG_MACH_NEO1973_GTA01
> &dev_attr_power_avdd_3v.attr,
> &dev_attr_reset.attr,
> @@ -566,7 +561,7 @@
> };
>
> static struct attribute *gta02_gps_sysfs_entries[] = {
> - &dev_attr_pwron.attr,
> + &dev_attr_power_on.attr,
> NULL
> };
>
> @@ -584,16 +579,21 @@
> case GTA01v3_SYSTEM_REV:
> break;
> case GTA01v4_SYSTEM_REV:
> - s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_RESET, S3C2410_GPIO_OUTPUT);
> + s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_RESET,
> + S3C2410_GPIO_OUTPUT);
> break;
> case GTA01Bv3_SYSTEM_REV:
> case GTA01Bv4_SYSTEM_REV:
> - s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_3V3, S3C2410_GPIO_OUTPUT);
> + s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_3V3,
> + S3C2410_GPIO_OUTPUT);
> /* fallthrough */
> case GTA01Bv2_SYSTEM_REV:
> - s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_2V8, S3C2410_GPIO_OUTPUT);
> - s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_3V, S3C2410_GPIO_OUTPUT);
> - s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_RESET, S3C2410_GPIO_OUTPUT);
> + s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_2V8,
> + S3C2410_GPIO_OUTPUT);
> + s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_EN_3V,
> + S3C2410_GPIO_OUTPUT);
> + s3c2410_gpio_cfgpin(GTA01_GPIO_GPS_RESET,
> + S3C2410_GPIO_OUTPUT);
> break;
> default:
> dev_warn(&pdev->dev, "Unknown GTA01 Revision 0x%x, "
> @@ -607,22 +607,24 @@
> gps_power_sequence_down();
>
> switch (system_rev) {
> + int entries = ARRAY_SIZE(gta01_gps_sysfs_entries);
> case GTA01v3_SYSTEM_REV:
> case GTA01v4_SYSTEM_REV:
> case GTA01Bv2_SYSTEM_REV:
> - gta01_gps_sysfs_entries[ARRAY_SIZE(gta01_gps_sysfs_entries)-3] =
> - &dev_attr_power_tcxo_2v8.attr;
> + gta01_gps_sysfs_entries[entries-3] =
> + &dev_attr_power_tcxo_2v8.attr;
> break;
> case GTA01Bv3_SYSTEM_REV:
> case GTA01Bv4_SYSTEM_REV:
> - gta01_gps_sysfs_entries[ARRAY_SIZE(gta01_gps_sysfs_entries)-3] =
> - &dev_attr_power_core_1v5.attr;
> - gta01_gps_sysfs_entries[ARRAY_SIZE(gta01_gps_sysfs_entries)-2] =
> - &dev_attr_power_vdd_core_1v5.attr;
> + gta01_gps_sysfs_entries[entries-3] =
> + &dev_attr_power_core_1v5.attr;
> + gta01_gps_sysfs_entries[entries-2] =
> + &dev_attr_power_vdd_core_1v5.attr;
> break;
> }
> #endif
> - return sysfs_create_group(&pdev->dev.kobj, >a01_gps_attr_group);
> + return sysfs_create_group(&pdev->dev.kobj,
> + >a01_gps_attr_group);
> }
>
> if (machine_is_neo1973_gta02()) {
> @@ -635,9 +637,9 @@
> neo1973_gps.regulator = regulator_get(
> &pdev->dev, "RF_3V");
> if (IS_ERR(neo1973_gps.regulator)) {
> - dev_err(&pdev->dev, "probe failed %d\n",
> - (int)neo1973_gps.regulator);
> - return (int)neo1973_gps.regulator;
> + dev_err(&pdev->dev, "probe failed %ld\n",
> + PTR_ERR(neo1973_gps.regulator));
> + return PTR_ERR(neo1973_gps.regulator);
> }
>
> dev_info(&pdev->dev, "FIC Neo1973 GPS Power Management:"
> @@ -648,9 +650,21 @@
> "AGPS PM features not available!!!\n",
> system_rev);
> return -1;
> - break;
> }
> - return sysfs_create_group(&pdev->dev.kobj, >a02_gps_attr_group);
> +
> + /*
> + * u-boot enables LDO5 (GPS), which doesn't make sense and
> + * causes confusion. We therefore disable the regulator here.
> + *
> + * We don't do this through the regulator API because we'd have
> + * to second-guess some of its internal logic and make it do
> + * something that isn't really part of its design.
> + */
> + pcf50633_reg_write(gta02_pcf_pdata.pcf,
> + PCF50633_REG_LDO5ENA, 0);
> +
> + return sysfs_create_group(&pdev->dev.kobj,
> + >a02_gps_attr_group);
> }
> return -1;
> }
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iJwEAQECAAYFAkl0QlUACgkQutgEj9ZLuzSGvgP/Yn0x1jEgxIjV8Zps0G6Y4hx+
+5W0SaYwlbezdDvCLwRzaC6saQq8ODdu50hhzziewNGVApP0EiFnalieoo/8R9EQ
endrY/TC+he2SZIxCvu5IoLDZhZ1KW6SyC/VwwZAsmOF1jnIq55FW747PQHUr5GX
vpOwHia2WBcaOqeEKIg=
=wip7
-----END PGP SIGNATURE-----
More information about the openmoko-kernel
mailing list