[U-Boot] [PATCH] common: env: support sata device

[Peng Fan]

Hi Tom,

Thanks for reviewing.

On Mon, Mar 28, 2016 at 11:15:43AM -0400, Tom Rini wrote:
>On Mon, Mar 28, 2016 at 05:37:16PM +0800, Peng Fan wrote:
>
>> Introduce env support for sata device.
>> 1. Implement write_env/read_env/env_relocate_spec/saveenv/sata_get_env_dev
>> 2. If want to enable this feature, define CONFIG_ENV_IS_IN_SATA, and
>>    define CONFIG_SYS_SATA_ENV_DEV or implement your own sata_get_ev_dev.
>[snip]
>> +/*
>> + * TODO: Support CONFIG_ENV_SIZE_REDUND CONFIG_ENV_OFFSET_REDUND
>> + */
>
>I'd like to see #error here as well, having been bit in the past in
>cases of "Oh, redund doesn't work?".

Will fix in V2.

>
>> +#if !defined(CONFIG_ENV_OFFSET) || !defined(CONFIG_ENV_SIZE)
>> +#error CONFIG_ENV_OFFSET or CONFIG_ENV_SIZE not defined
>> +#endif
>> +
>> +char *env_name_spec = "SATA";
>> +
>> +#ifdef ENV_IS_EMBEDDED
>> +env_t *env_ptr = (env_t *)(&environment[0]);
>
>So, you're not adding (and I'm not suggesting you do) SATA to the list
>of environment types where we support embedded env.  Please just remove
>these tests here and elsewhere.  Thanks!

Fix in V2.

>
>[snip]
>> +static inline int read_env(struct blk_desc *sata, unsigned long size,
>> +			   unsigned long offset, const void *buffer)
>> +{
>> +	uint blk_start, blk_cnt, n;
>> +
>> +	blk_start = ALIGN(offset, sata->blksz) / sata->blksz;
>> +	blk_cnt   = ALIGN(size, sata->blksz) / sata->blksz;
>> +
>> +	n = sata->block_read(sata, blk_start, blk_cnt, (uchar *)buffer);
>
>Here and elsewhere, please update to use blk_dread, etc, from blk.h to
>help future proof this code, thanks!

Fix in V2.

Thanks,
Peng.
>
>-- 
>Tom