Re: [PATCH 3/4] ide: add struct ide_dma_ops

Previous thread: [PATCH 4/4] ide: constify struct ide_dma_ops by Bartlomiej Zolnierkiewicz on Sunday, March 9, 2008 - 9:02 am. (4 messages)

Next thread: [PATCH 0/3] v4l: reduce stack usage by Marcin Slusarz on Sunday, March 9, 2008 - 9:47 am. (5 messages)
From: Bartlomiej Zolnierkiewicz
Date: Sunday, March 9, 2008 - 9:00 am

Add struct ide_dma_ops and convert core code + drivers to use it.

While at it:

* Drop "ide_" prefix from ->ide_dma_end and ->ide_dma_test_irq methods.

* siimage.c:
  - add siimage_ide_dma_test_irq() helper
  - print SATA warning in siimage_init_one()

* Remove no longer needed ->init_hwif implementations.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/arm/icside.c       |   24 ++++++---
 drivers/ide/arm/palm_bk3710.c  |    2 
 drivers/ide/cris/ide-cris.c    |   23 ++++-----
 drivers/ide/ide-cd.c           |    6 +-
 drivers/ide/ide-dma.c          |   59 +++++++++++++++---------
 drivers/ide/ide-floppy.c       |    6 +-
 drivers/ide/ide-io.c           |    8 +--
 drivers/ide/ide-iops.c         |   10 ++--
 drivers/ide/ide-probe.c        |    5 +-
 drivers/ide/ide-tape.c         |    6 +-
 drivers/ide/ide-taskfile.c     |    7 +-
 drivers/ide/ide.c              |    2 
 drivers/ide/mips/au1xxx-ide.c  |   31 ++++++------
 drivers/ide/pci/alim15x3.c     |   32 ++++---------
 drivers/ide/pci/cmd64x.c       |  100 +++++++++++++++++++----------------------
 drivers/ide/pci/cs5520.c       |   14 ++---
 drivers/ide/pci/hpt366.c       |   35 ++++++++------
 drivers/ide/pci/it821x.c       |    8 ++-
 drivers/ide/pci/ns87415.c      |    8 ++-
 drivers/ide/pci/pdc202xx_old.c |   35 ++++++--------
 drivers/ide/pci/sc1200.c       |   20 ++------
 drivers/ide/pci/scc_pata.c     |   11 ++--
 drivers/ide/pci/sgiioc4.c      |   26 ++++------
 drivers/ide/pci/siimage.c      |   68 ++++++++++++---------------
 drivers/ide/pci/sl82c105.c     |   27 +++--------
 drivers/ide/pci/tc86c001.c     |    7 ++
 drivers/ide/pci/trm290.c       |   16 ++++--
 drivers/ide/ppc/pmac.c         |   25 ++++++----
 drivers/ide/setup-pci.c        |    2 
 drivers/scsi/ide-scsi.c        |    6 +-
 include/linux/ide.h            |   23 +++++----
 31 files changed, 333 insertions(+), 319 deletions(-)

Index: ...
From: Sergei Shtylyov
Date: Wednesday, March 12, 2008 - 12:27 pm

Hello again. :-)


    I have expect you to drop the prefix from the method implementations which 


    I'd expect some init code weight loss at the expense of one more 
indirection when calling the methods... :-)

    Erm, I don't see 'hwif' declared in that function -- this won't compile.



    You need to decide between au1xxx_ and auide_ prefixes, or the file won't 


    ... and cmd648_ prefix here.








    Now where is the code which selects the correct dma_ops for the 




    Infixes galore! I'd killed both old_ and ide_ (but not Old IDE :-).





    I suggest removing infixes from the above functions instead of adding 

    Now what about this lonely method? It belongs to dma_ops I think...

MBR, Sergei
--

From: Bartlomiej Zolnierkiewicz
Date: Tuesday, March 18, 2008 - 7:12 am

Hi,

On Wednesday 12 March 2008, Sergei Shtylyov wrote:







done for ide_ "infixes" in all host drivers



done



fixed



fixed


It is needed also when DMA is not used so it was left alone
(it probably should be renamed to ->clear_irq later).

Thanks for the review, "take 2" below
(please verify that all issues were addressed).

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: add struct ide_dma_ops (take 2)

