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

[Timur Tabi]

On Mon, Dec 13, 2010 at 10:24 PM, Li Yang <leoli at freescale.com> wrote:

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

Oh, right.  I should have realized that.

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

Doesn't the device tree take care of that?  If the node exists, the
IRQ number is specified.  Otherwise, the feature is not enabled.

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

Hmmm.... I guess technically it is, but I wonder if it should be.

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

Well, either it does or it doesn't, but you can't add a p2020-specific
function to mpc85xx_ds.c.  You should just rename the function,
because I see no reason why it can't work on other DS boards.

The problem is the compatible string.  Each pixis implementation is
different, but they share several common traits.  I wonder if we need
to have a generic compatible string for the FPGA node.

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

I was thinking something like "fsl-fpga-event".

-- 
Timur Tabi
Linux kernel developer at Freescale