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.
- Implement write_env/read_env/env_relocate_spec/saveenv/sata_get_env_dev
- 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