Re: PATCH: CMD640 IDE chipset

[Stefan Talpalaru <stefantalpalaru@yahoo.com>]

Hello Alexander!

> 	Hello,
> On Wed, 29 Oct 2003 04:12:18 -0800 (PST)
> Stefan Talpalaru <stefantalpalaru@yahoo.com> 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

 where?

> holding the lock and that's why the wrappers came, so they
> are not useless. The locking in your patch is wrong -

  They are ugly.

> 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 

  I find it hard to believe (I'm running a patched kernel right now!),
but
 i'm only a newbie at kernel hacking, so please show me the light (not
joking).
  Thanks in advance.

> - 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

later,


=====
Stefan Talpalaru

__________________________________
Do you Yahoo!?
Protect your identity with Yahoo! Mail AddressGuard
http://antispam.yahoo.com/whatsnewfree