-stable review patch. If anyone has any objections, please let us know.
------------------
This is one heck of a confused driver. It uses a byte write to a dword
register to enable a ROM resource that it doesn't even seem to be using.
"Lost and wandering in the desert of confusion"
Signed-off-by: Linus Torvalds <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
drivers/ide/pci/hpt366.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
Index: linux-2.6.13.y/drivers/ide/pci/hpt366.c
===================================================================
--- linux-2.6.13.y.orig/drivers/ide/pci/hpt366.c
+++ linux-2.6.13.y/drivers/ide/pci/hpt366.c
@@ -1334,9 +1334,13 @@ static int __devinit init_hpt366(struct
static unsigned int __devinit init_chipset_hpt366(struct pci_dev *dev, const char *name)
{
int ret = 0;
- /* FIXME: Not portable */
+
+ /*
+ * FIXME: Not portable. Also, why do we enable the ROM in the first place?
+ * We don't seem to be using it.
+ */
if (dev->resource[PCI_ROM_RESOURCE].start)
- pci_write_config_byte(dev, PCI_ROM_ADDRESS,
+ pci_write_config_dword(dev, PCI_ROM_ADDRESS,
dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE);
pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, (L1_CACHE_BYTES / 4));
--
didn't Linus find similar bugs in a couple of the other hpt drivers as
well? if so can they be fixed at the same time?
David Lang
On Wed, 14 Sep 2005, Chris Wright wrote:
> Date: Wed, 14 Sep 2005 18:03:47 -0700
> From: Chris Wright <[email protected]>
> To: [email protected], [email protected]
> Cc: Justin Forbes <[email protected]>,
> Zwane Mwaikambo <[email protected]>, Theodore Ts'o <[email protected]>,
> Randy Dunlap <[email protected]>,
> Chuck Wolber <[email protected]>, [email protected], [email protected],
> [email protected], Linus Torvalds <[email protected]>,
> Chris Wright <[email protected]>
> Subject: [PATCH 04/11] hpt366: write the full 4 bytes of ROM address,
> not just low 1 byte
>
> -stable review patch. If anyone has any objections, please let us know.
> ------------------
>
> This is one heck of a confused driver. It uses a byte write to a dword
> register to enable a ROM resource that it doesn't even seem to be using.
>
> "Lost and wandering in the desert of confusion"
>
> Signed-off-by: Linus Torvalds <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> drivers/ide/pci/hpt366.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> Index: linux-2.6.13.y/drivers/ide/pci/hpt366.c
> ===================================================================
> --- linux-2.6.13.y.orig/drivers/ide/pci/hpt366.c
> +++ linux-2.6.13.y/drivers/ide/pci/hpt366.c
> @@ -1334,9 +1334,13 @@ static int __devinit init_hpt366(struct
> static unsigned int __devinit init_chipset_hpt366(struct pci_dev *dev, const char *name)
> {
> int ret = 0;
> - /* FIXME: Not portable */
> +
> + /*
> + * FIXME: Not portable. Also, why do we enable the ROM in the first place?
> + * We don't seem to be using it.
> + */
> if (dev->resource[PCI_ROM_RESOURCE].start)
> - pci_write_config_byte(dev, PCI_ROM_ADDRESS,
> + pci_write_config_dword(dev, PCI_ROM_ADDRESS,
> dev->resource[PCI_ROM_RESOURCE].start | PCI_ROM_ADDRESS_ENABLE);
>
> pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, (L1_CACHE_BYTES / 4));
>
> --
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare
David Lang <[email protected]> wrote:
>
> didn't Linus find similar bugs in a couple of the other hpt drivers as
> well? if so can they be fixed at the same time?
Adam Kropelin did a sweep and picked up four similar cases. I queued the
patches and they should be considered for 2.6.13.3
On Wed, 14 Sep 2005, Andrew Morton wrote:
> David Lang <[email protected]> wrote:
>>
>> didn't Linus find similar bugs in a couple of the other hpt drivers as
>> well? if so can they be fixed at the same time?
>
> Adam Kropelin did a sweep and picked up four similar cases. I queued the
> patches and they should be considered for 2.6.13.3
>
Thanks,
David Lang
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare
* David Lang ([email protected]) wrote:
> didn't Linus find similar bugs in a couple of the other hpt drivers as
> well? if so can they be fixed at the same time?
Yes, and they're in this series. This patch does:
drivers/ide/pci/hpt366.c
And patch 10/11 does:
drivers/ide/pci/cmd64x.c
drivers/ide/pci/hpt34x.c
Additionally, the sungem (5/11) and sunhme (6/11) changes are fallout
from PCI_ROM fixes.
I believe the remainder of the PCI_ROM related patches were primarily
for consistency.
thanks,
-chris
Hello!
> This is one heck of a confused driver. It uses a byte write to a dword
> register to enable a ROM resource that it doesn't even seem to be using.
Once upon a time when I was the PCI maintainer, I was arguing with Andre
Hedrick about this one and he kept asserting that enabling the ROM is
necessary to make the chip work. Not that I believe it :)
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Never make any mistaeks.
On Wed, 14 Sep 2005, Chris Wright wrote:
> * David Lang ([email protected]) wrote:
>> didn't Linus find similar bugs in a couple of the other hpt drivers as
>> well? if so can they be fixed at the same time?
>
> Yes, and they're in this series. This patch does:
>
> drivers/ide/pci/hpt366.c
>
> And patch 10/11 does:
>
> drivers/ide/pci/cmd64x.c
> drivers/ide/pci/hpt34x.c
>
> Additionally, the sungem (5/11) and sunhme (6/11) changes are fallout
> from PCI_ROM fixes.
>
> I believe the remainder of the PCI_ROM related patches were primarily
> for consistency.
sorry about the noise, I read through all the descriptions looking for
these, but I hadn't read all the patches.
David Lang
P.S. thanks to the kernel team for catching this before I got a chance to
install .13 on my system that has a hpt374 controller :-)
--
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
-- C.A.R. Hoare