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