[PATCH 02/11] Drop FIQ dependency for GTA01 configuration.

pHilipp Zabel philipp.zabel at gmail.com
Thu Oct 2 19:11:23 CEST 2008


Hi Jonas,

On Wed, Oct 1, 2008 at 11:30 AM, Jonas Bonn <jonas.bonn at gmail.com> wrote:
> When the config option MACH_NEO1973_GTA02 is not set, then we do not need to
> have any access to the FIQ symbols.
> ---
>  drivers/leds/Kconfig                 |    1 -
>  drivers/leds/leds-neo1973-vibrator.c |   10 ++++++++++
>  2 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index b3c1c7b..78640dd 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -166,7 +166,6 @@ config LEDS_PCA955X
>  config LEDS_NEO1973_VIBRATOR
>        tristate "Vibrator Support for the FIC Neo1973 GSM phone"
>        depends on LEDS_CLASS && MACH_NEO1973
> -       select S3C2440_C_FIQ

Isn't this select needed on GTA02? I don't know if maybe something like

-       select S3C2440_C_FIQ
+       select S3C2440_C_FIQ if MACH_NEO1973_GTA02

would be better?

>        help
>          This option enables support for the vibrator on the FIC Neo1973.
>
> diff --git a/drivers/leds/leds-neo1973-vibrator.c b/drivers/leds/leds-neo1973-vibrator.c
> index 2b50504..1c4db5d 100644
> --- a/drivers/leds/leds-neo1973-vibrator.c
> +++ b/drivers/leds/leds-neo1973-vibrator.c
> @@ -23,7 +23,9 @@
>  #include <asm/arch/gta01.h>
>  #include <asm/plat-s3c/regs-timer.h>
>
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>  #include <asm/arch-s3c2410/fiq_ipc_gta02.h>
> +#endif
>  #include <asm/plat-s3c24xx/neo1973.h>
>
>  #define COUNTER 64
> @@ -42,11 +44,13 @@ static void neo1973_vib_vib_set(struct led_classdev *led_cdev,
>        struct neo1973_vib_priv *vp =
>                container_of(led_cdev, struct neo1973_vib_priv, cdev);
>
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>        if (machine_is_neo1973_gta02()) { /* use FIQ to control GPIO */
>                fiq_ipc.vib_pwm = value; /* set it for FIQ */
>                fiq_kick(); /* start up FIQs if not already going */
>                return;
>        }
> +#endif

I don't think this is a good idea. If machine_is_neo1973_gta02()
returns false, this code will be compiled out anyway. So you are
adding redundancy to gain what? Faster compile time?
The same comment applies to all the other places where you wrap
conditional blocks like this.

>        /*
>         * value == 255 -> 99% duty cycle (full power)
>         * value == 128 -> 50% duty cycle (medium power)
> @@ -131,6 +135,7 @@ static int __init neo1973_vib_probe(struct platform_device *pdev)
>        neo1973_vib_led.gpio = r->start;
>        platform_set_drvdata(pdev, &neo1973_vib_led);
>
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>        if (machine_is_neo1973_gta02()) { /* use FIQ to control GPIO */
>                neo1973_gpb_setpin(neo1973_vib_led.gpio, 0); /* off */
>                s3c2410_gpio_cfgpin(neo1973_vib_led.gpio, S3C2410_GPIO_OUTPUT);
> @@ -139,6 +144,7 @@ static int __init neo1973_vib_probe(struct platform_device *pdev)
>                fiq_ipc.vib_pwm = 0; /* off */
>                goto configured;
>        }
> +#endif
>
>        /* TOUT3 */
>        if (neo1973_vib_led.gpio == S3C2410_GPB3) {
> @@ -150,7 +156,9 @@ static int __init neo1973_vib_probe(struct platform_device *pdev)
>                s3c2410_gpio_cfgpin(neo1973_vib_led.gpio, S3C2410_GPB3_TOUT3);
>                neo1973_vib_led.has_pwm = 1;
>        }
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>  configured:
> +#endif
>        mutex_init(&neo1973_vib_led.mutex);
>
>        return led_classdev_register(&pdev->dev, &neo1973_vib_led.cdev);
> @@ -158,9 +166,11 @@ configured:
>
>  static int neo1973_vib_remove(struct platform_device *pdev)
>  {
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>        if (machine_is_neo1973_gta02()) /* use FIQ to control GPIO */
>                fiq_ipc.vib_pwm = 0; /* off */
>        /* would only need kick if already off so no kick needed */
> +#endif
>
>        if (neo1973_vib_led.has_pwm)
>                s3c2410_pwm_disable(&neo1973_vib_led.pwm);
> --
> 1.5.4.3
>
>
>

regards
Philipp



More information about the openmoko-kernel mailing list