[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJYdmeOatYbZo616HZv_peyqQRa38gtF9eT483wKNkG8gfN84g@mail.gmail.com>
Date: Wed, 28 Jun 2023 10:12:22 +0200
From: Moritz Fischer <moritz.fischer.private@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Maciej Fijalkowski <maciej.fijalkowski@...el.com>, Moritz Fischer <moritzf@...gle.com>,
netdev@...r.kernel.org, pabeni@...hat.com, kuba@...nel.org,
edumazet@...gle.com, davem@...emloft.net, bryan.whitehead@...rochip.com,
UNGLinuxDriver@...rochip.com, stable@...r.kernel.org
Subject: Re: [PATCH net v3] net: lan743x: Don't sleep in atomic context
Hi Andrew,
On Tue, Jun 27, 2023 at 4:51 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > Side note would be that I don't see much value in iopoll.h's macros
> > returning
> >
> > (cond) ? 0 : -ETIMEDOUT; \
> >
> > this could be just !!cond but given the count of the callsites...probably
> > better to leave it as is.
>
> The general pattern everywhere in linux is:
>
> err = foo(bar);
> if (err)
> return err;
>
> We want functions to return meaningful error codes, otherwise the
> caller needs to figure out an error code and return it. Having iopoll
> return an error code means we have consistency. Otherwise i would
> expect some developers to decide on EIO, ETIMEDOUT, EINVAL, maybe
> ENXIO?
Can you clarify if you suggest to leave this alone as-is in patch, or
replace with something returning one of the errors above?
If the former, anything else missing in the patch?
Thanks,
Moritz
Powered by blists - more mailing lists