Hello, I wrote:
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -688,8 +688,7 @@ int ide_driveid_update(ide_drive_t *driv
*/
SELECT_MASK(drive, 1);
- if (IDE_CONTROL_REG)
- hwif->OUTB(drive->ctl,IDE_CONTROL_REG);
+ ide_set_irq(drive, 1);
If we're going to execute the command using polling, isn't it
logical to *disable* drive's interrupt instead of enabling it which
this code is currently doing? This looks like it might work only
for the drivers having the maskproc() method (of which hpt366.c is
the only one that I've ever dealt with).
Yes, this needs fixing.
But should we honor drive->quirk_list here? What its different
values mean? I'm seeing either 1 or 2 is used to decide whether to
set nIEN or not...
I did some research on ->quirk_list in the past but I couldn't exactly
figure it out. I also wasn't able to trace this code to its author
(not that I tried very hard)...
Anyway my current findings/theories are the following:
- we should make sure that all SELECT_MASK() users also call ide_set_irq()
(then sgiioc4's ->maskproc can be removed since it is a copy of ->set_irq)
I've kind of made sure about that already,
After another look, I've indeed correctly described this as "kind of" --
since ide_set_irq() is called only before issuing a command, while
SELECT_MASK() is called both before (but it seems to be always skipped due to
IDE_TFLAG_NO_SELECT_MASK being set for the commands executed in a normal way,
i.e. using interrupts) and after that (the latter is done if we are using the
polling mode). So, those maskproc() methods in hpt366 and sgiioc4 in fact
inadvertently clear nIEN after the polled command execution -- not that it's
wrong but why the hell? And those maskproc() methods duplicate nIEN setup
before the command execution -- except in ide_config_drive_speed() where
there's an obvious error...
hence I've already created a patch doing that...
As I have no idea why sgiioc4 needs to manipulate nIEN, I guess I'll just
CC the SGI people on that patch...
Thanks,
Bart
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
