linux-kernel - Re: [PATCH] cs5520: add missing IRQ setup for the second port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200906230305.55283.bzolnier@gmail.com>
Date:	Tue, 23 Jun 2009 03:05:55 +0200
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cs5520: add missing IRQ setup for the second port

On Tuesday 23 June 2009 02:30:11 Bartlomiej Zolnierkiewicz wrote:
> On Tuesday 23 June 2009 01:31:36 David Miller wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > Date: Mon, 22 Jun 2009 13:48:02 +0200
> > 
> > > From: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > > Subject: [PATCH] cs5520: add missing IRQ setup for the second port
> > > 
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > > ---
> > > This is obviously correct regression fix.  The only problem is that
> > > it cannot be applied under the new rigid policy before somebody with
> > > the hardware verifies it.  This will only result in a needless delay
> > > in this case (IMHO a common sense works better than rigid policies).
> > 
> > Such hard rules don't apply to regression fixes.  Patch
> > applied, thanks.
> > 
> > But I certainly would have required some positive testing for the
> > commit which introduced this problem!
> 
> You can from now on.
> 
> > I've added some verbosity to the commit message, so that people can
> > track where the problem was introduced, and exactly how this problem
> > arose, like so:
> > 
> > cs5520: add missing IRQ setup for the second port
> 
> > This fixes a regression introduced by commit
> > 86ccf37c6acd74cf7e4b7751ee045de19943c5a0 ("ide: remove pciirq argument
> > from ide_pci_setup_ports()")
> > 
> > ide_pci_setup_ports() would loop over the available ports, one
> > by one, recording IRQ numbers increasingly from the one passed
> > in as "pciirq".  The conversion only assigned the initial port's
> > IRQ, 14, but left the second one not setup.
> > 
> > [ Make commit message more verbose -DaveM ]
> 
> I didn't remember which commit was it so I didn't include it in the patch
> description.  This is certainly not the commit above (if you're making such,
> changes please make sure that you actually understand the code -- you have
> it easy now as it is orders of magnitude simpler than it was few years ago).
> 
> Looking a bit more in depth it could be that it was never a regression
> and I'm no longer sure that the change is correct (sorry for that, I was
> blinded by code in pata_cs5520.c).
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > Signed-off-by: David S. Miller <davem@...emloft.net>

Please give people some time (few days) to review and comment patches before
applying them.  I don't trust my own patches too much so I'm always looking
for some peer review (usually all host driver patches go through Sergei, in
this case Frans would have probably shot the patch down)..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/