Re: [PATCH] mtd: cfi_cmdset_0002: Support Persistent Protection Bits (PPB) locking

[Artem Bityutskiy <dedekind1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1863 bytes --]

On Mon, 2012-12-10 at 19:40 +0100, Stefan Roese wrote:
> On 12/10/2012 04:00 PM, Artem Bityutskiy wrote:
> > On Fri, 2012-12-07 at 08:22 +0100, Stefan Roese wrote:
> >> +       /*
> >> +        * Wait for some time as unlocking of all sectors takes quite long
> >> +        */
> >> +       timeo = jiffies + (2 * HZ);     /* 2s max (un)locking */
> > 
> > Please, use msecs_to_jiffies() instead.
> 
> Sure, thats better.

Would you please do this instead?

> AFAIK, chip->mutex protects the access to the chip itself. So that
> sequences are not interrupted.
> 
> I have to admit that I haven't looked into get_chip() so far. It seems
> to handle a state machine. Normally (idle state) it will just fall
> through (FL_READY).

So it looks like the idea is that you first take the mutex, then call
get_chip() which will wait for the chip becoming really ready, and then
you can safely use it.

> 
> > Why you need to drop the mutex here?
> 
> Not sure, that might not be necessary. Copy and past from another loop
> in the same file.

Probably from 'get_chip()' ?

> > Why is it not an ABBA deadlock to do this:
> > 
> > Task 1: In the loop above, has chip locked, doing
> >         mutex_lock(&chip->mutex);
> > 
> > Task 2: done mutex_lock(&chip->mutex), now doing
> >         ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
> 
> I don't see two different locks/mutexes (only A) here. As get_chip()
> does no request any real mutex. Please correct me if I'm wrong.

Right, there is indeed no deadlock.

> In many other places UDELAY() is called:
> 
> #define UDELAY(map, chip, adr, usec)  \
> do {  \
> 	mutex_unlock(&chip->mutex);  \
> 	cfi_udelay(usec);  \
> 	mutex_lock(&chip->mutex);  \
> } while (0)

Why not to use this as well then for consistency?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss