Hi Sjoerd,
On 28 February 2015 at 07:12, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Fri, 2015-02-20 at 12:23 -0700, Simon Glass wrote:
Hi Sjoerd,
On 19 February 2015 at 15:41, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
Properly map memory through map_sysmem so that pxe can be used from the sandbox.
Signed-off-by: Sjoerd Simons sjoerd.simons@collabora.co.uk
Please run your patches through patman as you seem to have style violations. Also can you add some notes about how you have tested this on real hardware?
Will do for the next round together with addressing your comments. One confused me tough, see below.
common/cmd_pxe.c | 80 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 46 insertions(+), 34 deletions(-)
@@ -1281,13 +1288,14 @@ static int parse_label(char **c, struct pxe_menu *cfg)
- Returns 1 on success, < 0 on error.
*/ -static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, struct pxe_menu *cfg, int nest_level) +static int parse_pxefile_top(cmd_tbl_t *cmdtp, char *p, unsigned long b,
struct pxe_menu *cfg, int nest_level)s/b/base/ or something more meaningful (fix above/below also)
{ struct token t;
char *s, *b, *label_name;
char *s, *label_name, *base; int err;
b = p;
base = p;This worries me - assigning a pointer to a ulong.
base is a pointer here though. So this comment confuses me.
Actually it confused me. I don't think it's good to use base as a pointer or a ulong in the same file. Maybe use base for the ulong and base_ptr for the pointer. Or base_addr and base. But code is harder to read if different functions in the file have different conventions.
Regards, Simon