[ECOS] I2C driver DSR locking

[ECOS] I2C driver DSR locking

[Rob Duncan]

Can someone explain to me how this code fragment from i2c_mcf52xx.c is
supposed to work?  extra->i2c_lock is a mutex to control access to the
extra->i2c_wait condition variable which is used to signal that the
extra->i2c_completed flag should be re-examined.  However
extra->i2c_completed is set true and the condition variable is
signaled by the DSR, which won't be able to run because the DSR lock
has been taken.  Isn't this liable to hang the system?  It looks to me
that locking the DSR is exactly what you don't want to do.

        cyg_drv_mutex_lock(&(extra->i2c_lock));
        cyg_drv_dsr_lock();
        while (! extra->i2c_completed) {
            cyg_drv_cond_wait(&(extra->i2c_wait));
        }
        cyg_drv_dsr_unlock();
        cyg_drv_mutex_unlock(&(extra->i2c_lock));

Thanks,

Rob.

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] I2C driver DSR locking

[Paul D. DeRocco]

> From: Rob Duncan
> 
> Can someone explain to me how this code fragment from 
> i2c_mcf52xx.c is supposed to work?  extra->i2c_lock is a 
> mutex to control access to the
> extra->i2c_wait condition variable which is used to signal that the 
> extra->i2c_completed flag should be re-examined.  However 
> i2c_completed 
> extra->is set true and the condition variable is
> signaled by the DSR, which won't be able to run because the 
> DSR lock has been taken.  Isn't this liable to hang the 
> system?  It looks to me that locking the DSR is exactly what 
> you don't want to do.
> 
>         cyg_drv_mutex_lock(&(extra->i2c_lock));
>         cyg_drv_dsr_lock();
>         while (! extra->i2c_completed) {
>             cyg_drv_cond_wait(&(extra->i2c_wait));
>         }
>         cyg_drv_dsr_unlock();
>         cyg_drv_mutex_unlock(&(extra->i2c_lock));

cyg_drv_cond_wait releases and reacquires the DSR lock, just like it
releases and reacquires the mutex. This allows the routine to coexist not
only with other threads, but with the DSR. You see the same pattern in most
drivers.

-- 

Ciao,               Paul D. DeRocco
Paul                mailto:pderocco@ix.netcom.com


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] I2C driver DSR locking

[Rob Duncan]

Ah!  That's the missing piece of information.  I'd assumed I knew how
cyg_drv_cond_wait() worked, but now that you've prompted me to check
the documentation I see I was wrong :-)

Thanks very much,

Rob.

On 4/3/07, Paul D. DeRocco <pderocco@ix.netcom.com> wrote:
> > From: Rob Duncan
> >
> > Can someone explain to me how this code fragment from
> > i2c_mcf52xx.c is supposed to work?  extra->i2c_lock is a
> > mutex to control access to the
> > extra->i2c_wait condition variable which is used to signal that the
> > extra->i2c_completed flag should be re-examined.  However
> > i2c_completed
> > extra->is set true and the condition variable is
> > signaled by the DSR, which won't be able to run because the
> > DSR lock has been taken.  Isn't this liable to hang the
> > system?  It looks to me that locking the DSR is exactly what
> > you don't want to do.
> >
> >         cyg_drv_mutex_lock(&(extra->i2c_lock));
> >         cyg_drv_dsr_lock();
> >         while (! extra->i2c_completed) {
> >             cyg_drv_cond_wait(&(extra->i2c_wait));
> >         }
> >         cyg_drv_dsr_unlock();
> >         cyg_drv_mutex_unlock(&(extra->i2c_lock));
>
> cyg_drv_cond_wait releases and reacquires the DSR lock, just like it
> releases and reacquires the mutex. This allows the routine to coexist not
> only with other threads, but with the DSR. You see the same pattern in most
> drivers.
>
> --
>
> Ciao,               Paul D. DeRocco
> Paul                mailto:pderocco@ix.netcom.com
>
>
> --
> Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
> and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
>
>

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] I2C driver DSR locking

