| Message ID | alpine.LFD.2.02.1102202038360.26526@bbs.intern |
|---|---|
| State | New |
| Headers | show |
Hello, No comments? Can someone commit? Thnx. Ciao, Gerhard -- http://www.wiesinger.com/ On Sun, 20 Feb 2011, Gerhard Wiesinger wrote: > Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com> > --- > hw/pcnet.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/pcnet.c b/hw/pcnet.c > index db52dc5..a6d5784 100644 > --- a/hw/pcnet.c > +++ b/hw/pcnet.c > @@ -1562,9 +1562,17 @@ void pcnet_h_reset(void *opaque) > > /* Initialize the PROM */ > > + // Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf > + // page 95 > + // bugfix under DOS with AMD netware driver: AMD PCNTNW Ethernet MLID > v3.10 (960115), network card not found > + // works well under DOS with AMD NDIS driver v2.0.1, Knoppix 6.2 ok > memcpy(s->prom, s->conf.macaddr.a, 6); > - s->prom[12] = s->prom[13] = 0x00; > - s->prom[14] = s->prom[15] = 0x57; > + s->prom[6] = s->prom[7] = 0x00; // Reserved Location: must be 00h > + s->prom[8] = 0x00; // Reserved Location: must be 00h > + s->prom[9] = 0x11; // Hardware ID: must be 11h if compatibility to AMD > drivers is desired > + s->prom[10] = s->prom[11] = 0x00; // User programmable space > + s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which > is the sum of bytes 00hâ??0Bh and bytes 0Eh and 0Fh, must therefore be > initialized with 0! > + s->prom[14] = s->prom[15] = 0x57; // Must be ASCII W (57h) if > compatibility to AMD driver software is desired > > for (i = 0,checksum = 0; i < 16; i++) > checksum += s->prom[i]; > -- > 1.7.3.4 > > Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com> > --- > hw/pcnet.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/pcnet.c b/hw/pcnet.c > index a6d5784..2acd5f9 100644 > --- a/hw/pcnet.c > +++ b/hw/pcnet.c > @@ -1571,7 +1571,7 @@ void pcnet_h_reset(void *opaque) > s->prom[8] = 0x00; // Reserved Location: must be 00h > s->prom[9] = 0x11; // Hardware ID: must be 11h if compatibility to AMD > drivers is desired > s->prom[10] = s->prom[11] = 0x00; // User programmable space > - s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which > is the sum of bytes 00hâ??0Bh and bytes 0Eh and 0Fh, must therefore be > initialized with 0! > + s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which > is the sum of bytes 00h-0Bh and bytes 0Eh and 0Fh, must therefore be > initialized with 0! > s->prom[14] = s->prom[15] = 0x57; // Must be ASCII W (57h) if > compatibility to AMD driver software is desired > > for (i = 0,checksum = 0; i < 16; i++) > -- > 1.7.3.4
On 22 February 2011 21:00, Gerhard Wiesinger <lists@wiesinger.com> wrote: > Hello, > > No comments? Can someone commit? Not my area of the code, but some general remarks: (1) your patch email seems to be in an odd format with two patches concatenated, the first of which just changes a line introduced in the first. You should fix this and resubmit as a single patch in the right format that makes the changes you want. (2) Style issues: QEMU's coding style is for C-style comments (/* .. */) not C++-style //. Also lines should be 80 characters maximum, so your end-of-line comments should probably be moved. (3) Your commit message could be better. Convention is that the summary (subject) should start with an indication of what area is being patched, eg hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers and then in the body of the commit message you have more detail. Remarks about what you're fixing and what you've tested go here, not in comments like: // bugfix under DOS with AMD netware driver: AMD PCNTNW Ethernet MLID v3.10 (960115), network card not found // works well under DOS with AMD NDIS driver v2.0.1, Knoppix 6.2 ok If you fix these cosmetic issues you'll have a patch which is easier to review and more likely to be applied. -- PMM
On Wed, 23 Feb 2011, Peter Maydell wrote: > On 22 February 2011 21:00, Gerhard Wiesinger <lists@wiesinger.com> wrote: >> Hello, > Not my area of the code, but some general remarks: > > (1) your patch email seems to be in an odd format with two patches > concatenated, the first of which just changes a line introduced in > the first. You should fix this and resubmit as a single patch in > the right format that makes the changes you want. > > (2) Style issues: QEMU's coding style is for C-style comments > (/* .. */) not C++-style //. Also lines should be 80 characters > maximum, so your end-of-line comments should probably be moved. > > (3) Your commit message could be better. Convention is that the > summary (subject) should start with an indication of what area > is being patched, eg > hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers > > and then in the body of the commit message you have more detail. > Remarks about what you're fixing and what you've tested go > here, not in comments like: > // bugfix under DOS with AMD netware driver: AMD PCNTNW Ethernet MLID > v3.10 (960115), network card not found > // works well under DOS with AMD NDIS driver v2.0.1, Knoppix 6.2 ok > > If you fix these cosmetic issues you'll have a patch which > is easier to review and more likely to be applied. I'm not a git expert. Can you explain how to merge 3 commit to one output as expected? Rest: no problem, will post it. Ciao, Gerhard -- http://www.wiesinger.com/
On 23 February 2011 18:06, Gerhard Wiesinger <lists@wiesinger.com> wrote: > I'm not a git expert. Can you explain how to merge 3 commit to one output as > expected? There are a number of ways to do it. I use stgit (http://www.procode.org/stgit/) to manage my patches, but if you don't want to use a new tool I guess the top answer here: http://stackoverflow.com/questions/616556/how-do-you-squash-commits-into-one-patch-with-git-format-patch is probably as good as anything else. -- PMM
On Wed, 23 Feb 2011, Peter Maydell wrote: > On 23 February 2011 18:06, Gerhard Wiesinger <lists@wiesinger.com> wrote: >> I'm not a git expert. Can you explain how to merge 3 commit to one output as >> expected? > > There are a number of ways to do it. I use stgit (http://www.procode.org/stgit/) > to manage my patches, but if you don't want to use a new tool I guess > the top answer here: > http://stackoverflow.com/questions/616556/how-do-you-squash-commits-into-one-patch-with-git-format-patch > is probably as good as anything else. Thanx for your help, resent the patch. For newbies for documentation how to submit a qemu patch made of several commits: # Last commit before patch git branch pcnet-amd-fix 1899e4afdc2d838be5625539df9c87cca49bdd70 git checkout pcnet-amd-fix git merge --squash master git commit -a -F - <<EOF hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers bugfix under DOS for AMD netware driver: AMD PCNTNW Ethernet MLID v3.10 (960115), network card not found bugfix works well under DOS with: 1.) AMD NDIS driver v2.0.1 2.) AMD PCNTNW Ethernet MLID v3.10 (960115) 3.) Knoppix 6.2 EOF git format-patch master git checkout master Ciao, Gerhard -- http://www.wiesinger.com/
On Wed, 23 Feb 2011, Gerhard Wiesinger wrote: > On Wed, 23 Feb 2011, Peter Maydell wrote: > >> On 23 February 2011 18:06, Gerhard Wiesinger <lists@wiesinger.com> wrote: >>> I'm not a git expert. Can you explain how to merge 3 commit to one output >>> as >>> expected? >> >> There are a number of ways to do it. I use stgit >> (http://www.procode.org/stgit/) >> to manage my patches, but if you don't want to use a new tool I guess >> the top answer here: >> http://stackoverflow.com/questions/616556/how-do-you-squash-commits-into-one-patch-with-git-format-patch >> is probably as good as anything else. > > Thanx for your help, resent the patch. For newbies for documentation how to submit a qemu patch made of several commits (updated for necessary signoff): # Last commit before patch git branch pcnet-amd-fix 1899e4afdc2d838be5625539df9c87cca49bdd70 git checkout pcnet-amd-fix git merge --squash master git commit -a -F - <<EOF hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers bugfix under DOS for AMD netware driver: AMD PCNTNW Ethernet MLID v3.10 (960115), network card not found bugfix works well under DOS with: 1.) AMD NDIS driver v2.0.1 2.) AMD PCNTNW Ethernet MLID v3.10 (960115) 3.) Knoppix 6.2 EOF # Signoff must be added git format-patch -s master git checkout master Ciao, Gerhard -- http://www.wiesinger.com/
Am 05.03.2011 um 13:48 schrieb Gerhard Wiesinger: > For newbies for documentation how to submit a qemu patch made of > several commits (updated for necessary signoff): > > # Last commit before patch > git branch pcnet-amd-fix 1899e4afdc2d838be5625539df9c87cca49bdd70 > git checkout pcnet-amd-fix # Or in short: git checkout -b pcnet-amd-fix 1899e4afdc2d838be5625539df9c87cca49bdd70 > git merge --squash master > git commit -a -F - <<EOF > hw/pcnet.c: Fix EPROM contents to suit AMD netware drivers > > bugfix under DOS for AMD netware driver: > AMD PCNTNW Ethernet MLID v3.10 (960115), network card not found > > bugfix works well under DOS with: > 1.) AMD NDIS driver v2.0.1 > 2.) AMD PCNTNW Ethernet MLID v3.10 (960115) > 3.) Knoppix 6.2 > EOF > # Signoff must be added > git format-patch -s master > git checkout master For the record, another possible way is: git rebase -i HEAD^^^ # rebase the last three commits: # edit the last two lines to start with fixup (or squash) # if you chose squash, you'll be able to edit the message immediately git commit --amend -s # to add the SoB and edit the message if you didn't before git format-patch HEAD^ Andreas
diff --git a/hw/pcnet.c b/hw/pcnet.c index db52dc5..a6d5784 100644 --- a/hw/pcnet.c +++ b/hw/pcnet.c @@ -1562,9 +1562,17 @@ void pcnet_h_reset(void *opaque) /* Initialize the PROM */ + // Datasheet: http://pdfdata.datasheetsite.com/web/24528/AM79C970A.pdf + // page 95 + // bugfix under DOS with AMD netware driver: AMD PCNTNW Ethernet MLID v3.10 (960115), network card not found + // works well under DOS with AMD NDIS driver v2.0.1, Knoppix 6.2 ok memcpy(s->prom, s->conf.macaddr.a, 6); - s->prom[12] = s->prom[13] = 0x00; - s->prom[14] = s->prom[15] = 0x57; + s->prom[6] = s->prom[7] = 0x00; // Reserved Location: must be 00h + s->prom[8] = 0x00; // Reserved Location: must be 00h + s->prom[9] = 0x11; // Hardware ID: must be 11h if compatibility to AMD drivers is desired + s->prom[10] = s->prom[11] = 0x00; // User programmable space + s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which is the sum of bytes 00hâ??0Bh and bytes 0Eh and 0Fh, must therefore be initialized with 0! + s->prom[14] = s->prom[15] = 0x57; // Must be ASCII W (57h) if compatibility to AMD driver software is desired for (i = 0,checksum = 0; i < 16; i++) checksum += s->prom[i]; -- 1.7.3.4 Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com> --- hw/pcnet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/pcnet.c b/hw/pcnet.c index a6d5784..2acd5f9 100644 --- a/hw/pcnet.c +++ b/hw/pcnet.c @@ -1571,7 +1571,7 @@ void pcnet_h_reset(void *opaque) s->prom[8] = 0x00; // Reserved Location: must be 00h s->prom[9] = 0x11; // Hardware ID: must be 11h if compatibility to AMD drivers is desired s->prom[10] = s->prom[11] = 0x00; // User programmable space - s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which is the sum of bytes 00hâ??0Bh and bytes 0Eh and 0Fh, must therefore be initialized with 0! + s->prom[12] = s->prom[13] = 0x00; // LSByte of two-byte checksum, which is the sum of bytes 00h-0Bh and bytes 0Eh and 0Fh, must therefore be initialized with 0! s->prom[14] = s->prom[15] = 0x57; // Must be ASCII W (57h) if compatibility to AMD driver software is desired for (i = 0,checksum = 0; i < 16; i++)
Signed-off-by: Gerhard Wiesinger <lists@wiesinger.com> --- hw/pcnet.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-)