[LinuxBIOS] r418 - in LinuxBIOSv3: arch/x86 device include include/arch/x86 include/arch/x86/arch include/device lib util/dtc

[Stefan Reinauer]

* Uwe Hermann <uwe at hermann-uwe.de> [070630 21:18]:
> On Fri, Jun 29, 2007 at 06:57:23PM +0200, svn at openbios.org wrote:
> > * start using arch/foo.h again instead of archfoo.h (trivial)
> > * make constructor an initializer.
> 
> I don't understand. What's an initializer in this context?

It does not allocate the data structure anymore since this has been done
already. Before any device with a constructor would actually create two
struct device  entries

> > +		printk(BIOS_INFO, "No constructor called for %s.\n", 
> > +			dev_id_string(&c->id));
> 
> BIOS_DEBUG?
 
Yeah. there are some other too verbose printks in the device code. 
 
> > +#ifndef ARCH_ELF_H
> > +#define ARCH_ELF_H
> 
> Following our current guidelines this should be ARCH_X86_ARCH_ELF_H,
> which looks a bit stupid, but I think it's necessary.
> 
> ARCH_ELF_H doesn't work as we'd have another ARCH_ELF_H if we add
> another architecture...
 
I think this is dangerous, because you would accidently be able to
include both the ppc and the x86 version of the file, which you dont
want.

> > + * Copyright (C) 2001 Linux Networx
> > + *
> > + * This program 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 2 of the License, or
> > + * (at your option) any later version.
> 
> This should be GPLv2, I think. We know from email conversation with
> Eric Biederman / Linux Networx that their code is GPLv2 only.
 
Ok. I took the file from v2. Should we change it? Can you make a patch

> But this file is based on Linux code anyway, thus definately GPLv2,
> I'll update the header and HACKING accordingly (later).
 

great!!

> > +typedef struct {
> > +	volatile unsigned int lock;
 
> Why volatile? I think the Linux file doesn't have that.
 
Then Linux is broken. The CPU is not supposed to cache this value under
any circumstances, hence volatile. If CPU1 writes it, CPU0 must not
assume the last value it wrote to it.

> > +} spinlock_t;
> 
> Drop spinlock_t and use 'struct spinlock' as we do for other structs?
 
I really hate this dropping of the _t types, but since we kind of agreed
on that... I guess you are right.
 
> > +#define barrier() __asm__ __volatile__("": : :"memory")
> 
> #define barrier() (__asm__ __volatile__("": : :"memory"))
> 
> (better safe than sorry)

this will break code using it. try barrier(); 


> > +#define spin_is_locked(x)	(*(volatile char *)(&(x)->lock) <= 0)
> > +#define spin_unlock_wait(x)	do { barrier(); } while(spin_is_locked(x))
> 
> #define spin_unlock_wait(x) (do { barrier(); } while(spin_is_locked(x)))
 


same thing. these are functions, not expressions.

> > +
> > +#define spin_lock_string \
> > +	"\n1:\t" \
> > +	"lock ; decb %0\n\t" \
> > +	"js 2f\n" \
> > +	".section .text.lock,\"ax\"\n" \
> > +	"2:\t" \
> > +	"cmpb $0,%0\n\t" \
> > +	"rep;nop\n\t" \
> > +	"jle 2b\n\t" \
> > +	"jmp 1b\n" \
> > +	".previous"
> > +
> > +/*
> > + * This works. Despite all the confusion.
> > + */
> > +#define spin_unlock_string \
> > +	"movb $1,%0"
> 
> Shall we drop these #defines? They're only used in one place and a small
> comment which says what the asm does should be enough.
 
Hm. possibly. Anyone has a comment?

> >  				/* convention: first property is labeled with path */
> > -				$6->label = strdup($3.val);
> > +				$6->label = strdup((char *)$3.val);
> 
> Is this the correct fix for the warning? I didn't notice this warning
> before so it must have been introduced by some recent change?

Maybe it is due to the compiler I am using..? 

It is the correct fix though, strdup expects a signed char.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866