[Daniel Helgason]

On Tue, 2007-04-03 at 14:28 -0700, Paul D. DeRocco wrote:
> > From: Rob Duncan
> > 
> > Can someone explain to me how this code fragment from 
> > i2c_mcf52xx.c is supposed to work?  extra->i2c_lock is a 
> > mutex to control access to the
> > extra->i2c_wait condition variable which is used to signal that the 
> > extra->i2c_completed flag should be re-examined.  However 
> > i2c_completed 
> > extra->is set true and the condition variable is
> > signaled by the DSR, which won't be able to run because the 
> > DSR lock has been taken.  Isn't this liable to hang the 
> > system?  It looks to me that locking the DSR is exactly what 
> > you don't want to do.
> > 
> >         cyg_drv_mutex_lock(&(extra->i2c_lock));
> >         cyg_drv_dsr_lock();
> >         while (! extra->i2c_completed) {
> >             cyg_drv_cond_wait(&(extra->i2c_wait));
> >         }
> >         cyg_drv_dsr_unlock();
> >         cyg_drv_mutex_unlock(&(extra->i2c_lock));
> 
> cyg_drv_cond_wait releases and reacquires the DSR lock, just like it
> releases and reacquires the mutex. This allows the routine to coexist not
> only with other threads, but with the DSR. You see the same pattern in most
> drivers.
> ...

Paul!

Do you (or anyone reading this list) know if this is true for any case
where a thread blocks after taking the scheduler lock? Or is this just
special for cyg_drv_cond_wait() function?

-- 
+---------------------------------------------
| Daniel Helgason <danielh@telus.net>



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] I2C driver DSR locking

[Nick Garnett]

Daniel Helgason <danielh@telus.net> writes:

> Do you (or anyone reading this list) know if this is true for any case
> where a thread blocks after taking the scheduler lock? Or is this just
> special for cyg_drv_cond_wait() function?

It is a general feature of all thread suspension. Each thread's
scheduler lock level is essentially part of its state and is saved and
restored across all thread switches. Historically this was not always
the case, and cond_wait had special code to support this kind of
use. But it was considered so useful that I made it a generic feature
several years ago.

-- 
Nick Garnett                                     eCos Kernel Architect
eCosCentric Limited     http://www.eCosCentric.com/   The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.    Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
**  Visit us at ESC Silicon Valley <http://www.embedded.com/esc/sv> **
**  April 3-5 2007, Booth 1922, San Jose McEnery Convention Center  **


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] I2C driver DSR locking

[Paul D. DeRocco]

> From: Nick Garnett
> 
> It is a general feature of all thread suspension. Each 
> thread's scheduler lock level is essentially part of its 
> state and is saved and restored across all thread switches. 
> Historically this was not always the case, and cond_wait had 
> special code to support this kind of use. But it was 
> considered so useful that I made it a generic feature several 
> years ago.

So the implication of this is that when a thread holds the DSR lock, it
can't be pre-empted, but it can still yield or sleep of its own volition. Is
that correct?

-- 

Ciao,               Paul D. DeRocco
Paul                mailto:pderocco@ix.netcom.com


--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] I2C driver DSR locking

[Nick Garnett]

"Paul D. DeRocco" <pderocco@ix.netcom.com> writes:

> > From: Nick Garnett
> > 
> > It is a general feature of all thread suspension. Each 
> > thread's scheduler lock level is essentially part of its 
> > state and is saved and restored across all thread switches. 
> > Historically this was not always the case, and cond_wait had 
> > special code to support this kind of use. But it was 
> > considered so useful that I made it a generic feature several 
> > years ago.
> 
> So the implication of this is that when a thread holds the DSR lock, it
> can't be pre-empted, but it can still yield or sleep of its own volition. Is
> that correct?

That is correct, it seemed to be the most useful behaviour. 

-- 
Nick Garnett                                     eCos Kernel Architect
eCosCentric Limited     http://www.eCosCentric.com/   The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.    Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss