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

[Li Yang]

On Tue, Dec 14, 2010 at 5:56 AM, Timur Tabi <timur.tabi at gmail.com> wrote:
> 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.

Any enabled IRQ is a valid wake-up source for standby.  The patch
enables a board specific interrupt for the purpose of wakeup.

>
> 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.

The IRQ is a board specific one from GPIO which not applicable on all
85xx boards.

>
>> +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?

It's already a board-specific one.

>
>> +
>> +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.

I'm not sure if other DS boards covered by this file has the same functionality.

>
>> +{
>> +       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".

Well, it's the name suggested by the board manual.  We may change it
to "event_button" if not too long.

- Leo