Re: [PATCH] pps: clients: gpio: fix interrupt handling order in remove path

[Rodolfo Giometti]

On 27/05/25 11:11, Farber, Eliav wrote:
> > > @@ -228,6 +228,7 @@ static void pps_gpio_remove(struct platform_device *pdev)
> > >    {
> > >        struct pps_gpio_device_data *data = platform_get_drvdata(pdev);
> > >
> > > +     free_irq(data->irq, data);
> >
> > Why not just use devm_free_irq()?
>
> As far as I understand, the main purpose of devm_*() is to provide
> hands-off resource management. devm_request_irq() is intended to
> eliminate the need for explicit cleanup in the remove() function by
> automatically freeing the IRQ after remove() returns.

In linux/kernel/irq/devres.c we can read:

/**
 *      devm_free_irq - free an interrupt
 *      @dev: device to free interrupt for
 *      @irq: Interrupt line to free
 *      @dev_id: Device identity to free
 *
 *      Except for the extra @dev argument, this function takes the
 *      same arguments and performs the same function as free_irq().
 *      This function instead of free_irq() should be used to manually
 *      free IRQs allocated with devm_request_irq().
 */

> In my opinion, calling devm_free_irq() undermines the benefit of using
> devm_request_irq() in the first place. If I need to explicitly free the
> IRQ during remove(), then I’m no longer relying on devm’s automatic
> cleanup - I’m effectively reverting to manual resource management while
> still using devm-style registration, which I find unnecessary.
>
> That said, if you still favor devm_free_irq(), I’ll revise the patch
> accordingly.

Since devm_free_irq() works exactly as free_irq() and can be used to manually 
free IRQs allocated with devm_request_irq(), I think it is less invasive. Isn't 
it? :-)

Ciao,

Rodolfo

--
GNU/Linux Solutions                  e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver                          giometti@xxxxxxxx
Embedded Systems                     phone:  +39 349 2432127
UNIX programming