lkml.org 
[lkml]   [2021]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v1 1/2] ata: libahci_platform: Get rid of dup message when IRQ can't be retrieved
    From
    Date
    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...

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

    MBR, Sergey

    \
     
     \ /
      Last update: 2021-12-10 12:21    [W:3.249 / U:0.840 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site