Re: [Qemu-devel] [PATCH 06/17] eepro100: symbolic names for pci registers

[Michael S. Tsirkin]

On Thu, Jan 07, 2010 at 12:14:17PM +0100, Stefan Weil wrote:
> Michael S. Tsirkin schrieb:
> > No functional changes. I verified that the generated binary
> > does not change in meaningful ways. Survived light usage
> > with linux guest.
> >
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> > hw/eepro100.c | 49 ++++++++++++++++++++++++++++++++-----------------
> > 1 files changed, 32 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/eepro100.c b/hw/eepro100.c
> > index 2a9e3b5..82e3766 100644
> > --- a/hw/eepro100.c
> > +++ b/hw/eepro100.c
> > @@ -412,19 +412,24 @@ static void pci_reset(EEPRO100State * s)
> > pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
> > /* PCI Device ID depends on device and is set below. */
> > /* PCI Command */
> > + /* TODO: this is the default, do not override. */
> > PCI_CONFIG_16(PCI_COMMAND, 0x0000);
> > /* PCI Status */
> > - PCI_CONFIG_16(PCI_STATUS, 0x2800);
> > + /* TODO: this seems to make no sense. */
> > + /* TODO: Value at RST# should be 0. */
> > + PCI_CONFIG_16(PCI_STATUS,
> > + PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_SIG_TARGET_ABORT);
> > /* PCI Revision ID */
> Hi,
> 
> this PCI status value is wrong. The correct value for PCI_STATUS is 0x0280
> and was fixed in the maintainer version in 2007:
> http://repo.or.cz/w/qemu/ar7.git/commitdiff/9da3830d81948cc1f666fcf562699f165b029a79
> 
> It was also fixed in a patch sent to qemu-devel (which was never applied):
> http://patchwork.ozlabs.org/patch/33962/
> 
> I'll send a new patch which fixes the wrong status value.
> 
> Antony, how can we speed up the synchronisation process for
> eepro100.c? Today, patches get lost without feedback.
> This is no wonder because you have to work on hundreds of patches.
> It was suggested that I should send patch series.
> My last patch serie with 3 patches is ready for integration
> since 2009-12-20.

A simple solution that worked for me is to publish a git tree and send
pull requests (in addition to individual patches).  This guarantees that
nothing is lost and makes it easy for Anthony to apply a set of changes
in one go. git merge also has smarter heuristics than git am.

I put the patch you just posted on my tree as it's PCI related and I
understand it, but if you create your own tree and want me to drop this
patch from my tree, pls let me know and I will.

> Paul, if I had commit rights, I could integrate the missing
> parts myself and maintain eepro100.c in the future. Of course
> I'd send the single changes to the list before comitting them.
> Don't you think that would be the best solution for all of us?
> 
> Regards,
> Stefan

Personally, I find it comforting that someone else pulls my tree instead
of commit rights: this ensures at least build testing is done before
my changes are inflicted on others :)

-- 
MST