On Wednesday 03 October 2007, Sergei Shtylyov wrote:
> Hello.
>
> Bartlomiej Zolnierkiewicz wrote:
>
> > * Set hwif->dma_base only if allocation of extra ports succeeds.
>
> > * Allocate hwif->dmatable_cpu before reserving I/O ports.
>
> Looks like a bit of teardown code was missed with that change...
Thanks for catching this!
Actually it turns out that the whole change was based on a thinko. :)
> > While at it:
>
> > * Move setting of hwif->dma_{base,master} from ide_{mapped_mmio,iomio}_dma()
> > to ide_setup_dma().
>
> > * Rename 'dma_base' argument to 'base' in ide_setup_dma().
>
> I'm not sure what that achieves...
Makes the code obey 80-columns limit and increases its readability :)
Revised patch follows...
[PATCH] ide: PCI BMDMA initialization fixes (take 2)
* Set hwif->dma_base only if allocation of extra ports succeeds.
While at it:
* Move setting of hwif->dma_{base,master} from ide_{mapped_mmio,iomio}_dma()
to ide_setup_dma().
* Rename 'dma_base' argument to 'base' in ide_setup_dma() (to make the code
obey 80-columns limit and increase its readability).
* Remove stale ide_setup_dma() comment.
v2:
* Change to allocate hwif->dmatable_cpu before reserving I/O ports missed
teardown code (spotted by Sergei). On the second thought this change is
actually unnecessary so revert it in v2.
* Make ide_release_dma_engine() void and remove needless comment.
Cc: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
drivers/ide/ide-dma.c | 32 ++++++++++----------------------
1 file changed, 10 insertions(+), 22 deletions(-)
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -901,10 +901,7 @@ void ide_dma_timeout (ide_drive_t *drive
EXPORT_SYMBOL(ide_dma_timeout);
-/*
- * Needed for allowing full modular support of ide-driver
- */
-static int ide_release_dma_engine(ide_hwif_t *hwif)
+static void ide_release_dma_engine(ide_hwif_t *hwif)
{
if (hwif->dmatable_cpu) {
pci_free_consistent(hwif->pci_dev,
@@ -913,7 +910,6 @@ static int ide_release_dma_engine(ide_hw
hwif->dmatable_dma);
hwif->dmatable_cpu = NULL;
}
- return 1;
}
static int ide_release_iomio_dma(ide_hwif_t *hwif)
@@ -956,12 +952,6 @@ static int ide_mapped_mmio_dma(ide_hwif_
{
printk(KERN_INFO " %s: MMIO-DMA ", hwif->name);
- hwif->dma_base = base;
-
- if(hwif->mate)
- hwif->dma_master = (hwif->channel) ? hwif->mate->dma_base : base;
- else
- hwif->dma_master = base;
return 0;
}
@@ -975,8 +965,6 @@ static int ide_iomio_dma(ide_hwif_t *hwi
return 1;
}
- hwif->dma_base = base;
-
if (hwif->cds->extra) {
hwif->extra_base = base + (hwif->channel ? 8 : 16);
@@ -991,10 +979,6 @@ static int ide_iomio_dma(ide_hwif_t *hwi
}
}
- if(hwif->mate)
- hwif->dma_master = (hwif->channel) ? hwif->mate->dma_base:base;
- else
- hwif->dma_master = base;
return 0;
}
@@ -1006,12 +990,9 @@ static int ide_dma_iobase(ide_hwif_t *hw
return ide_iomio_dma(hwif, base, ports);
}
-/*
- * This can be called for a dynamically installed interface. Don't __init it
- */
-void ide_setup_dma (ide_hwif_t *hwif, unsigned long dma_base, unsigned int num_ports)
+void ide_setup_dma(ide_hwif_t *hwif, unsigned long base, unsigned num_ports)
{
- if (ide_dma_iobase(hwif, dma_base, num_ports))
+ if (ide_dma_iobase(hwif, base, num_ports))
return;
if (ide_allocate_dma_engine(hwif)) {
@@ -1019,6 +1000,13 @@ void ide_setup_dma (ide_hwif_t *hwif, un
return;
}
+ hwif->dma_base = base;
+
+ if (hwif->mate)
+ hwif->dma_master = hwif->channel ? hwif->mate->dma_base : base;
+ else
+ hwif->dma_master = base;
+
if (!(hwif->dma_command))
hwif->dma_command = hwif->dma_base;
if (!(hwif->dma_vendor1))
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
