[U-Boot] [PATCH fix for v2014.10 4/5] stdio: Add force parameter to stdio_deregister

[Tom Rini]

On Thu, Oct 09, 2014 at 11:03:02AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 9 October 2014 10:23, Tom Rini <trini at ti.com> wrote:
> > On Thu, Oct 09, 2014 at 05:12:03PM +0200, Marek Vasut wrote:
> >> On Thursday, October 09, 2014 at 08:18:14 AM, Simon Glass wrote:
> >> > Hi,
> >> >
> >> > On 20 September 2014 08:54, Hans de Goede <hdegoede at redhat.com> wrote:
> >> > > In some cases we really want to move forward with a deregister, add a
> >> > > force parameter to allow this, and replace the dev with a nulldev in
> >> > > this case.
> >> > >
> >> > > Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >>
> >> [...]
> >>
> >> > > diff --git a/drivers/serial/serial-uclass.c
> >> > > b/drivers/serial/serial-uclass.c index d04104e..61cbdc6 100644
> >> > > --- a/drivers/serial/serial-uclass.c
> >> > > +++ b/drivers/serial/serial-uclass.c
> >> > > @@ -197,7 +197,7 @@ static int serial_pre_remove(struct udevice *dev)
> >> > >
> >> > >  #ifdef CONFIG_SYS_STDIO_DEREGISTER
> >> > >
> >> > >         struct serial_dev_priv *upriv = dev->uclass_priv;
> >> > >
> >> > > -       if (stdio_deregister_dev(upriv->sdev))
> >> > > +       if (stdio_deregister_dev(upriv->sdev), 0)
> >> >
> >> > That bracket seems to be in a strange place.
> >>
> >> Good find, thanks! I have two questions:
> >> 1) How come I did not notice this and my build didn't spit?
> >
> > Because only sandbox uses this right now I think.  But I'm unhappy that
> > I can't seem to make repeated runs of:
> > $ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \
> > 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc'
> > $ ./tools/buildman/buildman -b master -c 1 -ve -T 1 -j 9 \
> > 'arc|blackfin|microblaze|m68k|nds32|sparc|x86|aarch64|sandbox|mips|avr32|arm|powerpc' \
> > -svel
> >
> > Show me just new problems from the last time I did it.  I must be doing
> > something wrong, Simon?
> 
> I don't really see anything obviously wrong. But I'm not sure what you
> are expecting. This is going to just build the top commit in branch
> master, and if the commit hash has changed it will remove any previous
> results for that commit before starting the build. So you will always
> get a full list of errors, not a delta from last time. If you want
> that you could add a second commit with your fixes and not adjust the
> first commit in the branch (and use -c2).

Ah, OK, this got me going towards what I wanted.  I was assuming for
some incorrect reason that with -c1 it would just implicitly compare vs
the last build it had available.  I really want -c2 as I should have a
full build from the last go-round (which is to say really, last merge).
This is what I wanted (a simplified build):
trini at bill-the-cat:~/work/u-boot/u-boot (32d0192...)$
./tools/buildman/buildman -b HEAD -c 2 -ve -T 1 -j 9 'sandbox|sparc'
-svel
boards.cfg is up to date. Nothing to do.
Summary of 2 commits for 6 boards (1 thread, 9 jobs per thread)
01: usb: kbd: Remove check for already being registered
sparc: +   grsim_leon2 gr_cpci_ax2000 gr_xc3s_1500 grsim gr_ep2s60
+(grsim_leon2,gr_cpci_ax2000,gr_xc3s_1500,grsim,gr_ep2s60)
disk/part.c: In function `get_device':
+(grsim_leon2,gr_cpci_ax2000,gr_xc3s_1500,grsim,gr_ep2s60)
disk/part.c:454: warning: 'hwpart' might be used uninitialized in
this function
02: stdio: Add force parameter to stdio_deregister
sandbox: +   sandbox
+(sandbox) drivers/serial/serial-uclass.c: In function
‘serial_pre_remove’:
+(sandbox) drivers/serial/serial-uclass.c:201:2: error: too few
arguments to function ‘stdio_deregister_dev’
+(sandbox) include/stdio_dev.h:107:5: note: declared here
+(sandbox) make[2]: *** [drivers/serial/serial-uclass.o] Error 1
+(sandbox) make[1]: *** [drivers/serial] Error 2
+(sandbox) make: *** [sub-make] Error 2	
w+(sandbox) drivers/serial/serial-uclass.c:201:39: warning:
left-hand operand of comma expression has no effect
[-Wunused-value]

And I don't care about the stuff around 01 (it was like that before), I
care about 02 which is new problems.

> If you leave off '-b master -c1' it would have about the same effect,
> assuming that you have 'master' checked out.
> 
> In the second line you are specifying -ve twice but that doesn't
> matter. Why are you changing the thread/jobs defaults? Does that speed
> it up?

We have some race condition still on very large machines that I haven't
been able to track down.  Doing it that way was less problematic I
think.

> Also you don't need the quotes and the | between archs.

Oh, interesting.

> Also note there is --step to avoid building every commit. For example,
> this will build the upstream commit and your branch's top commit
> (only) assuming that master tracks upstream/master.
> 
> buildman -b master --step 0

I think this is what I really wanted, yes.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141009/bfd496c8/attachment.pgp>