[PATCH 7/7] P2020ds: add event button handler

Timur Tabi timur.tabi at gmail.com
Tue Dec 14 08:56:36 EST 2010


On Fri, Dec 3, 2010 at 6:34 AM, Li Yang <leoli at freescale.com> wrote:
> This can be used as a wakeup source for power management.

This patch doesn't actually add wake-up support.

This patch should probably be split up, since you're adding generic
functionality for the IRQ that applies to all 85xx boards, but you
only update the device tree for one board.

> +static irqreturn_t event_isr(int irq, void *dev_id)
> +{
> +
> +       printk(KERN_INFO "MPC85xxDS: Event button been pushed.\n");
> +       return IRQ_HANDLED;
> +}

Would it make sense to have this be a weak function, so that it would
be easier to implement board-specific support?

> +
> +static int __init p2020ds_ngpixis_init(void)

You're adding a function called "p2020ds_ngpixis_init" to the file
"mpc85xx_ds.c".  mpc85xx_ds.c supports more than just the P2020DS.

> +{
> +       int event_irq, ret;
> +       struct device_node *np;
> +
> +       np = of_find_compatible_node(NULL, NULL, "fsl,p2020ds-fpga");
> +       if (np) {
> +               event_irq = irq_of_parse_and_map(np, 0);
> +               ret = request_irq(event_irq, event_isr, 0, "event", NULL);

You should probably choose a less generic name than "event".

> +               if (ret)
> +                       printk(KERN_ERR "Can't request board event int\n");

Use pr_err()

-- 
Timur Tabi
Linux kernel developer at Freescale


More information about the Linuxppc-dev mailing list