From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Tue, 18 Feb 2014 12:24:06 +0100
Subject: [U-Boot] [PATCH v2 3/7] usb: eth: introduce support for Moschip
	USB ethernet
In-Reply-To: <20140217224124.29B4A381337@gemini.denx.de>
References: <1392665727-5734-1-git-send-email-gsi@denx.de>
	<201402172222.38911.marex@denx.de>
	<20140217224124.29B4A381337@gemini.denx.de>
Message-ID: <201402181224.06226.marex@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hello Wolfgang,

> Dear Marek,
> 
> In message <201402172222.38911.marex@denx.de> you wrote:
> > > > +#define PHY_CMD1_READ		BIT(6)
> > > > +#define PHY_CMD1_WRITE		BIT(5)
> > > > +#define PHY_CMD1_PHYADDR	BIT(0)
> > > > +
> > > > +#define PHY_CMD2_PEND		BIT(7)
> > > > +#define PHY_CMD2_READY		BIT(6)
> > > 
> > > ...
> > > 
> > > As mentioned in patch # 1, I object against the use of these
> > > obfuscating BIT() macros.  Please do not use these; use plain
> > > readable code, that leaves no ambiguities to the reader.
> > 
> > Just to chime in real quick, Linux uses these 'BIT()' macros, but I
> > personally have no hard feelings about them either way.
> 
> Yes, certain developers have been using this style before.  This does
> not make it any better.  Fact is, that I have no way to tell what the
> code means.  BIT(0) can be expected to have any of the following
> meanings: 0x01, 0x80, 0x8000, 0x80000000, ... So I always have to look
> up the macro definition first, before I can unerstand it.  And then I
> have to compute in my head what the number actually means.
> 
> Compare: BIT(6) or 0x40 - what is easier to write, to read, and to
> understand?

(1 << 6) is easier for me to read honestly, because then I can quickly 
crosscheck it with the datasheet.

> You dump a register - either with the BDI, or with some printf().
> You get 0x27051956.  Is BIT(17) set?  Is bit 0x00020000 set?
> Which of these questions is easier to answer (even when you are sure
> that this is on a system where bit no. 0 is the LSB)?

You have a point with the endianness here.

Best regards,
Marek Vasut