arcmsr changelog differs from diffs

arcmsr changelog differs from diffs

[Hans-Peter Jansen]

Hi Jeff,

while browsing through Linus' current check ins, I stumbled upon:

[SCSI] arcmsr: irq handler fixes, cleanups, micro-opts:

--8<--
488a5c8a9a3b67ae117784cd0d73bef53a73d57d
 drivers/scsi/arcmsr/arcmsr_hba.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c 
b/drivers/scsi/arcmsr/arcmsr_hba.c
index 7832a10..f4d2d52 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -422,7 +422,7 @@ static int arcmsr_probe(struct pci_dev *pdev,
 		goto out_release_regions;
 
 	error = request_irq(pdev->irq, arcmsr_do_interrupt,
-				IRQF_SHARED, "arcmsr", acb);
+			    IRQF_SHARED, "arcmsr", acb);
 	if (error)
 		goto out_free_ccb_pool;
 
-->8--

and: [SCSI] arcmsr: Fix hardware wait loops

--8<--
24430458bb924e371ff894e26bfa9f73707f53fb
 drivers/scsi/arcmsr/arcmsr_hba.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c 
b/drivers/scsi/arcmsr/arcmsr_hba.c
index 50e1310..7832a10 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2092,8 +2092,10 @@ static void arcmsr_iop_reset(struct 
AdapterControlBlock *acb)
 	if (atomic_read(&acb->ccboutstandingcount) != 0) {
 		/* talk to iop 331 outstanding command aborted */
 		arcmsr_abort_allcmd(acb);
+
 		/* wait for 3 sec for all command aborted*/
 		ssleep(3);
+
 		/* disable all outbound interrupt */
 		intmask_org = arcmsr_disable_outbound_ints(acb);
 		/* clear all outbound posted Q */
-->8--

where both changelogs differ significantly from the actual diffs, which both 
are simple WS fixups and nothing else. Does qgit fools me here, or is 
anything else wrong on my side?

Pete

Re: arcmsr changelog differs from diffs

[Björn Steinbrink]

On 2007.10.17 00:07:19 +0200, Hans-Peter Jansen wrote:
> Hi Jeff,
> 
> while browsing through Linus' current check ins, I stumbled upon:
> 
> [SCSI] arcmsr: irq handler fixes, cleanups, micro-opts:
> 
> --8<--
> 488a5c8a9a3b67ae117784cd0d73bef53a73d57d
>  drivers/scsi/arcmsr/arcmsr_hba.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c 
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 7832a10..f4d2d52 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -422,7 +422,7 @@ static int arcmsr_probe(struct pci_dev *pdev,
>  		goto out_release_regions;
>  
>  	error = request_irq(pdev->irq, arcmsr_do_interrupt,
> -				IRQF_SHARED, "arcmsr", acb);
> +			    IRQF_SHARED, "arcmsr", acb);
>  	if (error)
>  		goto out_free_ccb_pool;
>  
> -->8--
> 
> and: [SCSI] arcmsr: Fix hardware wait loops
> 
> --8<--
> 24430458bb924e371ff894e26bfa9f73707f53fb
>  drivers/scsi/arcmsr/arcmsr_hba.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c 
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 50e1310..7832a10 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2092,8 +2092,10 @@ static void arcmsr_iop_reset(struct 
> AdapterControlBlock *acb)
>  	if (atomic_read(&acb->ccboutstandingcount) != 0) {
>  		/* talk to iop 331 outstanding command aborted */
>  		arcmsr_abort_allcmd(acb);
> +
>  		/* wait for 3 sec for all command aborted*/
>  		ssleep(3);
> +
>  		/* disable all outbound interrupt */
>  		intmask_org = arcmsr_disable_outbound_ints(acb);
>  		/* clear all outbound posted Q */
> -->8--
> 
> where both changelogs differ significantly from the actual diffs, which both 
> are simple WS fixups and nothing else. Does qgit fools me here, or is 
> anything else wrong on my side?

Nothing wrong on your side. I took a look at the second one, and
everything but the whitespace changes already found its way into Linus'
tree via 1a4f550a09f89e3a15eff1971bc9db977571b9f6. One hunk of the
original patch[1] was actually made redundant because the code was
removed in that commit, so that's probably what James fixed (see full
commit message).

And then, if I may guess, James probably just noticed that there were
changes left and commited them (while they were now down to just the
whitespace change), without checking what changes were actually left (no
offense intended). At least I think that git wouldn't cripple the diff
if the changes that James checked in were not already whitespace-only at
the time he commited them, and the git history of his tree seems to
agree.

Probably the other commit is similar.

Björn

[1] http://linux.derkeiler.com/Mailing-Lists/Kernel/2007-07/msg11957.html

Re: arcmsr changelog differs from diffs

[Hans-Peter Jansen]

Dear Björn,

Am Mittwoch, 17. Oktober 2007 02:53 schrieb Björn Steinbrink:
> On 2007.10.17 00:07:19 +0200, Hans-Peter Jansen wrote:
> > Hi Jeff,
> >
> > while browsing through Linus' current check ins, I stumbled upon:
> >
> > [SCSI] arcmsr: irq handler fixes, cleanups, micro-opts:
> >
> > --8<--
> > 488a5c8a9a3b67ae117784cd0d73bef53a73d57d
> >  drivers/scsi/arcmsr/arcmsr_hba.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > index 7832a10..f4d2d52 100644
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> > @@ -422,7 +422,7 @@ static int arcmsr_probe(struct pci_dev *pdev,
> >  		goto out_release_regions;
> >
> >  	error = request_irq(pdev->irq, arcmsr_do_interrupt,
> > -				IRQF_SHARED, "arcmsr", acb);
> > +			    IRQF_SHARED, "arcmsr", acb);
> >  	if (error)
> >  		goto out_free_ccb_pool;
> >
> > -->8--
> >
> > and: [SCSI] arcmsr: Fix hardware wait loops
> >
> > --8<--
> > 24430458bb924e371ff894e26bfa9f73707f53fb
> >  drivers/scsi/arcmsr/arcmsr_hba.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > index 50e1310..7832a10 100644
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> > @@ -2092,8 +2092,10 @@ static void arcmsr_iop_reset(struct
> > AdapterControlBlock *acb)
> >  	if (atomic_read(&acb->ccboutstandingcount) != 0) {
> >  		/* talk to iop 331 outstanding command aborted */
> >  		arcmsr_abort_allcmd(acb);
> > +
> >  		/* wait for 3 sec for all command aborted*/
> >  		ssleep(3);
> > +
> >  		/* disable all outbound interrupt */
> >  		intmask_org = arcmsr_disable_outbound_ints(acb);
> >  		/* clear all outbound posted Q */
> > -->8--
> >
> > where both changelogs differ significantly from the actual diffs, which
> > both are simple WS fixups and nothing else. Does qgit fools me here, or
> > is anything else wrong on my side?
>
> Nothing wrong on your side. I took a look at the second one, and
> everything but the whitespace changes already found its way into Linus'
> tree via 1a4f550a09f89e3a15eff1971bc9db977571b9f6. One hunk of the
> original patch[1] was actually made redundant because the code was
> removed in that commit, so that's probably what James fixed (see full
> commit message).

Yup, I found the patch you mentioned also, but that time orginating from 
Nick Cheng (*).

That diff is unfortunately way too big for me to be able to decide, if it's 
worth it to replace a former instance of arcmsr-1.20.0.13 for a 2.6.18.8 
kernel, which I use and essentially depend on here.

Thus I decided now to bluntly cc Nick and ask directly this way:
Nick, does 1.20.0.15 provide any essential changes over 1.20.0.13 for a 
ARC-1130 with FW 1.41 (2006-05-24), which advise updating?

> And then, if I may guess, James probably just noticed that there were
> changes left and commited them (while they were now down to just the
> whitespace change), without checking what changes were actually left (no
> offense intended). At least I think that git wouldn't cripple the diff
> if the changes that James checked in were not already whitespace-only at
> the time he commited them, and the git history of his tree seems to
> agree.

(*) Thanks for the clarification, Björn. What made me stumbling was the 
different orginators, while Jeff isn't known with catching attention by 
assimilating other peoples contributions. 

> Probably the other commit is similar.
>
> Björn
>
> [1] http://linux.derkeiler.com/Mailing-Lists/Kernel/2007-07/msg11957.html

Pete

Re: arcmsr changelog differs from diffs

[Björn Steinbrink]

Hi Hans,

On 2007.10.17 12:41:11 +0200, Hans-Peter Jansen wrote:
> Dear Björn,
> 
> Am Mittwoch, 17. Oktober 2007 02:53 schrieb Björn Steinbrink:
> > And then, if I may guess, James probably just noticed that there were
> > changes left and commited them (while they were now down to just the
> > whitespace change), without checking what changes were actually left (no
> > offense intended). At least I think that git wouldn't cripple the diff
> > if the changes that James checked in were not already whitespace-only at
> > the time he commited them, and the git history of his tree seems to
> > agree.
> 
> (*) Thanks for the clarification, Björn. What made me stumbling was the 
> different orginators, while Jeff isn't known with catching attention by 
> assimilating other peoples contributions. 

I'm just guessing here, and certainly don't want to blame anyone, but
Jeff's original patch was from July, while the only date I can find for
Nick's commit is September 13th. So it was probably Nick who integrated
Jeff's changes into his commit (maybe thinking that Jeff's patch got
lost), not Jeff assimilating a subset of Nick's changes.

Björn

Re: arcmsr changelog differs from diffs

[Jeff Garzik]

Björn Steinbrink wrote:
> Hi Hans,
> 
> On 2007.10.17 12:41:11 +0200, Hans-Peter Jansen wrote:
>> Dear Björn,
>>
>> Am Mittwoch, 17. Oktober 2007 02:53 schrieb Björn Steinbrink:
>>> And then, if I may guess, James probably just noticed that there were
>>> changes left and commited them (while they were now down to just the
>>> whitespace change), without checking what changes were actually left (no
>>> offense intended). At least I think that git wouldn't cripple the diff
>>> if the changes that James checked in were not already whitespace-only at
>>> the time he commited them, and the git history of his tree seems to
>>> agree.
>> (*) Thanks for the clarification, Björn. What made me stumbling was the 
>> different orginators, while Jeff isn't known with catching attention by 
>> assimilating other peoples contributions. 
> 
> I'm just guessing here, and certainly don't want to blame anyone, but
> Jeff's original patch was from July, while the only date I can find for
> Nick's commit is September 13th. So it was probably Nick who integrated
> Jeff's changes into his commit 

Correct.

It's also quite normal for the maintainer to grab a patch off the list, 
integrate/massage/change it, and then apply it himself.

	Jeff