On Tuesday 29 April 2008, Vasiliy Leoenenko wrote:
> The first patch (support of long commands):
I like the idea of splitting this patch up in two separate patches/emails. But
please provide a descriptive subject and a short description that can will be
added to the git repository as commit text for each patch. And don't forget
your Signed-off-by line.
Please take a look at other patches on the list how this should be done. Best
would be if you could use the git tools to create (and send) the patches.
Thanks.
Some further remarks below:
> ===================================================
> diff -aupNr a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> --- a/drivers/mtd/cfi_flash.c 2008-04-21 02:39:38.000000000 +0400
> +++ b/drivers/mtd/cfi_flash.c 2008-04-29 15:57:51.000000000 +0400
> @@ -298,17 +298,29 @@ static inline void flash_unmap(flash_inf
> /*-----------------------------------------------------------------------
> * make a proper sized command based on the port and chip widths
> */
> -static void flash_make_cmd (flash_info_t * info, uchar cmd, void *cmdbuf)
> +static void flash_make_cmd (flash_info_t * info, ulong cmd, void *cmdbuf)
> {
> int i;
> + int cpofft;
> uchar *cp = (uchar *) cmdbuf;
> + uchar cp_val;
>
> #if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA)
> - for (i = info->portwidth; i > 0; i--)
> + for (i = sizeof(cfiword_t); i > 0; i--)
> + {
+ for (i = sizeof(cfiword_t); i > 0; i--) {
The code down below has some coding style issues too. Please
> + cpofft=(i-1);
+ cpofft = i - 1;
The code down below has some coding style issues too. Please try to be more
careful here.
And the resulting code looks like this:
#if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA)
for (i = sizeof(cfiword_t); i > 0; i--)
{
cpofft=(i-1);
#else
for (i = 1; i <= sizeof(cfiword_t); i++)
{
cpofft=(sizeof(cfiword_t)-i);
#endif
if( cpofft%info->chipwidth >= sizeof(ulong) ||
cpofft>=info->portwidth)
cp_val = 0x00;
else
cp_val = *((uchar*)&cmd + cpofft%info->chipwidth);
cp[i-1] = cp_val;
}
Apart from the coding-style issues, this is getting quite complex and unclear.
At least to me. Are you sure that this can't be written differently to make
it easier to understand?
Thanks.
Best regards,
Stefan
=====================================================================
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: [EMAIL PROTECTED]
=====================================================================
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
U-Boot-Users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/u-boot-users
