On 23/04/2019 11:00, Lokesh Vutla wrote:
> Hi Marc,

[...]

>> +/**
>> + * ti_sci_inta_set_type() - Update the trigger type of the irq.
>> + * @data:   Pointer to corresponding irq_data
>> + * @type:   Trigger type as specified by user
>> + *
>> + * Note: This updates the handle_irq callback for level msi.
>> + *
>> + * Return 0 if all went well else appropriate error.
>> + */
>> +static int ti_sci_inta_set_type(struct irq_data *data, unsigned int type)
>> +{
>> +    struct irq_desc *desc = irq_to_desc(data->irq);
>> +
>> +    /*
>> +     * .alloc default sets handle_edge_irq. But if the user specifies
>> +     * that IRQ is level MSI, then update the handle to handle_level_irq
>> +     */
>> +    if (type & IRQF_TRIGGER_HIGH)
>> +            desc->handle_irq = handle_level_irq;
>> +
>> +    return 0;
> 
> 
> Returning error value is causing request_irq to fail, so still returning 0. Do
> you suggest any other method to handle this?

But that is the very point, isn't it? If you pass the wrong triggering
type to request_irq, it *must* fail. What you should have is something like:

switch (type & IRQ_TYPE_SENSE_MASK) {
case IRQF_TRIGGER_HIGH:
        desc->handle_irq = handle_level_irq;
        return 0;
case IRQ_TYPE_EDGE_RISING:
        return 0;
default:
        return -EINVAL;
}

(adjust as necessary).

What's wrong with this?

Thanks,

        M.
-- 
Jazz is not dead. It just smells funny...

Reply via email to