Re: [Cbe-oss-dev] [patch 02/02] powerpc/cell: add support for power button of future IBM cell blades

[Benjamin Herrenschmidt]

On Mon, 2008-07-07 at 18:56 +0200, Christian Krafft wrote:
> From: Christian Krafft <[EMAIL PROTECTED]>
> 
> This patch adds support for the power button on future IBM cell blades.
> It actually doesn't shut down the machine. Instead it exposes an
> input device /dev/input/event0 to userspace which sends KEY_POWER
> if power button has been pressed.
> haldaemon actually recognizes the button, so a plattform independent acpid
> replacement should handle it correctly.
> 
> Signed-off-by: Christian Krafft <[EMAIL PROTECTED]>

Sorry Christian, i'm still not too happy with this one.

There are two issues at hand here:

 - The use of SELECT, that will be frowned on unfortunately.

 - I'm not too sure it's very safe the way you do it. If I understand
correctly, you can get called for that sysreset at -any- time, including
when interrupts are off right ?

That means potentially, code that has interrupts off will be interrupted
by input_report/input_sync, which is really bad (may corrupt the input
layer internal list management for example).

You could solve both things with a little trick: Have the platform code
just basically set a global flag when the button was pressed and have a
module that depends on INPUT & INPUT_DEV poll for it (slowly pls) and do
the input report.

Ben.

> Index: linux.git/arch/powerpc/platforms/cell/Kconfig
> ===================================================================
> --- linux.git.orig/arch/powerpc/platforms/cell/Kconfig
> +++ linux.git/arch/powerpc/platforms/cell/Kconfig
> @@ -87,9 +87,12 @@ config PPC_IBM_CELL_BLADE_BUTTONS
>       bool "IBM Cell Blade Buttons"
>       depends on CBE_RAS && PPC_IBM_CELL_BLADE
>       default y
> +     select INPUT
> +     select INPUT_EVDEV
>       help
>         Support Buttons on IBM Cell blades. This adds a method to
> -       trigger system reset via front panel pinhole button.
> +       trigger system reset via front panel pinhole button and
> +       an input device for the power button.
>  
>  config CBE_THERM
>       tristate "CBE thermal support"
> Index: linux.git/arch/powerpc/platforms/cell/ras.c
> ===================================================================
> --- linux.git.orig/arch/powerpc/platforms/cell/ras.c
> +++ linux.git/arch/powerpc/platforms/cell/ras.c
> @@ -14,6 +14,11 @@
>  #include <linux/smp.h>
>  #include <linux/reboot.h>
>  
> +#ifdef CONFIG_PPC_IBM_CELL_BLADE_BUTTONS
> +#include <linux/input.h>
> +#include <linux/platform_device.h>
> +#endif /* CONFIG_PPC_IBM_CELL_BLADE_BUTTONS */
> +
>  #include <asm/reg.h>
>  #include <asm/io.h>
>  #include <asm/prom.h>
> @@ -232,31 +237,76 @@ static struct notifier_block cbe_ptcal_r
>  
>  #ifdef CONFIG_PPC_IBM_CELL_BLADE_BUTTONS
>  static int sysreset_hack;
> +static struct input_dev *button_dev;
> +static struct platform_device *button_pdev;
>  
>  static int __init cbe_sysreset_init(void)
>  {
> +     int ret = 0;
> +     struct input_dev *dev;
>       struct cbe_pmd_regs __iomem *regs;
>  
>       sysreset_hack = machine_is_compatible("IBM,CBPLUS-1.0");
>       if (!sysreset_hack)
> -             return 0;
> +             goto out;
>  
>       regs = cbe_get_cpu_pmd_regs(0);
>       if (!regs)
> -             return 0;
> +             goto out;
>  
>       /* Enable JTAG system-reset hack */
>       out_be32(&regs->fir_mode_reg,
>               in_be32(&regs->fir_mode_reg) |
>               CBE_PMD_FIR_MODE_M8);
>  
> -     return 0;
> +     dev = input_allocate_device();
> +     if (!dev) {
> +             ret = -ENOMEM;
> +             printk(KERN_ERR "%s: Not enough memory\n", __func__);
> +             goto out;
> +     }
> +
> +     set_bit(EV_KEY, dev->evbit);
> +     set_bit(KEY_POWER, dev->keybit);
> +
> +     dev->name = "Power Button";
> +     dev->id.bustype = BUS_HOST;
> +
> +     /* this makes the button look like an acpi power button
> +      * no clue whether anyone relies on that though */
> +     dev->id.product = 0x02;
> +     dev->phys = "LNXPWRBN/button/input0";
> +
> +     button_pdev = platform_device_register_simple("power_button", 0, NULL, 
> 0);
> +     if (IS_ERR(button_pdev)) {
> +             ret = PTR_ERR(button_pdev);
> +             goto out_free_input;
> +     }
> +
> +     dev->dev.parent = &button_pdev->dev;
> +     ret = input_register_device(dev);
> +
> +     if (ret) {
> +             printk(KERN_ERR "%s: Failed to register device\n", __func__);
> +             goto out_free_pdev;
> +     }
> +
> +     button_dev = dev;
> +     goto out;
> +
> +out_free_pdev:
> +     platform_device_unregister(button_pdev);
> +out_free_input:
> +     input_free_device(dev);
> +out:
> +     return ret;
>  }
>  device_initcall(cbe_sysreset_init);
>  
>  int cbe_sysreset_hack(void)
>  {
>       struct cbe_pmd_regs __iomem *regs;
> +     u64 status;
>  
>       /*
>        * The BMC can inject user triggered system reset exceptions,
> @@ -267,10 +317,20 @@ int cbe_sysreset_hack(void)
>               regs = cbe_get_cpu_pmd_regs(0);
>               if (!regs)
>                       return 0;
> -             if (in_be64(&regs->ras_esc_0) & 0x0000ffff) {
> +             status = in_be64(&regs->ras_esc_0);
> +             if (status & 0x0000ffff) {
>                       out_be64(&regs->ras_esc_0, 0);
>                       return 0;
>               }
> +             if (status & 0x00010000) {
> +                     out_be64(&regs->ras_esc_0, 0);
> +                     if (!button_dev)
> +                             return 0;
> +                     input_report_key(button_dev, KEY_POWER, 1);
> +                     input_sync(button_dev);
> +                     input_report_key(button_dev, KEY_POWER, 0);
> +                     input_sync(button_dev);
> +             }
>       }
>       return 1;
>  }
> 

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev