[U-Boot] [PATCH] [WIP] sandbox: spi/sf emulation

[Simon Glass]

Hi Mike,

On Wed, Oct 26, 2011 at 1:37 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Wed, Oct 26, 2011 at 12:26, Simon Glass wrote:
>> On Wed, Oct 26, 2011 at 3:30 AM, Mike Frysinger wrote:
>>> --- a/arch/sandbox/cpu/os.c
>>> +++ b/arch/sandbox/cpu/os.c
>>>
>>> +off_t os_lseek(int fd, off_t offset, int whence)
>>> +{
>>> +       return lseek(fd, offset, whence);
>>> +}
>>
>> How are we going to deal with calls to something like this where
>> 'whence' is defined in an OS-specific header file? I believe we need
>> to add definitions to os.h to provide this feature, and make them
>> match Posix. Is there a better way? open has a similar problem.
>
> yeah, i noticed this, but was ignoring it for the time being.  the
> only portable way i think is to have our local os.h provide arbitrary
> defines which the rest of u-boot will utilize, and then in the os_xxx
> func, we translate into the host defines.  a bit ugly, but it's the
> only way to really work since there is no guarantee that something
> like O_RDONLY will always be defined to 0.

I care less about portability to non-POSIX, others may though. Perhaps
just defining the types and #defines with a suitable prefix in the
header file, and then a compile-time assert in the C would be good
enough.

>
>>> +const char *os_getenv(const char *name)
>>> +{
>>> +       /* We can't use getenv() as u-boot provides it own */
>>
>> Ick, but I can't see an easy alternative.
>
> i ended up not needing this interface so far, so i think i'll drop it
> now that it's been archived on the mailing list.

OK

>
>>> --- a/arch/sandbox/cpu/start.c
>>> +++ b/arch/sandbox/cpu/start.c
>>>
>>> +       if (os_getopt("-h", 0) || os_getopt("--help", 0)) {
>>> +               serial_puts(usage);
>>> +               return 0;
>>> +       }
>>
>> I did actually create something to parse args and put them into a
>> 'state' structure. But it uses getopt_long(). Any reason we can't?
>
> on reason i didn't was so i wouldn't have to worry about translating
> defines/structs used in u-boot and defines/structs used by the host
> system.  i.e. the same issue we face with bits for os_{open,lseek}.

Well I don't think we should be passing any getopt structures outside
os.c or parse.c. That is the intent of my state.c - a place where the
test state / arguments is stored and can be accessed when needed
without reference to os-dependent code / structures.

>
>>> --- a/include/os.h
>>> +++ b/include/os.h
>>>
>>> +const char *os_getenv(const char *name);
>>
>> I would really like to have full comments on all header functions
>> (with @param and @return).
>
> right, that's why this is a brain dump :).  i didn't want to document
> funcs i would ultimately throw away.

Fair enough, just being picky as I like comments :-)

>
> also, this os.h header needs #ifdef protection against multiple inclusion ...
> -mike
>

Regards,
Simon