Re: [PATCH 01/18] ide: add ->read_sff_dma_status method

On Sunday 07 September 2008, Sergei Shtylyov wrote:

Hello.

Bartlomiej Zolnierkiewicz wrote:

quoted

quoted

quoted

static void ide_tf_load(ide_drive_t *drive, ide_task_t *task)
{
	ide_hwif_t *hwif = drive->hwif;

@@ -323,6 +331,8 @@ static void ata_output_data(ide_drive_t 

void default_hwif_transport(ide_hwif_t *hwif)
{
+	hwif->read_sff_dma_status = ide_read_sff_dma_status;
+
	hwif->tf_load	  = ide_tf_load;
	hwif->tf_read	  = ide_tf_read;

   I also didn't understand the motivation behind putting this method 
together with the transport operations... IMO, DMA programming interface 
hardly has anything to do with transporting the data over IDE bus.

quoted

The motivation was that hwif->dma_ops is not available yet when
->read_sff_dma_status is used in ide_pci_check_simplex().

quoted

However I agree that it should somehow find its way into ->dma_ops
(as usual patches are stongly preffered :).

    Unless I'm missing something changing the place where hwif->dma_ops is 
initialized to sff_dma_ops (along the lines it was changed for hwif->dma_base) 
seems pretty trivial, so I wonder why you didn't do it in the same patch...

Indeed, it should be trivial now, one just needs to be careful to:

* move 'if (d->dma_ops) ...' from ide_init_port() into
  ->init_dma/ide_hwif_setup_dma()

* unset ->dma_ops on ->init_dma/ide_hwif_setup_dma() failures

I guess I overlooked it ATM of making the patch (or the code evolved
greatly in the meantime)...

[ It is really time consuming and difficult to recall the every small
  detail of every patch after few months (the patch was posted 10 weeks
  ago and merged 6 weeks ago)...  The most efficient way of handling
  such issues upon discovery is with sending patches... ]

Thanks,
Bart