Recursion in CFI driver. Is it really necessary to have.

[Nicolas Pitre]

On Mon, 15 Oct 2007, Alexey Korolev wrote:

> Hi,
> 
> > 
> > Wow.  That's certainly bad.
> > 
> > It's been a while that I wrote that code, but looking at it now I just
> > can't see how that could happen.  It is like if it was recursing on
> > itself, but the code explicitly guards against that.  See:
> > 
> > 		if (contender && contender != chip) {
> > 			[...]
> > 			ret = get_chip(map, contender, contender->start,
> > mode);
> > 
> > So from that point, 'chip' and 'contender' must be equal when get_chip()
> > is called again, and therefore recursion should stop there.
> > 
> I think it has a hole here. The issue takes place if we have three or more
> processes wishing to own chip.

Right.

> The most important here, that from point of silicon state machine design view
> there is no need to have recursion deeper than "level 1".

Exact.

> I've made the patch which heals the deadlock problem. Could you please look at
> this?

One issue: you now force the resettime behavior for every cases.  Should 
we care?

Also some style issues:

> @@ -709,6 +709,23 @@
>  		spin_unlock(&shared->lock);
>  	}
> 
> +	ret = chip_ready(map, chip, adr, mode);
> +	if (ret == -EAGAIN)
> +		goto retry;
> +
> +
> +	return ret;
> +}

One blank line too many above.

> +
> +static int chip_ready (struct map_info *map, struct flchip *chip, unsigned
> long adr, int mode)
> +{

You'll have to add a prototype for that function which is called before 
its definition.


Nicolas