From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Fri, 20 May 2011 10:37:05 +0200
Subject: [U-Boot] [PATCH v4 1/4] MX5: Make the weim structure complete
In-Reply-To: <1305833248-30716-1-git-send-email-fabio.estevam@freescale.com>
References: <1305833248-30716-1-git-send-email-fabio.estevam@freescale.com>
Message-ID: <4DD62831.3090108@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

On 05/19/2011 09:27 PM, Fabio Estevam wrote:
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---

Hi Fabio,


> + * WEIM CSnGCR1
> + */
> +#define CSEN(x)	(x)
> +#define SWR(x)		((x) << 1)
> +#define SRD(x)		((x) << 2)
> +#define MUM(x)		((x) << 3)
> +#define WFL(x)		((x) << 4)

Probably the macros for single bits can take confusion. Why do we need a
parameter ?
I am expecting that CSEN(1) sets the bit and CSEN(0) resets the bit, and
this is obviously not the case, because we use them in or-wise or to
mask some bits. For single bits there is no test about the parameter and
I could use CSEN(7), and that is completely wrong.

Is it not better to remove the parameter for single bits ? If we define
for example

	SRD	(1 << 2)

it is always clear what we want to do and there is no way to generate
confusion. And no need to check the parameter.

Best regards,
Stefano Babic

-- 
=====================================================================
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
=====================================================================