[U-Boot-Users] [PATCH v3 2/10] ata: add the support for SATA framework

[Wolfgang Denk]

In message <1206525185.3662.9.camel at localhost.localdomain> you wrote:
> - add the SATA framework
> - add the SATA command line
> 
> Signed-off-by: Dave Liu <daveliu at freescale.com>
> ---
...
> diff --git a/common/cmd_sata.c b/common/cmd_sata.c
> new file mode 100644
> index 0000000..9c6605a
> --- /dev/null
> +++ b/common/cmd_sata.c
...
> +extern int sata_low_level_read(int dev, u32 blknr, u32 blkcnt, void *buffer);
> +extern int sata_low_level_write(int dev, u32 blknr, u32 blkcnt, void *buffer);
> +extern int sata_low_level_init(int dev);
> +extern int sata_low_level_scan(int dev);

Never do this, prototype declararations belong to header files.

For example, this hides that you use
	int sata_low_level_read(int dev, u32 blknr, u32 blkcnt, void *buffer)
in one place and 
	u32 sata_low_level_read(int dev, u32 blknr, u32 blkcnt, void *buffer)
in another

(and yes, this needs to be cleaned up, too.

> +static ulong sata_read(int dev, ulong blknr, ulong blkcnt, void *buffer)
> +{
> +	return sata_low_level_read(dev, blknr, blkcnt, buffer);
> +}
> +
> +static ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer)
> +{
> +	return sata_low_level_write(dev, blknr, blkcnt, (void *)buffer);
> +}

It seems sata_read() and sata_write() are  just  wrappers  that  pass
arguments       unchanged      to      sata_low_level_read()      and
sata_low_level_write() - and it also  seems  that  the  *low_level*()
functions are not used anywhere else?

Then please get rid of this additional nesting.



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Buy land. They've stopped making it."                   - Mark Twain