From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ULyWZ-0004hV-OC for mharc-grub-devel@gnu.org; Sat, 30 Mar 2013 12:20:59 -0400 Received: from eggs.gnu.org ([208.118.235.92]:55251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULyWU-0004h6-Pd for grub-devel@gnu.org; Sat, 30 Mar 2013 12:20:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ULyWQ-0006VN-J0 for grub-devel@gnu.org; Sat, 30 Mar 2013 12:20:54 -0400 Received: from mail-ee0-f45.google.com ([74.125.83.45]:50117) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULyWP-0006VB-Uj for grub-devel@gnu.org; Sat, 30 Mar 2013 12:20:50 -0400 Received: by mail-ee0-f45.google.com with SMTP id b57so583050eek.32 for ; Sat, 30 Mar 2013 09:20:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=MzgZzglTEAEHPkdSbE3gk4V8Gh7v9T9PlAomZ9Oc6WY=; b=hWyiqbj5kWXpjkMhOjcS00jN7UfDRDczPdxH0tZ9WnAzRSadI92oeyckAJqZMEe+vt 9JWTxFZFU+e8W2SxmxjWrYbGf+OvHTuNRKEC9tev8gjMJIN+eSX0Tfh/ghxP+qFKWuje QSzh/kGe6pqiDQV+n7tK6gwn/uMsSryNizvFI+N6qKQZp0uL5ruq7zRk4Ywen+ITFYNr bwbTUwtJbdwXIBdLMibrfrnQWzWLgL2s+uXScNGbWJPxpRtxXk2/+QZYlTIUs0xK8nCG 30hH08d9ndysx8Mr4sBavaILzX/xTirhU71MKW7np8n+1z5MGRoFLyEQJ+QZmUxq5/lP cUjw== X-Received: by 10.15.43.132 with SMTP id x4mr19535389eev.31.1364660449194; Sat, 30 Mar 2013 09:20:49 -0700 (PDT) Received: from [192.168.56.2] ([81.81.226.53]) by mx.google.com with ESMTPS id r4sm10711321eeo.12.2013.03.30.09.20.47 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sat, 30 Mar 2013 09:20:48 -0700 (PDT) Message-ID: <515710E6.6090500@gmail.com> Date: Sat, 30 Mar 2013 17:20:54 +0100 From: Francesco Lavra User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: [PATCH 3/7] Initial support for U-Boot platforms References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 74.125.83.45 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Mar 2013 16:20:59 -0000 Mostly cosmetic comments from my side here... On 03/24/2013 06:01 PM, Leif Lindholm wrote: [...] > === added file 'grub-core/disk/uboot/ubootdisk.c' > --- grub-core/disk/uboot/ubootdisk.c 1970-01-01 00:00:00 +0000 > +++ grub-core/disk/uboot/ubootdisk.c 2013-03-24 12:58:23 +0000 [...] > +/* > + * uboot_disk_iterate(): > + * Itarator over enumerated disk devices. s/Itarator/Iterator [...] > === added file 'grub-core/kern/uboot/init.c' > --- grub-core/kern/uboot/init.c 1970-01-01 00:00:00 +0000 > +++ grub-core/kern/uboot/init.c 2013-03-24 12:58:23 +0000 [...] > +/* > + * grub_machine_get_bootlocation(): > + * Called from kern/main.c, which expects a device name (minus parentheses) > + * and a filesystem path back, if any are known. > + * Any returned values must be pointers to dynamically allocated strings. > + */ > +void > +grub_machine_get_bootlocation (char **device, char **path) > +{ > + char *tmp; > + > + tmp = uboot_env_get ("grub_bootdev"); > + if (tmp) > + { > + *device = grub_malloc (grub_strlen (tmp) + 1); > + if (*device == NULL) > + return; > + grub_strncpy (*device, tmp, grub_strlen (tmp) + 1); Why not simply call grub_strcpy (*device, tmp)? > + } > + else > + *device = NULL; > + > + tmp = uboot_env_get ("grub_bootpath"); > + if (tmp) > + { > + *path = grub_malloc (grub_strlen (tmp) + 1); > + if (*path == NULL) > + return; > + grub_strncpy (*path, tmp, grub_strlen (tmp) + 1); Ditto, grub_strncpy is not needed. > + } > + else > + *path = NULL; > +} > + > +void > +grub_uboot_fini (void) > +{ > + grub_ubootdisk_fini (); > + grub_console_fini (); > +} > > === added file 'grub-core/kern/uboot/uboot.c' > --- grub-core/kern/uboot/uboot.c 1970-01-01 00:00:00 +0000 > +++ grub-core/kern/uboot/uboot.c 2013-03-24 12:58:23 +0000 [...] > +/* All functions below are wrappers around the uboot_syscall() function */ > + > +/* > + * int API_getc(int *c) > + */ > +int > +uboot_getc (void) The comment preceding the function does not correspond to the function prototype. This applies as well to all the subsequent functions in this file. [...] > +int > +uboot_dev_enum (void) > +{ > + int max; > + > + grub_memset (&uboot_devices, 0, sizeof (uboot_devices)); > + max = sizeof (uboot_devices) / sizeof (struct device_info); > + > + /* > + * The API_DEV_ENUM call starts a fresh enumeration when passed a > + * struct device_info with a NULL cookie, and then depends on having > + * the prevoiusly enumerated device cookie "seeded" into the target s/prevoiusly/previously. [...] > === added file 'include/grub/uboot/uboot.h' > --- include/grub/uboot/uboot.h 1970-01-01 00:00:00 +0000 > +++ include/grub/uboot/uboot.h 2013-03-24 12:58:23 +0000 > @@ -0,0 +1,150 @@ > +/* uboot.h - declare variables and functions for U-Boot support */ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2013 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see . > + */ > + > +#ifndef GRUB_UBOOT_UBOOT_HEADER > +#define GRUB_UBOOT_UBOOT_HEADER 1 > + > +#include > +#include > + > +/* Functions. */ > +void grub_uboot_mm_init (void); > +void grub_uboot_init (void); > +void grub_uboot_fini (void); > + > +void uboot_return (int) __attribute__ ((noreturn)); > + > +grub_addr_t uboot_get_real_bss_start (void); > + > +grub_err_t grub_uboot_probe_hardware (void); > + > +extern grub_addr_t EXPORT_VAR (start_of_ram); > + > +grub_uint32_t EXPORT_FUNC (uboot_get_machine_type) (void); > +grub_addr_t EXPORT_FUNC (uboot_get_boot_data) (void); > + > + > +/* > + * The U-Boot API operates through a "syscall" interface, consisting of an > + * entry point address and a set of syscall numbers. The location of this > + * entry point is described in a structure allocated on the U-Boot heap. > + * We scan through a defined region around the hint address passed to us > + * from U-Boot. > + */ > +#include > + > +#define UBOOT_API_SEARCH_LEN (3 * 1024 * 1024) > +int uboot_api_init (void); > + > +/* All functions below are wrappers around the uboot_syscall() function */ > + > +/* > + * int API_getc(int *c) > + */ > +int uboot_getc (void); Same as for kern/uboot/uboot.c: the comments preceding the functions do not correspond to the function prototypes. -- Francesco