Re: [PATCH v2 06/18] m68k: Replace setup_irq() by request_irq()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Afzal,

On Thu, Feb 27, 2020 at 9:18 AM afzal mohammed <afzal.mohd.ma@xxxxxxxxx> wrote:
On Wed, Feb 26, 2020 at 10:42:00AM +1000, Greg Ungerer wrote:
-   setup_irq(TMR_IRQ_NUM, &m68328_timer_irq);
+   if (request_irq(TMR_IRQ_NUM, hw_tick, IRQF_TIMER, "timer", NULL))
+           pr_err("%s: request_irq() failed\n", "timer");

Why not just:

                pr_err("timer: request_irq() failed\n");

The reason to use %s was that it could be automated by cocci script &
the o/p didn't look bad. Second arg to pr_err is what cocci
presents me & there is wide variation in the name across the tree as
Finn noted.

Excerpts from v1 cover letter [1],

- setup_irq(E1,&act);
+ if (request_irq(E1,f_handler,f_flags,f_name,f_dev_id))
+       pr_err("request_irq() on %s failed\n", f_name);

[ don't get mislead by string contents used, this was for v1, just to
 show how the result was obtained. To take care of Finn's suggesstion,
 instead of modifying cocci & then making changes other changes over
 that (i could not fully automate w/ cocci, and Julia said my script
 is fine as is), it was easier to run sed over the v1  patches ]

And maybe would it be useful to print out the error return code from a
failed request_irq()?

Since most of the existing setup_irq() didn't even check & handle
error return, my first thought was just s/setup_irq/request_irq, it
was easier from scripting pointing of view. i felt uncomfortable doing
nothing in case of error. Also noted that request_irq() definition has
a "__much_check", so decided to add it.

Most (all?) of the code calling setup_irq() is very old, and most of the calls
happen very early, so any such failures are hard failures that prevent the
system from booting at all.  Hence printing a message may be futile, as it
may happen before the console has been initialized (modulo early-printk).

Just my 2 €c.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux