Re: [PATCH] pdc202xx_old: fix resetproc() method

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Bartlomiej Zolnierkiewicz wrote:

pdc202xx_reset() calls pdc202xx_reset_host() twice, for both channels, while
that function actually twiddles the single, shared software reset bit -- the
net effect is a duplicated reset and horrendous 4 second delay happening not
only on a channel reset but also when dma_lost_irq() and dma_clear() methods
are called.  Fold pdc202xx_reset_host() into pdc202xx_reset(), fix printk(),
and move it before the actual reset...

Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

---
The patch is against the ide-2.6.git master branch...

Bart, I know you have the docs... from the now removed source code I was able
to figure out that the bit in question most probably drives RST- signal on both
channels (it seems to require re-tuning drives on both channels which we are
currently not doing). If I'm right, why the hell we're twiddling it on a normal
SRST reset, and what's worse on an interrupt timeout conditions? Anyway, it's

Unfortunately the documentation doesn't say anything more than that the changing
of the bit value results in SRST on both channels...

I find it somewhat hard to believe that this bit controls SRST bits in the device control registers -- we have them both under our control anyway, why would we need such a "shortcut". I'd rather believe that this bit controls RESET- signal on both channels, with the neigboring bit 3 setting it to tristate mode the same way as it works on HPT37x (where BTW the datasheets call RESET- "SRST_") -- all this probably having to do with the hotswap support (busproc() method that manipulated bit 3 was removed from that driver by me 3 years ago)...

IIRC from some old discussions this is required by the chipset sometimes..

If that bit indeed controls SRST bits, I don't see how setting it in resetproc() helps (it can only harm, unwittingly resetting another channel). If this bit controls RESET-, I again don't see how/why it helps to hard-reset the channels in addition to the soft reset just performed. When pdc202xx_reset() is called as the dma_clear() method, the channel is probably going be reset soon due to BSY=1, and when pdc202xx_reset() is called from dma_lost_irq() method, it seems a clear overkill...
	
hardly a good idea to reset both channels when you only have problem with only
one of them, and I don't see any justification to doing that... So maybe this
patch should've actually wiped out that whole reset insanity for good?..

I'm pretty sure that there are valid reasons behind some of this insanity,
OTOH (per mail from Alan) it completely ignores the fact that the other port
may be active..

We may try removing it in incremental patch so it is easier to track/revert
it if things go wrong..

   OK, sounds like a plan...

MBR, Sergei
--
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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux