[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130415233151.GA30171@electric-eye.fr.zoreil.com>
Date: Tue, 16 Apr 2013 01:31:51 +0200
From: Francois Romieu <romieu@...zoreil.com>
To: Dmitry Kravkov <dmitry@...adcom.com>
Cc: "davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Eilon Greenstein <eilong@...adcom.com>
Subject: Re: [PATCH net-next 1/4] bnx2x: refactor nvram read procedure
Dmitry Kravkov <dmitry@...adcom.com> :
[...]
> > You memcpy a u32 to an array of bytes instead of copying it byte after
> > byte with proper shift operators and now you are paving the way for more
> > endianess headaches. I'd rather avoid the memcpy when readying data for
> > ethtool in the first place.
> In case of _be data I don't see any issue with copying _be32 it into byte array.
The modified code (bnx2x_nvram_read) will not be copying __be32 but u32.
It issmuggling _be data behind neutral u32 and using casting when the
kernel has provided cpu_to_{le / be} helpers for years. Think of type
checking as kind of messed up as soon as u32 *ret_val contains a _le or
_be data depending on the value of bool to_be.
I don't want to worry about the endianness of a u32. If it's a u32 (u16,
s32, whatever), I only want to think of bytes in it through '>>' or '<<'
operators. No need to remember the semantic of the last bnx2x_nvram_read_dword
parameter, if it's internal, external, nor assume that the lower layers
enforce some ability to memcpy it blindly. So I'd just favor using cpu order
as soon and as much as possible.
Of course it's your driver, whence your maintenance choices.
--
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists