From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Thu, 06 Sep 2012 12:15:42 +0200
Subject: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with
 ones defined at <compiler.h>
In-Reply-To: <5047ABE4.9050609@wwwdotorg.org>
References: <1345795995-24656-1-git-send-email-l.majewski@samsung.com>
	<1345795995-24656-3-git-send-email-l.majewski@samsung.com>
	<5047ABE4.9050609@wwwdotorg.org>
Message-ID: <20120906121542.4e05b1b9@amdc308.digital.local>
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

Hi Stephen,

> On 08/24/2012 02:13 AM, Lukasz Majewski wrote:
> > Custom definitions of le_XX_to_int functions have been replaced with
> > standard ones, defined at <compiler.h>
> 
> > diff --git a/disk/part_efi.c b/disk/part_efi.c
> 
> > -/* Convert char[8] in little endian format to the host format
> > integer
> > - */
> > -static inline unsigned long long le64_to_int(unsigned char *le64)
> > -{
> 
> So this original function takes a pointer to the value ...
> 
> >  	/* Check the first_usable_lba and last_usable_lba are
> > within the disk. */ lastlba = (unsigned long long)dev_desc->lba;
> > -	if (le64_to_int(pgpt_head->first_usable_lba) > lastlba) {
> > +	if (le64_to_cpu(pgpt_head->first_usable_lba) > lastlba) {
> 
> At this point in the series, first_usable_lba is a char[8], so this is
> passing the address of the first byte to both the original function
> le64_to_int(), and the replacement function le64_to_cpu(). However,
> le64_to_cpu() expects to receive the 64-bit value to swap, not a
> pointer to it - from compiler.h:
> 
> #define _uswap_64(x, sfx) \
>         ((((x) & 0xff00000000000000##sfx) >> 56) | \
> ...
> # define uswap_64(x) _uswap_64(x, ull)
> 
> le:
> # define le64_to_cpu(x)         (x)
> be:
> # define le64_to_cpu(x)         uswap_64(x)
> 
> So I think this patch breaks the code, and then the next patch fixes
> it, since it changes the type of first_usable_lba from char[8] to
> __le64?

First of all, thanks for very thorough review. 

You are right, my fault. Patch 2/6 and 3/6 shall be squashed together.
When squashed, no errors and warnings appear.

I've unnecessary separated them to show this two step change. For the
sake of bisecting - those shall be squashed. 


-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group