Add struct ide_dma_ops and convert core code + drivers to use it.

While at it:

* Drop "ide_" prefix from ->ide_dma_end and ->ide_dma_test_irq methods.

* Drop "ide_" "infixes" from DMA methods.

* au1xxx-ide.c:
  - use auide_dma_{test_irq,end}() directly in auide_dma_timeout()

* pdc202xx_old.c:
  - drop "old_" "infixes" from DMA methods

* siimage.c:
  - add siimage_dma_test_irq() helper
  - print SATA warning in siimage_init_one()

* Remove no longer needed ->init_hwif implementations.

v2:
* Changes based on review from Sergei:
  - s/siimage_ide_dma_test_irq/siimage_dma_test_irq/
  - s/drive->hwif/hwif/ in idefloppy_pc_intr().
  - fix patch description w.r.t. au1xxx-ide changes
  - fix au1xxx-ide build
  - fix naming for cmd64*_dma_ops
  - drop "ide_" and "old_" infixes
  - s/hpt3xxx_dma_ops/hpt37x_dma_ops/
  - s/hpt370x_dma_ops/hpt370_dma_ops/
  - use correct DMA ops for HPT302/N, HPT371/N and HPT374
  - s/it821x_smart_dma_ops/it821x_pass_through_dma_ops/

Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/arm/icside.c       |   24 +++++---
 drivers/ide/arm/palm_bk3710.c  |    2 
 drivers/ide/cris/ide-cris.c    |   23 ++++----
 drivers/ide/ide-cd.c           |    6 +-
 drivers/ide/ide-dma.c          |   59 +++++++++++++--------
 drivers/ide/ide-floppy.c       |    6 +-
 drivers/ide/ide-io.c           |    8 +-
 drivers/ide/ide-iops.c         |   10 +--
 drivers/ide/ide-probe.c        |    5 +
 ...
From: Sergei Shtylyov
Date: Friday, April 11, 2008 - 6:11 am

Hello.




    I'm still not seeing the code to deals with the PCI device ID 4 which may 
be HPT36x, or HPT370, or HPT372[N] depending on revision -- you're always 

    Wrong -- HPT374 should have hpt37x_dma_ops...

MBR, Sergei
--

From: Bartlomiej Zolnierkiewicz
Date: Saturday, April 12, 2008 - 5:32 am

Hi,


Arghhh.  I see now that the HPT370/370A needs a special handling...
(HPT372/372N has already been handled by 'idx++')


Yep, thanks for catching it.

interdiff between v2->v3:

[...]
v3:
* Two bugs slipped in v2 (noticed by Sergei):
  - use correct DMA ops for HPT374 (for real this time)
  - handle HPT370/HPT370A properly

[...]
diff -u b/drivers/ide/pci/hpt366.c b/drivers/ide/pci/hpt366.c
--- b/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -1489,7 +1489,7 @@
 		.enablebits	= {{0x50,0x04,0x04}, {0x54,0x04,0x04}},
 		.udma_mask	= ATA_UDMA5,
 		.port_ops	= &hpt3xx_port_ops,
-		.dma_ops	= &hpt370_dma_ops,
+		.dma_ops	= &hpt37x_dma_ops,
 		.host_flags	= IDE_HFLAGS_HPT3XX,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
@@ -1563,6 +1563,10 @@
 	d.name = info->chip_name;
 	d.udma_mask = info->udma_mask;
 
+	/* fixup ->dma_ops for HPT370/HPT370A */
+	if (info == &hpt370 || info == &hpt370a)
+		d.dma_ops = &hpt370_dma_ops;
+
 	pci_set_drvdata(dev, (void *)info);
 
 	if (info == &hpt36x || info == &hpt374)
--

From: Sergei Shtylyov
Date: Saturday, April 12, 2008 - 10:54 am

Hello.




Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei
--

Previous thread: [PATCH 4/4] ide: constify struct ide_dma_ops by Bartlomiej Zolnierkiewicz on Sunday, March 9, 2008 - 9:02 am. (4 messages)

Next thread: [PATCH 0/3] v4l: reduce stack usage by Marcin Slusarz on Sunday, March 9, 2008 - 9:47 am. (5 messages)