Re: PATCH: CMD640 IDE chipset

[Alexander Atanasov]

	Hello,

On Wed, 29 Oct 2003 04:12:18 -0800 (PST)
Stefan Talpalaru <stefantalpalaru@xxxxxxxxx> wrote:

> and also some useless code (the wrapers: __put_cmd640_reg() and 
> __get_cmd640_reg() - which I removed and placed the locks where
> needed; the pci_conf1() and pci_conf2() functions).

	These wrappers were added to correct the locking in
driver. There are places that you must access serveral registers
holding the lock and that's why the wrappers came, so they
are not useless. The locking in your patch is wrong -
the problem is that you call get_cmd640_reg or put_cmd640_reg, which
try to take ide_lock, while already holding it and it's a deadlock 
- thats what the wrappers solved. So, please , drop that change.


       setup_count |= __get_cmd640_reg(arttim_regs[index]) & 0x3f;
-       __put_cmd640_reg(arttim_regs[index], setup_count);
-       __put_cmd640_reg(drwtim_regs[index], pack_nibbles(active_count, recovery
_count));
+       setup_count |= get_cmd640_reg(arttim_regs[index]) & 0x3f;
+       put_cmd640_reg(arttim_regs[index], setup_count);
+       put_cmd640_reg(drwtim_regs[index], pack_nibbles(active_count, recovery_c
ount));
        spin_unlock_irqrestore(&ide_lock, flags);
 }

	here

-       __put_cmd640_reg(reg, b);
+       put_cmd640_reg(reg, b);
        spin_unlock_irqrestore(&ide_lock, flags);

	and here for example.

--
have fun,
alex
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/