lkml.org 
[lkml]   [2007]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void
Hello again. :-)

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void

Below are my nits on the patch itself, and the code it changes.

> Index: b/drivers/ide/pci/atiixp.c
> ===================================================================
> --- a/drivers/ide/pci/atiixp.c
> +++ b/drivers/ide/pci/atiixp.c
> @@ -121,7 +121,7 @@ static int atiixp_ide_dma_host_on(ide_dr
> return __ide_dma_host_on(drive);
> }
>
> -static int atiixp_ide_dma_host_off(ide_drive_t *drive)
> +static void atiixp_ide_dma_host_off(ide_drive_t *drive)
> {
> struct pci_dev *dev = drive->hwif->pci_dev;
> unsigned long flags;
[...]
> @@ -306,7 +306,7 @@ static void __devinit init_hwif_atiixp(i
> hwif->udma_four = 0;
>
> hwif->ide_dma_host_on = &atiixp_ide_dma_host_on;
> - hwif->ide_dma_host_off = &atiixp_ide_dma_host_off;
> + hwif->dma_host_off = &atiixp_ide_dma_host_off;
> hwif->ide_dma_check = &atiixp_dma_check;
> if (!noautodma)
> hwif->autodma = 1;

Would seem logical to get rid of ide_ in the function's name also...

> Index: b/drivers/ide/pci/sgiioc4.c
> ===================================================================
> --- a/drivers/ide/pci/sgiioc4.c
> +++ b/drivers/ide/pci/sgiioc4.c
> @@ -282,12 +282,11 @@ sgiioc4_ide_dma_on(ide_drive_t * drive)
> return HWIF(drive)->ide_dma_host_on(drive);
> }
>
> -static int
> -sgiioc4_ide_dma_off_quietly(ide_drive_t * drive)
> +static void sgiioc4_ide_dma_off_quietly(ide_drive_t *drive)
> {
> drive->using_dma = 0;
>
> - return HWIF(drive)->ide_dma_host_off(drive);
> + drive->hwif->dma_host_off(drive);
> }
>
> static int sgiioc4_ide_dma_check(ide_drive_t *drive)
> @@ -317,12 +316,9 @@ sgiioc4_ide_dma_host_on(ide_drive_t * dr
> return 1;
> }
>
> -static int
> -sgiioc4_ide_dma_host_off(ide_drive_t * drive)
> +static void sgiioc4_ide_dma_host_off(ide_drive_t * drive)
> {
> sgiioc4_clearirq(drive);
> -
> - return 0;
> }
>
> static int
> @@ -612,10 +608,10 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
> hwif->ide_dma_end = &sgiioc4_ide_dma_end;
> hwif->ide_dma_check = &sgiioc4_ide_dma_check;
> hwif->ide_dma_on = &sgiioc4_ide_dma_on;
> - hwif->ide_dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
> + hwif->dma_off_quietly = &sgiioc4_ide_dma_off_quietly;
> hwif->ide_dma_test_irq = &sgiioc4_ide_dma_test_irq;
> hwif->ide_dma_host_on = &sgiioc4_ide_dma_host_on;
> - hwif->ide_dma_host_off = &sgiioc4_ide_dma_host_off;
> + hwif->dma_host_off = &sgiioc4_ide_dma_host_off;
> hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
> hwif->ide_dma_timeout = &__ide_dma_timeout;

The same here...

> Index: b/drivers/ide/pci/sl82c105.c
> ===================================================================
> --- a/drivers/ide/pci/sl82c105.c
> +++ b/drivers/ide/pci/sl82c105.c
> @@ -261,26 +261,24 @@ static int sl82c105_ide_dma_on (ide_driv
>
> if (config_for_dma(drive)) {
> config_for_pio(drive, 4, 0, 0);

Ugh, this forces PIO4 on fallback... and dma_on() doesn't select any modes
in any other driver but this one. :-/

> - return HWIF(drive)->ide_dma_off_quietly(drive);
> + drive->hwif->dma_off_quietly(drive);
> + return 0;
> }
> printk(KERN_INFO "%s: DMA enabled\n", drive->name);
> return __ide_dma_on(drive);
> }
>
> -static int sl82c105_ide_dma_off_quietly (ide_drive_t *drive)
> +static void sl82c105_ide_dma_off_quietly(ide_drive_t *drive)

Also worth renaming...

> {
> u8 speed = XFER_PIO_0;
> - int rc;
> -
> +
> DBG(("sl82c105_ide_dma_off_quietly(drive:%s)\n", drive->name));
>
> - rc = __ide_dma_off_quietly(drive);
> + ide_dma_off_quietly(drive);
> if (drive->pio_speed)

Should always be non-zero since explicitly initialized.

> speed = drive->pio_speed - XFER_PIO_0;
> config_for_pio(drive, speed, 0, 1);
> drive->current_speed = drive->pio_speed;

dma_off() shouldn't be changing current_speed IMHO.

> -
> - return rc;
> }

The patch to fix those two functions is also cooking...

MBR, Sergei

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-01-20 23:13    [W:1.263 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site