All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Sergey Shtylyov <s.shtylyov@omp.ru>
Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hans de Goede <hdegoede@redhat.com>, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH v1 1/2] ata: libahci_platform: Get rid of dup message when IRQ can't be retrieved
Date: Fri, 10 Dec 2021 13:36:38 +0200	[thread overview]
Message-ID: <YbM7xkTazM76CVvD@smile.fi.intel.com> (raw)
In-Reply-To: <9e6b2e9a-e958-0c14-6570-135607041978@omp.ru>

On Fri, Dec 10, 2021 at 02:19:52PM +0300, Sergey Shtylyov wrote:
> On 12/10/21 1:46 PM, Andy Shevchenko wrote:
> 
> >>>> platform_get_irq() will print a message when it fails.
> >>>> No need to repeat this.
> >>>>
> >>>> While at it, drop redundant check for 0 as platform_get_irq() spills
> >>>> out a big WARN() in such case.
> >>>
> >>> The reason you should be able to remove the "if (!irq)" test is that
> >>> platform_get_irq() never returns 0. At least, that is what the function kdoc
> >>> says. But looking at platform_get_irq_optional(), which is called by
> >>> platform_get_irq(), the out label is:
> >>>
> >>> 	WARN(ret == 0, "0 is an invalid IRQ number\n");
> >>> 	return ret;
> >>>
> >>> So 0 will be returned as-is. That is rather weird. That should be fixed to
> >>> return -ENXIO:
> >>>
> >>> 	if (WARN(ret == 0, "0 is an invalid IRQ number\n"))
> >>> 		return -ENXIO;
> >>> 	return ret;
> >>
> >>    My unmerged patch (https://marc.info/?l=linux-kernel&m=163623041902285) does this
> >> but returns -EINVAL instead.
> >>
> >>> Otherwise, I do not think that removing the "if (!irq)" hunk is safe. no ?
> >>
> >>    Of course it isn't...
> > 
> > It's unsubstantiated statement. The vIRQ 0 shouldn't be returned by any of
> > those API calls.
> 
>    We do _not_ know what needs to be fixed, that's the problem, and that's why the WARN()
> is there...

So, have you seen this warning (being reported) related to libahci_platform?
If no, what we are discussing about then? The workaround is redundant and
no need to have a dead code in the driver, really.

> > If it is the case, go and fix them, no need to workaround
> > in each of the callers.
> 
>    There's a need to work around as long as IRQ0 ican be returned, otherwise
>    we get partly functioning or non-functioning drivers...

You get them unfunctioning anyways and you get the big WARN() even before this
patch.


-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2021-12-10 11:37 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 14:59 [PATCH v1 1/2] ata: libahci_platform: Get rid of dup message when IRQ can't be retrieved Andy Shevchenko
2021-12-09 14:59 ` [PATCH v1 2/2] ata: libahci_platform: Remove bogus 32-bit DMA mask attempt Andy Shevchenko
2021-12-10 19:34   ` Andy Shevchenko
2021-12-11  0:04     ` Damien Le Moal
2021-12-17  0:58   ` Damien Le Moal
2021-12-17 11:57     ` Andy Shevchenko
2021-12-09 15:21 ` [PATCH v1 1/2] ata: libahci_platform: Get rid of dup message when IRQ can't be retrieved Hans de Goede
2021-12-09 17:24 ` Sergey Shtylyov
2021-12-09 17:42   ` Andy Shevchenko
2021-12-09 18:22     ` Sergey Shtylyov
2021-12-09 19:22       ` Andy Shevchenko
2021-12-09 19:27         ` Andy Shevchenko
2021-12-09 20:31           ` Sergey Shtylyov
2021-12-09 20:29         ` Sergey Shtylyov
2021-12-10 10:44           ` Andy Shevchenko
2021-12-10 11:14             ` Sergey Shtylyov
2021-12-10 11:28               ` Andy Shevchenko
2021-12-10 17:39                 ` Sergey Shtylyov
2021-12-10 17:51                   ` Andy Shevchenko
2021-12-09 22:49 ` Damien Le Moal
2021-12-09 22:57   ` Damien Le Moal
2021-12-10  8:47   ` Andy Shevchenko
2021-12-10 16:38     ` Sergey Shtylyov
2021-12-10 17:57       ` Andy Shevchenko
2021-12-10  8:59   ` Sergey Shtylyov
2021-12-10 10:46     ` Andy Shevchenko
2021-12-10 11:19       ` Sergey Shtylyov
2021-12-10 11:36         ` Andy Shevchenko [this message]
2021-12-10 17:15           ` Sergei Shtylyov
2021-12-10 17:59             ` Andy Shevchenko
2021-12-10 19:01               ` Sergey Shtylyov
2021-12-10 19:25                 ` Andy Shevchenko
2021-12-10 19:30                   ` Sergey Shtylyov
2021-12-10 19:35                     ` Sergei Shtylyov
2021-12-11 10:13                       ` Sergey Shtylyov
2021-12-13 11:26                         ` Andy Shevchenko
2021-12-10 23:45     ` Damien Le Moal
2021-12-11 10:25       ` Sergey Shtylyov
2021-12-12 22:39         ` Damien Le Moal
2021-12-13 11:52           ` Andy Shevchenko
2021-12-13 21:36             ` Damien Le Moal
2021-12-13 22:02               ` Andy Shevchenko
2021-12-13 11:49         ` Andy Shevchenko
2021-12-13 11:46       ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YbM7xkTazM76CVvD@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=axboe@kernel.dk \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.shtylyov@omp.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.