Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <alpine.LNX.2.22.394.2002270908380.8@nippy.intranet>
 <a682c89d-baf2-3d3c-647f-a07b2a146c9f@linux-m68k.org> <alpine.LNX.2.22.394.2002261637400.8@nippy.intranet>
 <caa5686a-5be3-5848-fdee-36f54237ccb6@linux-m68k.org> <alpine.LNX.2.22.394.2002261151220.9@nippy.intranet>
 <73c3ad08-963d-fea2-91d7-b06e4ef8d3ef@linux-m68k.org> <20200227081805.GA5746@afzalpc>
In-Reply-To: <20200227081805.GA5746@afzalpc>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu, 27 Feb 2020 09:32:46 +0100
Message-ID: <CAMuHMdWVVWaoHA1Tie5APYBq3Pa3s4BAoWN1jAACAZZS65UA7w@mail.gmail.com>
Subject: Re: [PATCH v2 06/18] m68k: Replace setup_irq() by request_irq()
To:     afzal mohammed <afzal.mohd.ma@gmail.com>
Cc:     Greg Ungerer <gerg@linux-m68k.org>,
        Finn Thain <fthain@telegraphics.com.au>,
        linux-m68k <linux-m68k@lists.linux-m68k.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Afzal,

On Thu, Feb 27, 2020 at 9:18 AM afzal mohammed <afzal.mohd.ma@gmail.com> 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@linux-m68k.org

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