On Sunday 05 August 2007, Sergei Shtylyov wrote:
> The Marvell bridge chips used on HighPoint SATA cards do not seem to support
> the UltraDMA modes 1, 2, and 3 (as well as any MWDMA modes), so the driver
> needs to account for this in the udma_filter() method. In order to achieve
> that, do the following changes:
>
> - install the method for all chips, not only HPT36x/370 (improve code formatting
> by killing an extra tabs while at it);
>
> - add to the end of the 'switch' statement in hpt3xx_udma_filter() case for
> HPT372[AN] and HPT374 chips upon which the SATA cards are based and check
> there whether we're dealing with SATA drive (by looking at words 80 and 93
> of the drive's identify data), reorder HPT370[A] cases for consistency...
>
> Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
applied but
> ---
> This is against the current Linus tree and unfortunately I was able to only
> compile test it since that tree gives MODPOST warning and dies early.
> Bob, please test it if/when you'll be able to and report the results...
>
> drivers/ide/pci/hpt366.c | 75 ++++++++++++++++++++++++++---------------------
> 1 files changed, 43 insertions(+), 32 deletions(-)
>
> Index: linux-2.6/drivers/ide/pci/hpt366.c
> ===================================================================
> --- linux-2.6.orig/drivers/ide/pci/hpt366.c
> +++ linux-2.6/drivers/ide/pci/hpt366.c
> @@ -1,5 +1,5 @@
> /*
> - * linux/drivers/ide/pci/hpt366.c Version 1.11 Aug 4, 2007
> + * linux/drivers/ide/pci/hpt366.c Version 1.12 Aug 5, 2007
> *
> * Copyright (C) 1999-2003 Andre Hedrick <andre@xxxxxxxxxxxxx>
> * Portions Copyright (C) 2001 Sun Microsystems, Inc.
> @@ -517,29 +517,17 @@ static int check_in_drive_list(ide_drive
> }
>
> /*
> - * Note for the future; the SATA hpt37x we must set
> - * either PIO or UDMA modes 0,4,5
> + * The Marvell bridge chips used on the HighPoint SATA cards do not seem
> + * to support the UltraDMA modes 1, 2, and 3 -- as well as any MWDMA modes
> + * (that we should start filtering out once the IDE core allows that).
> */
> -
> static u8 hpt3xx_udma_filter(ide_drive_t *drive)
> {
> struct hpt_info *info = pci_get_drvdata(HWIF(drive)->pci_dev);
> + struct hd_driveid *id = drive->id;
> u8 mask;
>
> switch (info->chip_type) {
HPT374/HPT372[NA] case could be added here so re-ordering wouldn't be needed.
> - case HPT370A:
> - if (!HPT370_ALLOW_ATA100_5 ||
> - check_in_drive_list(drive, bad_ata100_5))
> - return 0x1f;
> - else
> - return 0x3f;
> - case HPT370:
> - if (!HPT370_ALLOW_ATA100_5 ||
> - check_in_drive_list(drive, bad_ata100_5))
> - mask = 0x1f;
> - else
> - mask = 0x3f;
> - break;
> case HPT36x:
> if (!HPT366_ALLOW_ATA66_4 ||
> check_in_drive_list(drive, bad_ata66_4))
> @@ -551,6 +539,30 @@ static u8 hpt3xx_udma_filter(ide_drive_t
> check_in_drive_list(drive, bad_ata66_3))
> mask = 0x07;
> break;
> + case HPT370:
> + if (!HPT370_ALLOW_ATA100_5 ||
> + check_in_drive_list(drive, bad_ata100_5))
> + mask = 0x1f;
> + else
> + mask = 0x3f;
ATA_UDMA* defines should be used if you insist on re-ordering
> + break;
> + case HPT370A:
> + if (!HPT370_ALLOW_ATA100_5 ||
> + check_in_drive_list(drive, bad_ata100_5))
> + return 0x1f;
> + else
> + return 0x3f;
ditto
> + case HPT372 :
> + case HPT372A:
> + case HPT372N:
> + case HPT374 :
> + /*
> + * Check for SATA drive by verifying that the word 93 is 0 and
> + * the drive is ATA-5 or higher compatible.
> + */
> + if (id->hw_config == 0 && (id->major_rev_num & 0x7fe0))
Same check as in ide-iops.c::eighty_ninty_three().
Would make sense to add ide_id_is_sata_dev() inline to <linux/ide.h>.
> + return 0x71;
> + /* fall thru */
> default:
> return 0x7f;
HPT371[N]/HPT302[N] will use the default mask which is correct but adds
hidden dependency on HPT*_ALLOW_ATA_133 being always defined as "1".
IMO all HPT*_ALLOW_ATA* defines should just go away...
Also now that ->udma_filter is always present the initial hwif->ultra_mask
doesn't matter so as well we may set it to ATA_UDMA6 (0x7f) and cleanup
struct hpt_info (by removing max_ultra after fixing init_chipset_hpt366()
to use info->chip_type >= HPT374 check instead), init_setup_hpt366() and
hpt366_chipsets[] (by removing udma_mask).
> }
> @@ -1229,25 +1241,24 @@ static unsigned int __devinit init_chips
>
> static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
> {
> - struct pci_dev *dev = hwif->pci_dev;
> - struct hpt_info *info = pci_get_drvdata(dev);
> - int serialize = HPT_SERIALIZE_IO;
> - u8 scr1 = 0, ata66 = hwif->channel ? 0x01 : 0x02;
> - u8 chip_type = info->chip_type;
> - u8 new_mcr, old_mcr = 0;
> + struct pci_dev *dev = hwif->pci_dev;
> + struct hpt_info *info = pci_get_drvdata(dev);
> + int serialize = HPT_SERIALIZE_IO;
> + u8 scr1 = 0, ata66 = hwif->channel ? 0x01 : 0x02;
> + u8 chip_type = info->chip_type;
> + u8 new_mcr, old_mcr = 0;
>
> /* Cache the channel's MISC. control registers' offset */
> - hwif->select_data = hwif->channel ? 0x54 : 0x50;
> + hwif->select_data = hwif->channel ? 0x54 : 0x50;
>
> - hwif->tuneproc = &hpt3xx_tune_drive;
> - hwif->speedproc = &hpt3xx_tune_chipset;
> - hwif->quirkproc = &hpt3xx_quirkproc;
> - hwif->intrproc = &hpt3xx_intrproc;
> - hwif->maskproc = &hpt3xx_maskproc;
> - hwif->busproc = &hpt3xx_busproc;
> + hwif->tuneproc = &hpt3xx_tune_drive;
> + hwif->speedproc = &hpt3xx_tune_chipset;
> + hwif->quirkproc = &hpt3xx_quirkproc;
> + hwif->intrproc = &hpt3xx_intrproc;
> + hwif->maskproc = &hpt3xx_maskproc;
> + hwif->busproc = &hpt3xx_busproc;
>
> - if (chip_type <= HPT370A)
> - hwif->udma_filter = &hpt3xx_udma_filter;
> + hwif->udma_filter = &hpt3xx_udma_filter;
Uh, the only real change here consists of the three lines above, the rest
is just a noise caused by removal of one tab.
Such changes are really not worth it - in this case it caused rejects in
two patches from IDE quilt tree which I had to fix manually.
Thanks,
Bart
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
