All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerhard Sittig <gsi@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 1/7] include: move the BIT() macro into the common.h header file
Date: Mon, 17 Feb 2014 22:33:16 +0100	[thread overview]
Message-ID: <20140217213316.GA4524@book.gsilab.sittig.org> (raw)
In-Reply-To: <20140217210156.AB7BE381337@gemini.denx.de>

On Mon, Feb 17, 2014 at 22:01 +0100, Wolfgang Denk wrote:
> 
> Dear Gerhard Sittig,
> 
> In message <1392665727-5734-2-git-send-email-gsi@denx.de> you wrote:
> > several compile units and local header files re-invented the
> > BIT() macro, move BIT() and BITMASK() declarations into the
> > common.h header file and adjust call sites
> ...
> > diff --git a/include/common.h b/include/common.h
> > index 221b7768c5be..49885f3c01bf 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -967,6 +967,9 @@ static inline phys_addr_t map_to_sysmem(const void *ptr)
> >  #define ALIGN(x,a)		__ALIGN_MASK((x),(typeof(x))(a)-1)
> >  #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
> >  
> > +#define BIT(n)			(1U << (n))
> > +#define BITMASK(bits)		(BIT(bits) - 1)
> > +
> 
> I'm sorry, but these macros are ugly and make the code harder to read.
> Also, they are inherently non-portable.  I guess you are not aware
> that "bit 0" is the MSB on Power architecture, not the LSB as you
> expect in your definition.
> 
> I strongly reommend NOT to use any such macros.  Adding these to
> common.h could be considered as an invitation to use them, which is
> the opposite of what I want.
> 
> So sorry, but this is a clear NAK.

That's fine with me.  So this patch can get dropped and nothing
changes for the existing code base.

The MCS7830 driver will then need a respin (after I took in some
more feedback, of course).  There was the question of whether to
use the BIT(5) macro or re-invented (1 << 5) or 0x20 numbers.
The feedback now allows a clear answer. :)


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

  reply	other threads:[~2014-02-17 21:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-17 19:35 [U-Boot] [PATCH v2 0/7] usb: eth: introduce Moschip MCS7830 driver Gerhard Sittig
2014-02-17 19:35 ` [U-Boot] [PATCH v2 1/7] include: move the BIT() macro into the common.h header file Gerhard Sittig
2014-02-17 21:01   ` Wolfgang Denk
2014-02-17 21:33     ` Gerhard Sittig [this message]
2014-02-17 19:35 ` [U-Boot] [PATCH v2 2/7] usb: eth: don't ifdef routine declarations in usb_ether.h Gerhard Sittig
2014-02-17 22:20   ` Simon Glass
2014-02-17 19:35 ` [U-Boot] [PATCH v2 3/7] usb: eth: introduce support for Moschip USB ethernet Gerhard Sittig
2014-02-17 20:57   ` Marek Vasut
2014-02-23 20:16     ` Gerhard Sittig
2014-02-24 17:48       ` Marek Vasut
2014-02-17 21:11   ` Wolfgang Denk
2014-02-17 21:22     ` Marek Vasut
2014-02-17 22:41       ` Wolfgang Denk
2014-02-18 11:24         ` Marek Vasut
2014-02-17 19:35 ` [U-Boot] [PATCH v2 4/7] config: alpha-sort USB ethernet items for Asix and SMSC Gerhard Sittig
2014-02-17 19:35 ` [U-Boot] [PATCH v2 5/7] config: enable Moschip USB ethernet support for several boards Gerhard Sittig
2014-02-17 19:35 ` [U-Boot] [PATCH v2 6/7] config: enable USB ethernet for taskit stamp9g20 Gerhard Sittig
2014-02-17 19:35 ` [U-Boot] [PATCH v2 7/7] usb: doc: update README.usb to list all USB ethernet options Gerhard Sittig

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=20140217213316.GA4524@book.gsilab.sittig.org \
    --to=gsi@denx.de \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.