ide: remove ide_find_best_pio_mode()
PATCH 8/15 ide: remove ide_find_best_pio_mode()
* Add ->host_flags to ide_hwif_t to store ide_pci_device_t.host_flags, assign it in setup-pci.c:ide_pci_setup_ports().
* Add IDE_HFLAG_PIO_NO_{BLACKLIST,DOWNGRADE} to ide_pci_device_t.host_flags and teach ide_get_best_pio_mode() about them. Also remove needless !drive->id check while at it (drive->id is always present).
* Convert amd74xx, via82cxxx and ide-timing.h to use ide_get_best_pio_mode() and then remove no longer needed ide_find_best_pio_mode().
There should be no functionality changes caused by this patch.
--- drivers/ide/ide-lib.c | 13 +++++++------ drivers/ide/ide-timing.h | 18 ++---------------- drivers/ide/ide.c | 2 ++ drivers/ide/pci/amd74xx.c | 20 ++++++++++++-------- drivers/ide/pci/via82cxxx.c | 22 +++++++++++++--------- drivers/ide/setup-pci.c | 2 ++ include/linux/ide.h | 7 +++++++ 7 files changed, 45 insertions(+), 39 deletions(-)
drivers/ide/ide-lib.c +7 -6
@@ -291,11 +291,11 @@ u8 ide_get_best_pio_mode (ide_drive_t *dstruct hd_driveid * id = drive->id;int overridden =0 ; -if (mode_wanted !=255 ) { - pio_mode = mode_wanted; - }else if (!drive->id) { - pio_mode =0 ; - }else if ((pio_mode =ide_scan_pio_blacklist (id->model)) !=-1 ) { +if (mode_wanted !=255 ) +return min_t (u8, mode_wanted, max_mode); + +if ((drive->hwif->host_flags &IDE_HFLAG_PIO_NO_BLACKLIST ) ==0 && + (pio_mode =ide_scan_pio_blacklist (id->model)) !=-1 ) {printk (KERN_INFO "%s: is on PIO blacklist\n" , drive->name); }else { pio_mode = id->tPIO;
@@ -324,7 +324,8 @@ u8 ide_get_best_pio_mode (ide_drive_t *d/* * Conservative "downgrade" for all pre-ATA2 drives */ -if (pio_mode && pio_mode <4 ) { +if ((drive->hwif->host_flags &IDE_HFLAG_PIO_NO_DOWNGRADE ) ==0 && + pio_mode && pio_mode <4 ) { pio_mode--;printk (KERN_INFO "%s: applying conservative " "PIO \"downgrade\"\n" , drive->name);
drivers/ide/ide-timing.h +2 -16
@@ -106,21 +106,6 @@ static struct ide_timing ide_timing[] =#define XFER_EPIO 0x01#define XFER_PIO 0x00 -static short ide_find_best_pio_mode (ide_drive_t *drive) -{ -struct hd_driveid *id = drive->id; -short best =0 ; - -/* EIDE PIO modes */ -if ((id->field_valid &2 ) && (id->capability &8 )) { -if ((best = (drive->id->eide_pio_modes &4 ) ?XFER_PIO_5 : - (drive->id->eide_pio_modes &2 ) ?XFER_PIO_4 : - (drive->id->eide_pio_modes &1 ) ?XFER_PIO_3 :0 ))return best; - } - -return min_t (u8, id->tPIO,2 ); -} -static void ide_timing_quantize (struct ide_timing *t,struct ide_timing *q,int T ,int UT ) { q->setup =EZ (t->setup *1000 ,T );
@@ -210,7 +195,8 @@ static int ide_timing_compute(ide_drive_*/ if ((speed &XFER_MODE ) !=XFER_PIO ) { -ide_timing_compute (drive,ide_find_best_pio_mode (drive), &p,T ,UT ); +u8 pio =ide_get_best_pio_mode (drive,255 ,5 ); +ide_timing_compute (drive,XFER_PIO_0 + pio, &p,T ,UT );ide_timing_merge (&p, t, t,IDE_TIMING_ALL ); }
drivers/ide/ide.c +2 -0
@@ -455,6 +455,8 @@ static void ide_hwif_restore(ide_hwif_t hwif->straight8 = tmp_hwif->straight8; hwif->bus_state = tmp_hwif->bus_state; + hwif->host_flags = tmp_hwif->host_flags; + hwif->atapi_dma = tmp_hwif->atapi_dma; hwif->ultra_mask = tmp_hwif->ultra_mask; hwif->mwdma_mask = tmp_hwif->mwdma_mask;
drivers/ide/pci/amd74xx.c +12 -8
@@ -1,5 +1,5 @@/* -* Version 2.20 +* Version 2.21 * * AMD 755/756/766/8111 and nVidia nForce/2/2s/3/3s/CK804/MCP04 * IDE driver for Linux.
@@ -275,10 +275,8 @@ static int amd_set_drive(ide_drive_t *drstatic void amd74xx_tune_drive (ide_drive_t *drive,u8 pio) { -if (pio ==255 ) { -amd_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );amd_set_drive (drive,XFER_PIO_0 +min_t (byte, pio,5 )); }
@@ -287,12 +285,14 @@ static int amd74xx_ide_dma_check(ide_dri {u8 speed =ide_max_dma_mode (drive); -if (speed ==0 ) - speed =ide_find_best_pio_mode (drive); +if (speed ==0 ) { +amd74xx_tune_drive (drive,255 ); +return -1 ; + }amd_set_drive (drive, speed); -if (drive->autodma && (speed &XFER_MODE ) !=XFER_PIO ) +if (drive->autodma)return 0 ;return -1 ;
@@ -455,6 +455,8 @@ static void __devinit init_hwif_amd74xx( .autodma = AUTODMA, \ .enablebits = {{0x40,0x02,0x02}, {0x40,0x01,0x01}}, \ .bootable = ON_BOARD, \ + .host_flags = IDE_HFLAG_PIO_NO_BLACKLIST \ + | IDE_HFLAG_PIO_NO_DOWNGRADE, \ }#define DECLARE_NV_DEV (name_str) \
@@ -465,6 +467,8 @@ static void __devinit init_hwif_amd74xx( .autodma = AUTODMA, \ .enablebits = {{0x50,0x02,0x02}, {0x50,0x01,0x01}}, \ .bootable = ON_BOARD, \ + .host_flags = IDE_HFLAG_PIO_NO_BLACKLIST \ + | IDE_HFLAG_PIO_NO_DOWNGRADE, \ }static ide_pci_device_t amd74xx_chipsets[]__devinitdata = {
drivers/ide/pci/via82cxxx.c +13 -9
@@ -1,6 +1,6 @@/* * -* Version 3.45 +* Version 3.46 * * VIA IDE driver for Linux. Supported southbridges: *
@@ -203,10 +203,8 @@ static int via_set_drive(ide_drive_t *drstatic void via82cxxx_tune_drive (ide_drive_t *drive,u8 pio) { -if (pio ==255 ) { -via_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );via_set_drive (drive,XFER_PIO_0 +min_t (u8, pio,5 )); }
@@ -223,12 +221,14 @@ static int via82cxxx_ide_dma_check (ide_ {u8 speed =ide_max_dma_mode (drive); -if (speed ==0 ) - speed =ide_find_best_pio_mode (drive); +if (speed ==0 ) { +via82cxxx_tune_drive (drive,255 ); +return -1 ; + }via_set_drive (drive, speed); -if (drive->autodma && (speed &XFER_MODE ) !=XFER_PIO ) +if (drive->autodma)return 0 ;return -1 ;
@@ -503,7 +503,9 @@ static ide_pci_device_t via82cxxx_chipse .init_hwif = init_hwif_via82cxxx, .autodma = NOAUTODMA, .enablebits = {{0x40,0x02,0x02}, {0x40,0x01,0x01}}, - .bootable = ON_BOARD + .bootable = ON_BOARD, + .host_flags = IDE_HFLAG_PIO_NO_BLACKLIST + | IDE_HFLAG_PIO_NO_DOWNGRADE, },{ /* 1 */ .name = "VP_IDE", .init_chipset = init_chipset_via82cxxx,
@@ -511,6 +513,8 @@ static ide_pci_device_t via82cxxx_chipse .autodma = AUTODMA, .enablebits = {{0x00,0x00,0x00}, {0x00,0x00,0x00}}, .bootable = ON_BOARD, + .host_flags = IDE_HFLAG_PIO_NO_BLACKLIST + | IDE_HFLAG_PIO_NO_DOWNGRADE, } };
drivers/ide/setup-pci.c +2 -0
@@ -613,6 +613,8 @@ void ide_pci_setup_ports(struct pci_develse ide_hwif_setup_dma (dev, d, hwif); bypass_legacy_dma: + hwif->host_flags = d->host_flags; +if (d->init_hwif)/* Call chipset-specific routine * for each enabled hwif
include/linux/ide.h +7 -0
@@ -681,6 +681,8 @@ typedef struct hwif_s {u8 straight8;/* Alan's straight 8 check */ u8 bus_state;/* power state of the IDE bus */ +u8 host_flags; +u8 atapi_dma;/* host supports atapi_dma */ u8 ultra_mask;u8 mwdma_mask;
@@ -1245,7 +1247,12 @@ typedef struct ide_pci_enablebit_s {enum {/* Uses ISA control ports not PCI ones. */ IDE_HFLAG_ISA_PORTS = (1 <<0 ), +/* single port device */ IDE_HFLAG_SINGLE = (1 <<1 ), +/* don't use legacy PIO blacklist */ +IDE_HFLAG_PIO_NO_BLACKLIST = (1 <<2 ), +/* don't use conservative PIO "downgrade" */ +IDE_HFLAG_PIO_NO_DOWNGRADE = (1 <<3 ), }; typedef struct ide_pci_device_s {
Quoted 7 lines · Bartlomiej Zolnierkiewicz
* Add ->host_flags to
ide_hwif_tto store ide_pci_device_t.host_flags, assign it in setup-pci.c:ide_pci_setup_ports().* Add IDE_HFLAG_PIO_NO_{BLACKLIST,DOWNGRADE} to ide_pci_device_t.host_flags and teach
ide_get_best_pio_mode()about them. Also remove needless !drive->id check while at it (drive->idis always present).
Quoted 6 lines
* Convert amd74xx, via82cxxx and
ide-timing.hto useide_get_best_pio_mode()and then remove no longer neededide_find_best_pio_mode().There should be no functionality changes caused by this patch.
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Still I have something to ask...
Quoted 7 lines
Index:
b/drivers/ide/ide-lib.c
===================================================================drivers/ide/ide-lib.c +0
-0@@ -291,11 +291,11 @@ u8 ide_get_best_pio_mode (ide_drive_t *dstruct hd_driveid * id = drive->id;int overridden =0 ;
drivers/ide/ide-lib.c u8 ide_get_best_pio_mode (ide_drive_t *d · line 297
Quoted 10 lines
drivers/ide/ide-lib.c +4
-5-if (mode_wanted !=255 ) { - pio_mode = mode_wanted; - }else if (!drive->id) { - pio_mode =0 ; - }else if ((pio_mode =ide_scan_pio_blacklist (id->model)) !=-1 ) { +if (mode_wanted !=255 ) +return min_t (u8, mode_wanted, max_mode); + + if ((drive->hwif->host_flags & IDE_HFLAG_PIO_NO_BLACKLIST) == 0 &&
C also has ! operator. ;-)
drivers/ide/ide-lib.c u8 ide_get_best_pio_mode (ide_drive_t *d · line 327
Quoted 10 lines
drivers/ide/ide-lib.c +2
-1+ (pio_mode = ide_scan_pio_blacklist(id->model)) != -1) { printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name); } else { pio_mode = id->tPIO;@@ -324,7 +324,8 @@ u8 ide_get_best_pio_mode (ide_drive_t *d/* * Conservative "downgrade" for all pre-ATA2 drives */ -if (pio_mode && pio_mode <4 ) { + if ((drive->hwif->host_flags & IDE_HFLAG_PIO_NO_DOWNGRADE) == 0 &&
It really does! :-)
Quoted 18 lines
drivers/ide/ide-lib.c +1
-0+ pio_mode && pio_mode < 4) { pio_mode--;printk (KERN_INFO "%s: applying conservative " "PIO \"downgrade\"\n" , drive->name);Index:
b/drivers/ide/pci/amd74xx.c
===================================================================drivers/ide/pci/amd74xx.c +1
-1@@ -1,5 +1,5 @@/* -* Version 2.20 +* Version 2.21 * * AMD 755/756/766/8111 and nVidia nForce/2/2s/3/3s/CK804/MCP04 * IDE driver for Linux. @@ -275,10 +275,8 @@ static int amd_set_drive(ide_drive_t *drstatic void amd74xx_tune_drive (ide_drive_t *drive,u8 pio)
drivers/ide/pci/amd74xx.c static int amd_set_drive(ide_drive_t *dr · line 282
Quoted 10 lines
drivers/ide/pci/amd74xx.c +2
-4{ -if (pio ==255 ) { -amd_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );amd_set_drive (drive,XFER_PIO_0 +min_t (byte, pio,5 )); }
Erm, I don't get it -- why not just use ide_get_best_pio_mode() for every
'pio'? There should be no functionality changes, no?
Quoted 14 lines
Index:
b/drivers/ide/pci/via82cxxx.c
===================================================================drivers/ide/pci/via82cxxx.c +1
-1@@ -1,6 +1,6 @@/* * -* Version 3.45 +* Version 3.46 * * VIA IDE driver for Linux. Supported southbridges: * @@ -203,10 +203,8 @@ static int via_set_drive(ide_drive_t *dr
drivers/ide/pci/via82cxxx.c static int via_set_drive(ide_drive_t *dr · line 209
Quoted 10 lines
drivers/ide/pci/via82cxxx.c +2
-4static void via82cxxx_tune_drive (ide_drive_t *drive,u8 pio) { -if (pio ==255 ) { -via_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );via_set_drive (drive,XFER_PIO_0 +min_t (u8, pio,5 ));
Same question here...
MBR, Sergei
Quoted 9 lines · Sergei Shtylyov
Quoted 6 lines · Bartlomiej Zolnierkiewicz
* Add ->host_flags to
ide_hwif_tto store ide_pci_device_t.host_flags, assign it in setup-pci.c:ide_pci_setup_ports().* Add IDE_HFLAG_PIO_NO_{BLACKLIST,DOWNGRADE} to ide_pci_device_t.host_flags and teach
ide_get_best_pio_mode()about them. Also remove needless !drive->id check while at it (drive->idis always present).
Quoted 8 lines
Quoted 6 lines
* Convert amd74xx, via82cxxx and
ide-timing.hto useide_get_best_pio_mode()and then remove no longer neededide_find_best_pio_mode().There should be no functionality changes caused by this patch.
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>Acked-by Shtylyov <[email protected]>
added
Quoted 10 lines
Still I have something to ask...
Quoted 8 lines
Index:
b/drivers/ide/ide-lib.c
===================================================================drivers/ide/ide-lib.c +0
-0@@ -291,11 +291,11 @@ u8 ide_get_best_pio_mode (ide_drive_t *dstruct hd_driveid * id = drive->id;int overridden =0 ;
Quoted 11 lines
drivers/ide/ide-lib.c u8 ide_get_best_pio_mode (ide_drive_t *d · line 297
Quoted 9 lines
drivers/ide/ide-lib.c +4
-5-if (mode_wanted !=255 ) { - pio_mode = mode_wanted; - }else if (!drive->id) { - pio_mode =0 ; - }else if ((pio_mode =ide_scan_pio_blacklist (id->model)) !=-1 ) { +if (mode_wanted !=255 ) +return min_t (u8, mode_wanted, max_mode); + + if ((drive->hwif->host_flags & IDE_HFLAG_PIO_NO_BLACKLIST) == 0 &&C also has ! operator. ;-)
IMO !() is less readable and more error-prone than () == 0.
Matter of style.
Quoted 4 lines
Quoted 4 lines
drivers/ide/ide-lib.c +1
-0+ (pio_mode = ide_scan_pio_blacklist(id->model)) != -1) { printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name); } else { pio_mode = id->tPIO;
Quoted 8 lines
drivers/ide/ide-lib.c u8 ide_get_best_pio_mode (ide_drive_t *d · line 327
Quoted 6 lines
drivers/ide/ide-lib.c +1
-1@@ -324,7 +324,8 @@ u8 ide_get_best_pio_mode (ide_drive_t *d/* * Conservative "downgrade" for all pre-ATA2 drives */ -if (pio_mode && pio_mode <4 ) { + if ((drive->hwif->host_flags & IDE_HFLAG_PIO_NO_DOWNGRADE) == 0 &&It really does! :-)
This turns into ExtremeNitpicking. ;-)
Quoted 26 lines
Quoted 26 lines
drivers/ide/ide-lib.c +1
-0+ pio_mode && pio_mode < 4) { pio_mode--;printk (KERN_INFO "%s: applying conservative " "PIO \"downgrade\"\n" , drive->name);Index:
b/drivers/ide/pci/amd74xx.c
===================================================================drivers/ide/pci/amd74xx.c +3
-5@@ -1,5 +1,5 @@/* -* Version 2.20 +* Version 2.21 * * AMD 755/756/766/8111 and nVidia nForce/2/2s/3/3s/CK804/MCP04 * IDE driver for Linux. @@ -275,10 +275,8 @@ static int amd_set_drive(ide_drive_t *drstatic void amd74xx_tune_drive (ide_drive_t *drive,u8 pio) { -if (pio ==255 ) { -amd_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );
Quoted 5 lines
drivers/ide/pci/amd74xx.c static int amd_set_drive(ide_drive_t *dr · line 282
Quoted 2 lines
drivers/ide/pci/amd74xx.c +0
-0amd_set_drive (drive,XFER_PIO_0 +min_t (byte, pio,5 )); }Erm, I don't get it -- why not just use
ide_get_best_pio_mode()for every
'pio'? There should be no functionality changes, no?
Indeed but ide_get_best_pio_mode() call goes away completely in patch #13/15.
Please don't miss "The Big Picture".
Quoted 23 lines
Quoted 23 lines
Index:
b/drivers/ide/pci/via82cxxx.c
===================================================================drivers/ide/pci/via82cxxx.c +3
-5@@ -1,6 +1,6 @@/* * -* Version 3.45 +* Version 3.46 * * VIA IDE driver for Linux. Supported southbridges: * @@ -203,10 +203,8 @@ static int via_set_drive(ide_drive_t *drstatic void via82cxxx_tune_drive (ide_drive_t *drive,u8 pio) { -if (pio ==255 ) { -via_set_drive (drive,ide_find_best_pio_mode (drive)); -return ; - } +if (pio ==255 ) + pio =ide_get_best_pio_mode (drive,255 ,5 );
Quoted 3 lines
drivers/ide/pci/via82cxxx.c static int via_set_drive(ide_drive_t *dr · line 209
Quoted 1 line
drivers/ide/pci/via82cxxx.c +0
-0via_set_drive (drive,XFER_PIO_0 +min_t (u8, pio,5 ));Same question here...
ditto
General remark: please note that the keeping the development _process_ steady is much more important than getting _single_ change perfect (it just need to be good enough) because otherwise we end up investing too much time on things which are not that important in the long-term and time is a very limited resource.
Well, I tended to be perfectionist myself until I took the grasp of above.
;-)
Thanks,
Bart