mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Roland Hieber <rhi@pengutronix.de>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH 1/4] net/e1000: Map custom error codes to more appropriate errno values
Date: Fri, 4 Jan 2019 12:16:14 +0100	[thread overview]
Message-ID: <20190104111614.hsxnwjwu6r3xzvgf@pengutronix.de> (raw)
In-Reply-To: <CAHQ1cqF2CbQNGb=EvQSyf==kE=eSzLwR9ByBM52O8yAJZJKsbg@mail.gmail.com>

On Thu, Jan 03, 2019 at 05:47:09PM -0800, Andrey Smirnov wrote:
> That's definitely true. When writing this patch I looked for usages
> similar to what you describe in the actual code of the driver, but
> didn't see anything that should cause a problem. The driver has a
> pretty sizable set of error codes, but AFAICT none of them are really
> used as anything more than a negative number passed up the call chain.
> Grepping for "E1000_ERR_*" in drivers/net/e1000 doesn't seem to show
> any usages in any comparison statements. Finding all of the points in
> the driver where error codes cross over into generic codebase and
> doing errno re-mapping there seemed like a more invasive/complicated
> alternative, so I did go for it.
> 
> That's how my thinking went, anyway. I can re-do the patch if we
> decide that maintaining unique ID for each E1000_ERR_* code is
> desired.

Yeah, I also did a few quick greps through the code, and didn't find any
such usages. I also noticed that sometimes the e1000 driver returns
-EIO etc. directly instead of the E1000_ERR_* codes. I don't think your
patch needs a respin though, it was only a small nitpick from my side,
all of that can still be cleaned up in additional patches, if someone
wants to do it. ;-)

 - Roland

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

      reply	other threads:[~2019-01-04 11:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-13  7:03 Andrey Smirnov
2018-12-13  7:03 ` [PATCH 2/4] net/e1000: Do not discard EEPROM error code in e1000_setup_link() Andrey Smirnov
2018-12-13  7:03 ` [PATCH 3/4] net/e1000: Use dev_err to report error Andrey Smirnov
2018-12-13  7:03 ` [PATCH 4/4] net/e1000: Only read EEPROM_INIT_CONTROL2_REG if it is needed Andrey Smirnov
2018-12-17  9:42 ` [PATCH 1/4] net/e1000: Map custom error codes to more appropriate errno values Sascha Hauer
2019-01-03 15:31 ` Roland Hieber
2019-01-04  1:47   ` Andrey Smirnov
2019-01-04 11:16     ` Roland Hieber [this message]

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=20190104111614.hsxnwjwu6r3xzvgf@pengutronix.de \
    --to=rhi@pengutronix.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox