Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Bartlomiej Zolnierkiewicz wrote: [PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, cmd64x, pdc202xx_{new,old} and piix drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) v2: * piix: fix cable detection for 82801AA_1 and 82372FB_1 [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * cmd64x: use hwif->cds->udma_mask [ Suggested by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * aec62xx: fix newly introduced bug - check DMA status not command register [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] v3: * piix: use hwif->cds->udma_mask [ Suggested by Sergei Shtylyov <[EMAIL PROTECTED]>. ] Index: b/drivers/ide/pci/aec62xx.c === --- a/drivers/ide/pci/aec62xx.c +++ b/drivers/ide/pci/aec62xx.c @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) { + struct pci_dev *dev = hwif->pci_dev; + hwif->autodma = 0; hwif->tuneproc = &aec62xx_tune_drive; hwif->speedproc = &aec62xx_tune_chipset; - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) hwif->serialized = hwif->channel; if (hwif->mate) @@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx( return; } - hwif->ultra_mask = 0x7f; + hwif->ultra_mask = hwif->cds->udma_mask; + + /* atp865 and atp865r */ + if (hwif->ultra_mask == 0x3f) { + /* check bit 0x10 of DMA status register */ + if (inb(pci_resource_start(dev, 4) + 2) & 0x10) + hwif->ultra_mask = 0x7f; /* udma0-6 */ + } + IMO, this asks to be put into init_setup_aec6x80() instead but since I'm already cleaning up that function, I'll move it there myself. MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
[PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, cmd64x, pdc202xx_{new,old} and piix drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) v2: * piix: fix cable detection for 82801AA_1 and 82372FB_1 [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * cmd64x: use hwif->cds->udma_mask [ Suggested by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * aec62xx: fix newly introduced bug - check DMA status not command register [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] v3: * piix: use hwif->cds->udma_mask [ Suggested by Sergei Shtylyov <[EMAIL PROTECTED]>. ] Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> --- drivers/ide/ide.c |3 - drivers/ide/pci/aec62xx.c | 19 +- drivers/ide/pci/alim15x3.c | 13 ++- drivers/ide/pci/cmd64x.c | 15 drivers/ide/pci/pdc202xx_new.c |9 - drivers/ide/pci/pdc202xx_old.c |7 +++ drivers/ide/pci/piix.c | 73 + drivers/ide/pci/sis5513.c |5 ++ include/linux/ide.h|1 9 files changed, 86 insertions(+), 59 deletions(-) Index: b/drivers/ide/ide.c === --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h hwif->bus_state = BUSSTATE_ON; hwif->atapi_dma = 0;/* disable all atapi dma */ - hwif->ultra_mask = 0x80;/* disable all ultra */ - hwif->mwdma_mask = 0x80;/* disable all mwdma */ - hwif->swdma_mask = 0x80;/* disable all swdma */ init_completion(&hwif->gendev_rel_comp); Index: b/drivers/ide/pci/aec62xx.c === --- a/drivers/ide/pci/aec62xx.c +++ b/drivers/ide/pci/aec62xx.c @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) { + struct pci_dev *dev = hwif->pci_dev; + hwif->autodma = 0; hwif->tuneproc = &aec62xx_tune_drive; hwif->speedproc = &aec62xx_tune_chipset; - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) hwif->serialized = hwif->channel; if (hwif->mate) @@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx( return; } - hwif->ultra_mask = 0x7f; + hwif->ultra_mask = hwif->cds->udma_mask; + + /* atp865 and atp865r */ + if (hwif->ultra_mask == 0x3f) { + /* check bit 0x10 of DMA status register */ + if (inb(pci_resource_start(dev, 4) + 2) & 0x10) + hwif->ultra_mask = 0x7f; /* udma0-6 */ + } + hwif->mwdma_mask = 0x07; hwif->swdma_mask = 0x07; @@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x07, /* udma0-2 */ },{ /* 1 */ .name = "AEC6260", .init_setup = init_setup_aec62xx, @@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= NOAUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 2 */ .name = "AEC6260R", .init_setup = init_setup_aec62xx, @@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = NEVER_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 3 */ .name = "AEC6X80", .init_setup = init_setup_aec6x80, @@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= AUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ },{ /* 4 */ .name = "AEC6X80R", .init_setup = init_setup_aec6x80, @@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ } }; Index: b/drivers/ide/pci/ali
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Bartlomiej Zolnierkiewicz wrote: Index: b/drivers/ide/pci/cmd64x.c === --- a/drivers/ide/pci/cmd64x.c +++ b/drivers/ide/pci/cmd64x.c @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i hwif->swdma_mask = 0x07; if (dev->device == PCI_DEVICE_ID_CMD_643) - hwif->ultra_mask = 0x80; + hwif->ultra_mask = 0x00; if (dev->device == PCI_DEVICE_ID_CMD_646) - hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80; + hwif->ultra_mask = + (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00; if (dev->device == PCI_DEVICE_ID_CMD_648) hwif->ultra_mask = 0x1f; Hm, well, this doesn't look consistent with the changes in other drivers. This driver asks for explicit hwif->cds->ultra_mask initializers, IMO... You'd only have to check for PCI-646 revisions < 5 then... reworked Thanks. :-) Index: b/drivers/ide/pci/piix.c === --- a/drivers/ide/pci/piix.c +++ b/drivers/ide/pci/piix.c default: if (!hwif->udma_four) hwif->udma_four = piix_cable_detect(hwif); This one also certainly asks for explicit hwif->cds->ultra_mask initializers... Thus almost all of this switch statement could go away... Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away Why? Could add another argument to that macro... (=> a lot of duplicated code)... could be done in the future... Yes, of course. Thanks, Bart MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
[PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, cmd64x, pdc202xx_new and pdc202xx_old drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) v2: * piix: fix cable detection for 82801AA_1 and 82372FB_1 [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * cmd64x: use hwif->cds->udma_mask [ Suggested by Sergei Shtylyov <[EMAIL PROTECTED]>. ] * aec62xx: fix newly introduced bug - check DMA status not command register [ Noticed by Sergei Shtylyov <[EMAIL PROTECTED]>. ] Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> --- drivers/ide/ide.c |3 --- drivers/ide/pci/aec62xx.c | 19 +-- drivers/ide/pci/alim15x3.c | 13 +++-- drivers/ide/pci/cmd64x.c | 15 --- drivers/ide/pci/pdc202xx_new.c |9 - drivers/ide/pci/pdc202xx_old.c |7 ++- drivers/ide/pci/piix.c |6 +- drivers/ide/pci/sis5513.c |5 - include/linux/ide.h|1 + 9 files changed, 60 insertions(+), 18 deletions(-) Index: b/drivers/ide/ide.c === --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h hwif->bus_state = BUSSTATE_ON; hwif->atapi_dma = 0;/* disable all atapi dma */ - hwif->ultra_mask = 0x80;/* disable all ultra */ - hwif->mwdma_mask = 0x80;/* disable all mwdma */ - hwif->swdma_mask = 0x80;/* disable all swdma */ init_completion(&hwif->gendev_rel_comp); Index: b/drivers/ide/pci/aec62xx.c === --- a/drivers/ide/pci/aec62xx.c +++ b/drivers/ide/pci/aec62xx.c @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) { + struct pci_dev *dev = hwif->pci_dev; + hwif->autodma = 0; hwif->tuneproc = &aec62xx_tune_drive; hwif->speedproc = &aec62xx_tune_chipset; - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) hwif->serialized = hwif->channel; if (hwif->mate) @@ -286,7 +288,15 @@ static void __devinit init_hwif_aec62xx( return; } - hwif->ultra_mask = 0x7f; + hwif->ultra_mask = hwif->cds->udma_mask; + + /* atp865 and atp865r */ + if (hwif->ultra_mask == 0x3f) { + /* check bit 0x10 of DMA status register */ + if (inb(pci_resource_start(dev, 4) + 2) & 0x10) + hwif->ultra_mask = 0x7f; /* udma0-6 */ + } + hwif->mwdma_mask = 0x07; hwif->swdma_mask = 0x07; @@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x07, /* udma0-2 */ },{ /* 1 */ .name = "AEC6260", .init_setup = init_setup_aec62xx, @@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= NOAUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 2 */ .name = "AEC6260R", .init_setup = init_setup_aec62xx, @@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = NEVER_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 3 */ .name = "AEC6X80", .init_setup = init_setup_aec6x80, @@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= AUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ },{ /* 4 */ .name = "AEC6X80R", .init_setup = init_setup_aec6x80, @@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ } }; Index: b/drivers/ide/pci/alim15x3.c === --- a/drivers/i
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Sergei Shtylyov wrote: > > Hello. > > Bartlomiej Zolnierkiewicz wrote: > >> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks > >> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask >> * add udma_mask field to ide_pci_device_t and use it to initialize >> ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers >> * fix UDMA masks to match with chipset specific *_ratemask() >> (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask >>filtering method - done in the next patch) > >More issues with aec62xx.c driver, found after looking at the next > patch... > >> Index: b/drivers/ide/pci/aec62xx.c >> === >> --- a/drivers/ide/pci/aec62xx.c >> +++ b/drivers/ide/pci/aec62xx.c >> @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips >> >> static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) >> { >> + struct pci_dev *dev = hwif->pci_dev; >> + >> hwif->autodma = 0; >> hwif->tuneproc = &aec62xx_tune_drive; >> hwif->speedproc = &aec62xx_tune_chipset; >> >> - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) >> + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) >> hwif->serialized = hwif->channel; >> >> if (hwif->mate) >> @@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx( >> return; >> } >> >> - hwif->ultra_mask = 0x7f; >> + hwif->ultra_mask = hwif->cds->udma_mask; >> + >> + /* atp865 and atp865r */ >> + if (hwif->ultra_mask == 0x3f) { >> + unsigned long io = pci_resource_start(dev, 4); >> + >> + if (inb(io) & 0x10) >> + hwif->ultra_mask = 0x7f; /* udma0-6 */ >> + } >> + > >Looks like another intruduced buglet: you're reading DMA command, but > aec62xx_ratemask() was reading DMA status originally for this bit. Doh, fixed. Thanks for catching this. >> hwif->mwdma_mask = 0x07; >> hwif->swdma_mask = 0x07; > >Hm, caught another nit: this driver doesn't actually support single-word > DMA modes... :-) added to TODO >> Index: b/drivers/ide/pci/alim15x3.c >> === >> --- a/drivers/ide/pci/alim15x3.c >> +++ b/drivers/ide/pci/alim15x3.c >> @@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a >> >> hwif->atapi_dma = 1; >> >> - if (m5229_revision > 0x20) >> - hwif->ultra_mask = 0x7f; >> + if (m5229_revision <= 0x20) >> + hwif->ultra_mask = 0x00; /* no udma */ >> + else if (m5229_revision < 0xC2) >> + hwif->ultra_mask = 0x07; /* udma0-2 */ >> + else if (m5229_revision == 0xC2 || m5229_revision == 0xC3) >> + hwif->ultra_mask = 0x1f; /* udma0-4 */ >> + else if (m5229_revision == 0xC4) >> + hwif->ultra_mask = 0x3f; /* udma0-5 */ >> + else >> + hwif->ultra_mask = 0x7f; /* udma0-6 */ >> + >> hwif->mwdma_mask = 0x07; >> hwif->swdma_mask = 0x07; > >Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... > And PIO setting via speedproc() method is broken -- it passes to tuneproc() > method mode number not biased by -XFER_PIO_0 beforehand. Will cook up some > patch, maybe... :-/ Please do, I added this to IDE TODO to not forget about the issue... Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hi, Sergei Shtylyov wrote: > > Hello. > > Bartlomiej Zolnierkiewicz wrote: > >> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks > >> * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask >> * add udma_mask field to ide_pci_device_t and use it to initialize >> ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers >> * fix UDMA masks to match with chipset specific *_ratemask() >> (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask >>filtering method - done in the next patch) > >More nit picking (-: > >> Index: b/drivers/ide/pci/cmd64x.c >> === >> --- a/drivers/ide/pci/cmd64x.c >> +++ b/drivers/ide/pci/cmd64x.c >> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i >> hwif->swdma_mask = 0x07; >> >> if (dev->device == PCI_DEVICE_ID_CMD_643) >> - hwif->ultra_mask = 0x80; >> + hwif->ultra_mask = 0x00; >> if (dev->device == PCI_DEVICE_ID_CMD_646) >> - hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80; >> + hwif->ultra_mask = >> + (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00; >> if (dev->device == PCI_DEVICE_ID_CMD_648) >> hwif->ultra_mask = 0x1f; > >Hm, well, this doesn't look consistent with the changes in other drivers. > This driver asks for explicit hwif->cds->ultra_mask initializers, IMO... >You'd only have to check for PCI-646 revisions < 5 then... reworked >> Index: b/drivers/ide/pci/piix.c >> === >> --- a/drivers/ide/pci/piix.c >> +++ b/drivers/ide/pci/piix.c >> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide >> case PCI_DEVICE_ID_INTEL_82371FB_0: >> case PCI_DEVICE_ID_INTEL_82371FB_1: >> case PCI_DEVICE_ID_INTEL_82371SB_1: >> - hwif->ultra_mask = 0x80; >> + hwif->ultra_mask = 0x00; >> break; >> case PCI_DEVICE_ID_INTEL_82371AB: >> case PCI_DEVICE_ID_INTEL_82443MX_1: >> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide >> case PCI_DEVICE_ID_INTEL_82801AB_1: >> hwif->ultra_mask = 0x07; >> break; >> + case PCI_DEVICE_ID_INTEL_82801AA_1: >> + case PCI_DEVICE_ID_INTEL_82372FB_1: >> + hwif->ultra_mask = 0x1f; >> + break; > >Alas, I'm afraid this part is wrong! >At least, the cable detection should work for 82801AA the same way as for > the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think > we should fall thru here. yes (extra "break:" shouldn't be there), fixed >> default: >> if (!hwif->udma_four) >> hwif->udma_four = piix_cable_detect(hwif); > >This one also certainly asks for explicit hwif->cds->ultra_mask > initializers... Thus almost all of this switch statement could go away... Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away (=> a lot of duplicated code)... could be done in the future... Thanks, Bart - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
> should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light > on how it should be calculated)... Well, this seems fixed in libata drivers. The libata code for tuning is based upon the BIOS programmers guide. That seemed to be the best coverage of a minimal selection. It's also got a big "confidential" stamp on it so I can't pass it on. Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Alan wrote: Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver I've looked thru the specs and it seemed to me that ULi hardware is much broken PIO wise: their max active time is 8 cycles even on taskfile access which gives 240 ns while standard requeires 290 ns for modes 0 thru 2... I've also noted that the tuneproc() method in both cmd64x.c and alim15x3.c seems to misdo recovery calculation, taking address setup into account -- that should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light on how it should be calculated)... Well, this seems fixed in libata drivers. Alan MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Sergei Shtylyov wrote: Not a suprise to be honest. I fixed some of the ALi stuff when I did it and I think that was pushed back into drivers/ide. The CMD64x hasn't had much love really. Another buglet found by random glancing at this driver: /** * cmd648_dma_stop - DMA stop callback * @qc: Command in progress * * DMA has completed. */ static void cmd648_bmdma_stop(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; struct pci_dev *pdev = to_pci_dev(ap->host->dev); u8 dma_intr; int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0; int dma_mask = ap->port_no ? ARTTIM2 : CFR; ata_bmdma_stop(qc); pci_read_config_byte(pdev, dma_reg, &dma_intr); pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask); } dma_reg and dma_mask initializers must have been swapped since ARTTIM2 and CFR are regster names. So, the code reads/writes semi-random regs... BTW, on PCI0646U2 and later chips, the interrupt status (it's not really DMA interrupt status but a latched INTRQ signal not "coupled" with DMA logic, according to the datasheets) can be read from MRDMODE reg. which is accessible in I/O space at BMIDE base + 1 which is certainly faster. That's what drivers/ide/cmd64x.c is doing in its test_dma_irq() method (however, it's doign this on PCI0643 and early revs of PCI0646 which don't have these bits). The driver's dma_end() method is acting really strange: it checks if the cjip is PCI-648/9 and reads the PCI config space to clear those interrupt bits while these chips have them in I/O mapped MRDMODE; OTOH, it ignores these bits on earlier chips which have them in oonfig. space only (CFR/ARTTIM23 regs)... go figure. I'm going to clean this up but don't heve the h/w handy... :-/ Wouldn't mind the older 64x (not 640) data sheets if they are sharable. Sent what I had on this machine. Will looks for newer revision of PCJ0646U2 spec elsewhere... Sent rev. 1.3... Hopefully gkernel.sourceforge.net/specs/ will be updated. MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Alan wrote: Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code from the IDE driver. No way it could be working. You may check against the PC64x datasheets (if you have them -- if you don't I think I may share) and see for yourself -- it's abolutely idiotic. I.e. MWDMA2 should be working due to the way the driver is written (it sets up PIO4 timings when auto-tuning DMA) but not the other modes since speedproc() method is brain-damaged in this part. Not a suprise to be honest. I fixed some of the ALi stuff when I did it and I think that was pushed back into drivers/ide. The CMD64x hasn't had much love really. Another buglet found by random glancing at this driver: /** * cmd648_dma_stop - DMA stop callback * @qc: Command in progress * * DMA has completed. */ static void cmd648_bmdma_stop(struct ata_queued_cmd *qc) { struct ata_port *ap = qc->ap; struct pci_dev *pdev = to_pci_dev(ap->host->dev); u8 dma_intr; int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0; int dma_mask = ap->port_no ? ARTTIM2 : CFR; ata_bmdma_stop(qc); pci_read_config_byte(pdev, dma_reg, &dma_intr); pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask); } dma_reg and dma_mask initializers must have been swapped since ARTTIM2 and CFR are regster names. So, the code reads/writes semi-random regs... Wouldn't mind the older 64x (not 640) data sheets if they are sharable. Sent what I had on this machine. Will looks for newer revision of PCJ0646U2 spec elsewhere... MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Alan wrote: Wouldn't mind the older 64x (not 640) data sheets if they are sharable. If they are sharable, I would love to archive them at http://gkernel.sourceforge.net/specs/ Regards, Jeff - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
> >>Ugh, I'm not seeing any *actual* support for MW/SW DMA in this > >> driver... > > > Thats long been broken. Should be correct in the libata driver > > Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code > from the IDE driver. No way it could be working. You may check against the > PC64x datasheets (if you have them -- if you don't I think I may share) and > see for yourself -- it's abolutely idiotic. Not a suprise to be honest. I fixed some of the ALi stuff when I did it and I think that was pushed back into drivers/ide. The CMD64x hasn't had much love really. Wouldn't mind the older 64x (not 640) data sheets if they are sharable. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Alan wrote: Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code from the IDE driver. No way it could be working. You may check against the PC64x datasheets (if you have them -- if you don't I think I may share) and see for yourself -- it's abolutely idiotic. Alan WBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
On Mon, 22 Jan 2007 21:17:33 +0300 > Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... Thats long been broken. Should be correct in the libata driver Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Bartlomiej Zolnierkiewicz wrote: [PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) More issues with aec62xx.c driver, found after looking at the next patch... Index: b/drivers/ide/pci/aec62xx.c === --- a/drivers/ide/pci/aec62xx.c +++ b/drivers/ide/pci/aec62xx.c @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) { + struct pci_dev *dev = hwif->pci_dev; + hwif->autodma = 0; hwif->tuneproc = &aec62xx_tune_drive; hwif->speedproc = &aec62xx_tune_chipset; - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) hwif->serialized = hwif->channel; if (hwif->mate) @@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx( return; } - hwif->ultra_mask = 0x7f; + hwif->ultra_mask = hwif->cds->udma_mask; + + /* atp865 and atp865r */ + if (hwif->ultra_mask == 0x3f) { + unsigned long io = pci_resource_start(dev, 4); + + if (inb(io) & 0x10) + hwif->ultra_mask = 0x7f; /* udma0-6 */ + } + Looks like another intruduced buglet: you're reading DMA command, but aec62xx_ratemask() was reading DMA status originally for this bit. hwif->mwdma_mask = 0x07; hwif->swdma_mask = 0x07; Hm, caught another nit: this driver doesn't actually support single-word DMA modes... :-) Index: b/drivers/ide/pci/alim15x3.c === --- a/drivers/ide/pci/alim15x3.c +++ b/drivers/ide/pci/alim15x3.c @@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a hwif->atapi_dma = 1; - if (m5229_revision > 0x20) - hwif->ultra_mask = 0x7f; + if (m5229_revision <= 0x20) + hwif->ultra_mask = 0x00; /* no udma */ + else if (m5229_revision < 0xC2) + hwif->ultra_mask = 0x07; /* udma0-2 */ + else if (m5229_revision == 0xC2 || m5229_revision == 0xC3) + hwif->ultra_mask = 0x1f; /* udma0-4 */ + else if (m5229_revision == 0xC4) + hwif->ultra_mask = 0x3f; /* udma0-5 */ + else + hwif->ultra_mask = 0x7f; /* udma0-6 */ + hwif->mwdma_mask = 0x07; hwif->swdma_mask = 0x07; Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... And PIO setting via speedproc() method is broken -- it passes to tuneproc() method mode number not biased by -XFER_PIO_0 beforehand. Will cook up some patch, maybe... :-/ MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello. Bartlomiej Zolnierkiewicz wrote: [PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) More nit picking (-: Index: b/drivers/ide/pci/cmd64x.c === --- a/drivers/ide/pci/cmd64x.c +++ b/drivers/ide/pci/cmd64x.c @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i hwif->swdma_mask = 0x07; if (dev->device == PCI_DEVICE_ID_CMD_643) - hwif->ultra_mask = 0x80; + hwif->ultra_mask = 0x00; if (dev->device == PCI_DEVICE_ID_CMD_646) - hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80; + hwif->ultra_mask = + (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00; if (dev->device == PCI_DEVICE_ID_CMD_648) hwif->ultra_mask = 0x1f; Hm, well, this doesn't look consistent with the changes in other drivers. This driver asks for explicit hwif->cds->ultra_mask initializers, IMO... You'd only have to check for PCI-646 revisions < 5 then... Index: b/drivers/ide/pci/piix.c === --- a/drivers/ide/pci/piix.c +++ b/drivers/ide/pci/piix.c @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide case PCI_DEVICE_ID_INTEL_82371FB_0: case PCI_DEVICE_ID_INTEL_82371FB_1: case PCI_DEVICE_ID_INTEL_82371SB_1: - hwif->ultra_mask = 0x80; + hwif->ultra_mask = 0x00; break; case PCI_DEVICE_ID_INTEL_82371AB: case PCI_DEVICE_ID_INTEL_82443MX_1: @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide case PCI_DEVICE_ID_INTEL_82801AB_1: hwif->ultra_mask = 0x07; break; + case PCI_DEVICE_ID_INTEL_82801AA_1: + case PCI_DEVICE_ID_INTEL_82372FB_1: + hwif->ultra_mask = 0x1f; + break; Alas, I'm afraid this part is wrong! At least, the cable detection should work for 82801AA the same way as for the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think we should fall thru here. default: if (!hwif->udma_four) hwif->udma_four = piix_cable_detect(hwif); This one also certainly asks for explicit hwif->cds->ultra_mask initializers... Thus almost all of this switch statement could go away... MBR, Sergei - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
[PATCH] ide: fix UDMA/MWDMA/SWDMA masks * use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask * add udma_mask field to ide_pci_device_t and use it to initialize ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers * fix UDMA masks to match with chipset specific *_ratemask() (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask filtering method - done in the next patch) Signed-off-by: Bartlomiej Zolnierkiewicz <[EMAIL PROTECTED]> --- drivers/ide/ide.c |3 --- drivers/ide/pci/aec62xx.c | 20 ++-- drivers/ide/pci/alim15x3.c | 13 +++-- drivers/ide/pci/cmd64x.c |5 +++-- drivers/ide/pci/pdc202xx_new.c |9 - drivers/ide/pci/pdc202xx_old.c |7 ++- drivers/ide/pci/piix.c |6 +- drivers/ide/pci/sis5513.c |5 - include/linux/ide.h|1 + 9 files changed, 56 insertions(+), 13 deletions(-) Index: b/drivers/ide/ide.c === --- a/drivers/ide/ide.c +++ b/drivers/ide/ide.c @@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h hwif->bus_state = BUSSTATE_ON; hwif->atapi_dma = 0;/* disable all atapi dma */ - hwif->ultra_mask = 0x80;/* disable all ultra */ - hwif->mwdma_mask = 0x80;/* disable all mwdma */ - hwif->swdma_mask = 0x80;/* disable all swdma */ init_completion(&hwif->gendev_rel_comp); Index: b/drivers/ide/pci/aec62xx.c === --- a/drivers/ide/pci/aec62xx.c +++ b/drivers/ide/pci/aec62xx.c @@ -270,11 +270,13 @@ static unsigned int __devinit init_chips static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif) { + struct pci_dev *dev = hwif->pci_dev; + hwif->autodma = 0; hwif->tuneproc = &aec62xx_tune_drive; hwif->speedproc = &aec62xx_tune_chipset; - if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) + if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF) hwif->serialized = hwif->channel; if (hwif->mate) @@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx( return; } - hwif->ultra_mask = 0x7f; + hwif->ultra_mask = hwif->cds->udma_mask; + + /* atp865 and atp865r */ + if (hwif->ultra_mask == 0x3f) { + unsigned long io = pci_resource_start(dev, 4); + + if (inb(io) & 0x10) + hwif->ultra_mask = 0x7f; /* udma0-6 */ + } + hwif->mwdma_mask = 0x07; hwif->swdma_mask = 0x07; @@ -354,6 +365,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x07, /* udma0-2 */ },{ /* 1 */ .name = "AEC6260", .init_setup = init_setup_aec62xx, @@ -363,6 +375,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= NOAUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 2 */ .name = "AEC6260R", .init_setup = init_setup_aec62xx, @@ -373,6 +386,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = NEVER_BOARD, + .udma_mask = 0x1f, /* udma0-4 */ },{ /* 3 */ .name = "AEC6X80", .init_setup = init_setup_aec6x80, @@ -382,6 +396,7 @@ static ide_pci_device_t aec62xx_chipsets .channels = 2, .autodma= AUTODMA, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ },{ /* 4 */ .name = "AEC6X80R", .init_setup = init_setup_aec6x80, @@ -392,6 +407,7 @@ static ide_pci_device_t aec62xx_chipsets .autodma= AUTODMA, .enablebits = {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}}, .bootable = OFF_BOARD, + .udma_mask = 0x3f, /* udma0-5 */ } }; Index: b/drivers/ide/pci/alim15x3.c === --- a/drivers/ide/pci/alim15x3.c +++ b/drivers/ide/pci/alim15x3.c @@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a hwif->atapi_dma = 1; - if (m5229_revision > 0x20) - hwif->ultra_mask = 0x7f; + if (m5229_revision <= 0x20) + hwif->ultra_mask = 0x00; /* no udma */ + else if (m5229_revision < 0xC2) + hwif->u