ide: make ide_rate_filter() also respect PIO and SW/MW DMA mode masks (take 2)
PATCH 3/4 ide: make ide_rate_filter() also respect PIO and SW/MW DMA mode masks (take 2)
Make ide_rate_filter() also respect PIO/SWDMA/MWDMA mode masks. While at it, make the udma_filter() method calls take precedence over using the mode masks.
---
Alas, the previous version of the patch wasn't even worth signing it off -- the code had to be much trickier. The patch against the current Linus' tree and unfortunately I was able to only compile test it since that tree gives MODPOST warning and dies early on bootup. I'm hoping to give it more tesring when/if I have time... :-)
drivers/ide/ide-dma.c | 7 +++--- drivers/ide/ide-lib.c | 57 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 25 deletions(-)
drivers/ide/ide-dma.c +4 -3
@@ -664,10 +664,11 @@ static unsigned int ide_get_mode_mask(idif ((id->field_valid &4 ) ==0 )break ; - mask = id->dma_ultra & hwif->ultra_mask; -if (hwif->udma_filter) - mask &= hwif->udma_filter (drive); + mask = hwif->udma_filter (drive); +else + mask = hwif->ultra_mask; + mask &= id->dma_ultra;if ((mask &0x78 ) && (eighty_ninty_three (drive) ==0 )) mask &=0x07 ;
drivers/ide/ide-lib.c +35 -22
73 lines not on this page · raw message
Hi,
Quoted 1 line · Sergei Shtylyov
Make
ide_rate_filter()also respect PIO/SWDMA/MWDMA mode masks. While at it,
Hm, this seems to be already dealt with by:
after applying the above patch ide_rate_filter() looks like this:
/**
* ide_rate_filter - filter transfer mode * @drive: IDE device * @speed: desired speed
*
* Given the available transfer modes this function returns
* the best available speed at or below the speed requested.
*
* TODO: check device PIO capabilities
*/
u8 ide_rate_filter(ide_drive_t *drive, u8 speed)
{
ide_hwif_t *hwif = drive->hwif;
u8 mode = ide_find_dma_mode(drive, speed);
if (mode == 0) {
if (hwif->pio_mask)
mode = fls(hwif->pio_mask) - 1 + XFER_PIO_0;
else
mode = XFER_PIO_4;
}// printk("%s: mode 0x%02x, speed 0x%02x\n", __FUNCTION__, mode, speed);
return min(speed, mode); }
What is still missing from IDE tree is "respecting device PIO limits" (sorry I was quoting from memory it is TODO not FIXME [1]) which probably comes down to adding ide_get_best_pio_mode() call to ide_rate_filter() (IMO this should be done before ide-mode-limiting-fixes-for-user-requested-speed-changes.patch changes).
[1] http://www.mail-archive.com/[email protected]/msg09750.html
Ah, I see the source of the confusion now, I misunderstood your remark about ->mwdma_mask being ignored (for some reason I though that you were referring to MWDMA mode not being tried if device refuses SET_XFER command w/ UDMA).
ide_find_dma_mode() is a bit tricky because it used also by ide_max_dma_mode() but it should take care of ->mwdma_mask if ->udma_mask is zero just fine.
Quoted 1 line
make the
udma_filter()method calls take precedence over using the mode masks.
This change is OK - could you make a new patch?
Quoted 5 lines
Signed-off-by Sergei Shtylyov <[email protected]>---
Alas, the previous version of the patch wasn't even worth signing it off -- the code had to be much trickier. The patch against the current Linus' tree and
take #1 was correct - i.e. if UDMA0 is not available we want to go down to MWDMA2 and not MWDMA0 (this is what the old code was doing BTW)
Quoted 105 lines
unfortunately I was able to only compile test it since that tree gives MODPOST warning and dies early on bootup. I'm hoping to give it more tesring when/if I have time... :-)
drivers/ide/ide-dma.c | 7 +++--- drivers/ide/ide-lib.c | 57 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 25 deletions(-)Index:
linux-2.6/drivers/ide/ide-dma.c
===================================================================drivers/ide/ide-dma.c +4
-3@@ -664,10 +664,11 @@ static unsigned int ide_get_mode_mask(idif ((id->field_valid &4 ) ==0 )break ; - mask = id->dma_ultra & hwif->ultra_mask; -if (hwif->udma_filter) - mask &= hwif->udma_filter (drive); + mask = hwif->udma_filter (drive); +else + mask = hwif->ultra_mask; + mask &= id->dma_ultra;if ((mask &0x78 ) && (eighty_ninty_three (drive) ==0 )) mask &=0x07 ;Index:
linux-2.6/drivers/ide/ide-lib.c
===================================================================drivers/ide/ide-lib.c +35
-22@@ -75,38 +75,51 @@ EXPORT_SYMBOL(ide_xfer_verbose);* * Given the available transfer modes this function returns * the best available speed at or below the speed requested. -* -* FIXME: filter also PIO/SWDMA/MWDMA modes */ u8 ide_rate_filter (ide_drive_t *drive,u8 speed) { -#ifdef CONFIG_BLK_DEV_IDEDMA ide_hwif_t *hwif = drive->hwif; -u8 mask = hwif->ultra_mask, mode =XFER_MW_DMA_2 ; +u8 mask, base = speed &0xf8 , bit =1 << (speed &0x07 ); -if (hwif->udma_filter) - mask = hwif->udma_filter (drive); +switch (base) { +#ifdef CONFIG_BLK_DEV_IDEDMA +case XFER_UDMA_0 : + mask = hwif->udma_filter ? hwif->udma_filter (drive) : + hwif->ultra_mask; -/* -* TODO: speed > XFER_UDMA_2 extra check is needed to avoid false -* cable warning from eighty_ninty_three(), moving ide_rate_filter() -* calls from ->speedproc to core code will make this hack go away -*/ -if (speed >XFER_UDMA_2 ) { -if ((mask &0x78 ) && (eighty_ninty_three (drive) ==0 )) +/* +* TODO: if speed > XFER_UDMA_2, an extra check is needed to +* avoid a false cable warning from eighty_ninty_three() -- +* moving ide_rate_filter() calls from ->speedproc to core +* code will make this hack go away... +*/ +if (speed >XFER_UDMA_2 && (mask &0x78 ) && +eighty_ninty_three (drive) ==0 ) mask &=0x07 ; - } - -if (mask) - mode =fls (mask) -1 +XFER_UDMA_0 ; -// printk("%s: mode 0x%02x, speed 0x%02x\n", __FUNCTION__, mode, speed); - -return min (speed, mode); -#else /* !CONFIG_BLK_DEV_IDEDMA */ -return min (speed, (u8 )XFER_PIO_4 ); +if (mask & bit) +break ; + base =XFER_MW_DMA_0 ; +/* Fall thru */ +case XFER_MW_DMA_0 : +if (hwif->mwdma_mask & bit) +break ; + base =XFER_SW_DMA_0 ; +/* Fall thru */ +case XFER_SW_DMA_0 : +if (hwif->swdma_mask & bit) +break ; + base =XFER_PIO_0 ; +/* Fall thru */ #endif /* CONFIG_BLK_DEV_IDEDMA */ +case XFER_PIO_0 : +if (hwif->pio_mask & bit) +break ; +default : +BUG (); + } +return base | bit; }EXPORT_SYMBOL (ide_rate_filter);
Once I quothed:
Quoted 2 lines
Make
ide_rate_filter()also respect PIO/SWDMA/MWDMA mode masks. While at it, make theudma_filter()method calls take precedence over using the mode masks.
This one not looking to pretty -- I've geve some thought on how to
beautify all these switch fallthoughs but haven't got something much better then uour original approach (AKA Occam's razor :-).
Quoted 1 line
Signed-off-by Sergei Shtylyov <[email protected]>
Farewell!
Hi,
Quoted 8 lines · Sergei Shtylyov
Once I quothed:
Quoted 2 lines
Make
ide_rate_filter()also respect PIO/SWDMA/MWDMA mode masks. While at it, make theudma_filter()method calls take precedence over using the mode masks.This one not looking to pretty -- I've geve some thought on how to
beautify all these switch fallthoughs but haven't got something much better then uour original approach (AKA Occam's razor :-).
Care to recast the patch to contain only udma_filter() part?
BTW ide_rate_filter() still lacks respecting PIO limits of the device (Occam's razor doesn't apply to "PIO side" of ide_rate_filter() ;-)
Thanks,
Bart
Once I quothed:
Quoted 5 lines
Quoted 2 lines
Make
ide_rate_filter()also respect PIO/SWDMA/MWDMA mode masks. While at it, make theudma_filter()method calls take precedence over using the mode masks.This one not looking to pretty -- I've geve some thought on how to
beautify all these switch fallthoughs but haven't got something much better then uour original approach (AKA Occam's razor :-).
So, it's probably better forget about this patch. But please do take patch
4/4 to merge either before or after your "Occam's razor" -- it makes its best at masking out the MWDMA modes anyways...
Farewell!
Quoted 7 lines · Sergei Shtylyov
Once I quothed:
Quoted 4 lines
Quoted 4 lines
Make
ide_rate_filter()also respect PIO/SWDMA/MWDMA mode masks. While at it,
make theudma_filter()method calls take precedence over using the mode masks.
Quoted 7 lines
Quoted 3 lines
This one not looking to pretty -- I've geve some thought on how to
beautify all these switch fallthoughs but haven't got something much better then uour original approach (AKA Occam's razor :-).So, it's probably better forget about this patch. But please do take patch
4/4 to merge either before or after your "Occam's razor" -- it makes its best at masking out the MWDMA modes anyways...
Sure, could you re-send patch 4/4 synced against current Linus' tree?
Thanks,
Bart