From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void
Date: Sat, 20 Jan 2007 23:56:26 +0300 [thread overview]
Message-ID: <45B281FA.9050107@ru.mvista.com> (raw)
In-Reply-To: <20070119003213.14846.1366.sendpatchset@localhost.localdomain>
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
next prev parent reply other threads:[~2007-01-20 20:56 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-01-19 0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 1/15] ACPI support for IDE devices Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 2/15] via82cxxx/pata_via: correct PCI_DEVICE_ID_VIA_SATA_EIDE ID and add support for CX700 and 8237S Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 3/15] it8213: fix build and ->ultra_mask Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 4/15] ide: convert ide_hwif_t.mmio into flag Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 5/15] hpt34x: hpt34x_tune_chipset() (->speedproc) fix Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 6/15] atiixp/jmicron/triflex: fix PIO fallback Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 7/15] piix: cleanup Bartlomiej Zolnierkiewicz
2007-01-19 0:31 ` [PATCH 8/15] ide: disable DMA in ->ide_dma_check for "no IORDY" case Bartlomiej Zolnierkiewicz
2007-01-19 16:26 ` Sergei Shtylyov
[not found] ` <58cb370e0701191047h524434eobdb9d86ed614bc71@mail.gmail.com>
2007-01-19 19:11 ` Bartlomiej Zolnierkiewicz
2007-01-19 19:35 ` Sergei Shtylyov
[not found] ` <58cb370e0701191200i10119313i4aacae9c504a02e4@mail.gmail.com>
2007-01-19 21:43 ` Bartlomiej Zolnierkiewicz
2007-01-20 17:09 ` Sergei Shtylyov
[not found] ` <58cb370e0701200947p578f4d74g1fee511ded6c9a77@mail.gmail.com>
2007-01-20 18:21 ` Bartlomiej Zolnierkiewicz
2007-01-20 18:41 ` Sergei Shtylyov
2007-01-25 17:03 ` Sergei Shtylyov
2007-01-19 0:32 ` [PATCH 9/15] sgiioc4: fix sgiioc4_ide_dma_check() to enable/disable DMA properly Bartlomiej Zolnierkiewicz
2007-01-19 0:32 ` [PATCH 10/15] ide: add ide_set_dma() helper Bartlomiej Zolnierkiewicz
2007-01-20 20:22 ` Sergei Shtylyov
2007-01-19 0:32 ` [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
2007-01-20 20:56 ` Sergei Shtylyov [this message]
[not found] ` <58cb370e0702021206h205ff983r88fb4bdbea42ed9f@mail.gmail.com>
2007-02-02 22:39 ` Bartlomiej Zolnierkiewicz
2007-01-19 0:32 ` [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void Bartlomiej Zolnierkiewicz
2007-01-20 20:46 ` Sergei Shtylyov
[not found] ` <58cb370e0702021128g15cac87ar507c20e78ded9464@mail.gmail.com>
2007-02-02 21:16 ` Bartlomiej Zolnierkiewicz
2007-03-26 17:19 ` Sergei Shtylyov
2007-04-20 20:36 ` Sergei Shtylyov
2007-01-19 0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-01-22 16:19 ` Sergei Shtylyov
[not found] ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
2007-02-03 0:39 ` Bartlomiej Zolnierkiewicz
2007-02-03 16:30 ` Sergei Shtylyov
2007-01-22 18:17 ` Sergei Shtylyov
2007-01-22 18:46 ` Alan
2007-01-22 20:28 ` Sergei Shtylyov
2007-01-22 21:31 ` Alan
2007-01-22 22:39 ` Jeff Garzik
[not found] ` <45B6071C.90703@ru.mvista.com>
[not found] ` <45B761F8.1060505@ru.mvista.com>
2007-01-25 22:20 ` Updated SiI h/w docs (was Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks) Jeff Garzik
2007-01-26 13:14 ` Sergei Shtylyov
2007-01-23 14:51 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Sergei Shtylyov
2007-01-25 15:40 ` Sergei Shtylyov
2007-01-31 20:38 ` Sergei Shtylyov
2007-01-31 23:24 ` Alan
[not found] ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
2007-02-03 0:50 ` Bartlomiej Zolnierkiewicz
2007-01-19 0:32 ` [PATCH 14/15] ide: rework the code for selecting the best DMA transfer mode Bartlomiej Zolnierkiewicz
2007-01-22 19:48 ` Sergei Shtylyov
[not found] ` <58cb370e0702021207o435f39cdsf3abb0d55829fc45@mail.gmail.com>
2007-02-02 23:57 ` Bartlomiej Zolnierkiewicz
2007-01-19 0:32 ` [PATCH 15/15] ide: add ide_tune_dma() helper Bartlomiej Zolnierkiewicz
2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45B281FA.9050107@ru.mvista.com \
--to=sshtylyov@ru.mvista.com \
--cc=bzolnier@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.