> On Mon, Jul 29, 2013 at 6:24 PM, Loh Tien Hock <thloh@xxxxxxxxxx> wrote:
>> On Tue, Jul 30, 2013 at 12:11 AM, Linus Walleij
>
>>>>>> +static int altera_gpio_irq_set_type(struct irq_data *d,
>>>>>> + unsigned int type) {
>>>>>> + struct altera_gpio_chip *altera_gc =
>>>>>> +irq_data_get_irq_chip_data(d);
>>>>>> +
>>>>>> + if (type == IRQ_TYPE_NONE)
>>>>>> + return 0;
>>>>>> +
>>>>>> + if (altera_gc->level_trigger) {
>>>>>> + if (type == IRQ_TYPE_LEVEL_HIGH)
>>>>>> + return 0;
>>>>>> + } else {
>>>>>> + if (type == IRQ_TYPE_EDGE_RISING &&
>>>>>> + altera_gc->edge_type == ALTERA_IRQ_RISING)
>>>>>> + return 0;
>>>>>> + else if (type == IRQ_TYPE_EDGE_FALLING &&
>>>>>> + altera_gc->edge_type == ALTERA_IRQ_FALLING)
>>>>>> + return 0;
>>>>>> + else if (type == IRQ_TYPE_EDGE_BOTH &&
>>>>>> + altera_gc->edge_type == ALTERA_IRQ_BOTH)
>>>>>> + return 0;
>>>>>
>>>>> I don't quite realize why you need the local version of the IRQflag
>>>>> at all. Just store the standard edge indicator in an unsigned long?
>>>>
>>>> I don't understand the comment. Can you elaborate further?
>>>
>>> I misunderstood it for the above reason. But get rid of the custom
>>> ALTERA_IRQ_* defines and just use the standard definitions for
>>> IRQ_TYPE_*, the local defines does not add anything useful.
>>
>> We still need to compare the altera_gc->edge_type to the expected ones
>> (so that we don't allow incorrect configuration, eg. the GPIO is
>> configured as rising, but set_type tries to set falling).
>> So we should compare it with magic numbers, instead of custom defines
>> like the example below?
>> if (type == IRQ_TYPE_EDGE_RISING &&
>> altera_gc->edge_type == 0)
>> return 0;
>
> And why can't altera_gc->edge_type use exactly the same enumerators so it becomes:
>
> (type == IRQ_TYPE_EDGE_RISING &&
> altera_gc->edge_type == IRQ_TYPE_EDGE_RISING)
>
> ?
>
We're unable to do that because the tool that generates the dts file
had the number that way, and if we were to change to code to your
suggestion, the tool needs to be changed and it will break backward
compatibility.
>
>
> Yours,
> Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
