Re: [PATCH 12/15] ide: make ide_hwif_t.ide_dma_host_on void

[Bartlomiej Zolnierkiewicz]

Sergei Shtylyov wrote:
> 
> Hello again. :-)
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
>> [PATCH] ide: make ide_hwif_t.ide_dma_host_on void
> 
>> * since ide_hwif_t.ide_dma_host_on is called either when drive->using_dma == 1
>>   or when return value is discarded make it void, also drop "ide_" prefix
>> * make __ide_dma_host_on() void and drop "__" prefix
> 
>    Below are some nits which also apply to the previous patch...
> 
>> Index: b/drivers/ide/pci/atiixp.c
>> ===================================================================
>> --- a/drivers/ide/pci/atiixp.c
>> +++ b/drivers/ide/pci/atiixp.c
>> @@ -101,7 +101,7 @@ static u8 atiixp_dma_2_pio(u8 xfer_rate)
>>       }
>>  }
>>
>> -static int atiixp_ide_dma_host_on(ide_drive_t *drive)
>> +static void atiixp_ide_dma_host_on(ide_drive_t *drive)
>>  {
> 
>    Would seem logical to get rid of ide_ in this function's name also...

fixed in v2 version of the patch, thanks

>>       struct pci_dev *dev = drive->hwif->pci_dev;
>>       unsigned long flags;
> [...]
>> Index: b/drivers/ide/pci/sgiioc4.c
>> ===================================================================
>> --- a/drivers/ide/pci/sgiioc4.c
>> +++ b/drivers/ide/pci/sgiioc4.c
> [...]
>> @@ -307,13 +307,8 @@ sgiioc4_ide_dma_test_irq(ide_drive_t * d
>>       return sgiioc4_checkirq(HWIF(drive));
>>  }
>>
>> -static int
>> -sgiioc4_ide_dma_host_on(ide_drive_t * drive)
>> +static void sgiioc4_ide_dma_host_on(ide_drive_t * drive)
> 
>    Same comment here...

ditto

I also fixed the previous patch.

>>  {
>> -     if (drive->using_dma)
>> -             return 0;
>> -
>> -     return 1;
>>  }
>>
>>  static void sgiioc4_ide_dma_host_off(ide_drive_t * drive)
>> @@ -610,7 +605,7 @@ ide_init_sgiioc4(ide_hwif_t * hwif)
>>       hwif->ide_dma_on = &sgiioc4_ide_dma_on;
>>       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->dma_host_on = &sgiioc4_ide_dma_host_on;
>>       hwif->dma_host_off = &sgiioc4_ide_dma_host_off;
>>       hwif->ide_dma_lostirq = &sgiioc4_ide_dma_lostirq;
>>       hwif->ide_dma_timeout = &__ide_dma_timeout;
> 
>    Unrelated note: not sure why this default value needs explicit
> assignemnt...

SGIIOC4 is not PCI IDE BMDMA compatible - it uses its own SG list
format which supports 64-bit addresses.  Thus sgiioc4 driver doesn't use
the default IDE PCI initialization code [ ide_setup_pci_device[s]() ].

The default values are only assigned when using ide_setup_pci_device[s]():

ide_setup_pci_device[s]()
	do_ide_setup_pci_device()
		ide_pci_setup_ports()
			->init_setup_dma() [ or ide_hwif_setup_dma() ]
				->init_dma [ or ide_setup_dma() ]

Thanks,
Bart

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