Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Andreas Färber]

Hello Maciej,

Am 10.08.2012 15:15, schrieb Maciej W. Rozycki:
>>>> Actually there were better patches for the same bug by Meador, including
>>>> git-style rather than SVN patches and adding a helper to initialize it
>>>> consistently at all call sites.
> 
>  I find quilt patches easier to manage when I need to reorder them, 
> revert, manually edit the diffs (that I routinely do), etc.  Perhaps I'm 
> just outdated, but that's the workflow I've found most efficient for me 
> while not disturbing anyone else.  I've used quilt patches with the Linux 
> kernel including upstream submission successfully as well, for many years 
> now, and I don't remember anyone complaining about their formatting.  
> Also automatic patch retriever/tester scripts that some mailing lists have 
> watching process them correctly.
> 
>  Let's therefore please focus on the technical value of these changes 
> rather than their envelope.

Both are important to those of us reviewing and working in maintenance.

http://wiki.qemu.org/Contribute/SubmitAPatch

For example, our usual convention would've been to use a "target-mips:"
rather than "MIPS:" prefix (the directory name), if you follow the list
and history. We also don't usually indent paragraphs within the commit
message, especially not with differing indentation, and our Coding Style
is without space before parenthesis. Patches that look odd may get less
review attention. Not everything is mandatory of course, but maybe you
can also see the other side of being flooded with patches.

http://gmane.org/plot-rate.php?group=gmane.comp.emulators.qemu

>>>> There's also DSP, Octeon, mips64 and signal handling patches around that
>>>> someone needs to volunteer to update, test, clean up and queue.
>>>> That a patch is on the list doesn't imply that it "just" needs to be
>>>> applied though. So please be careful which patches you ping.
>>>
>>> Yes, hence my suggestion to look for patches which got reviewed.
>>>
>>> (Although speaking as somebody who has in the past submitted patches
>>> which got neither reviewed nor rejected, I have some sympathy for the
>>> idea that if nobody among us cares enough to look at a patch at all the
>>> default should be to apply it.)
>>
>> From my memory Maciej himself retracted his patches in reaction to a
>> reply from his colleague Meador. That might not show up when looking at
>> just one unthreaded reply-less patch, so in general ack but needs to
>> look at context, too.
> 
>  I don't recollect retracting any of my patches although I'll be making 
> the adjustments previously requested and produce more.  Any patches that 
> may have overlapped with an earlier submission come from the same tree, 
> except I regenerated them and retested against the then current top of the 
> tree; I may have updated them too to address any problems spotted while 
> processing them.

OK, so you didn't retract them but Meador did comment:

> I submitted a patch to fix this issue and the FCR0 issue a few months back 
> [1].
> Andreas reviewed it, but the patch never got committed.
> 
> [1] http://patchwork.ozlabs.org/patch/144353/

They're definitely different in scope and changes, whatever process and
tools you guys use internally. And our policy is to give preference to
the earlier patch to not invite people to redo other people's patches
with different SoB (not saying you are, same company after all).

Either way, the committed patch is now missing the info that this issue
was originally Reported-by and attempted to fix by Khansa Butt, not
employed by Mentor nor using their tree.

>> Doing follow-ups based on this one or, in worst case, reverting is
>> certainly possible but the decision-making would best be done by someone
>> who actually uses mips - not that there's no users, just no volunteer
>> for a staging tree yet.
> 
>  I'm willing to provide assistance as time permits, in particular to move 
> forward with any changes I have proposed either myself or on behalf of 
> someone else, although owing to other commitments regrettably I can't 
> commit to regular testing/mainentance.

You could keep the status in MAINTAINERS as "Odd Fixes" but set up a git
branch where maintainers can pull from and that you / other contributors
can develop against.

Me, I'm still interested in modelling MIPS CPU models as proper QOM
subclasses if we can sort out the initialization and code placement
issues that Meador was poking at for FCR0 - moving code from
cpu_mips_init() into mips_cpu_initfn() and killing cpu_state_reset().

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg