Re: [PATCH] Fix checking for multiple boot tags in board config.

[Imre Deak <imre.deak@nokia.com>]

On Fri, 2006-04-07 at 16:58 +0100, ext Jonathan McDowell wrote:
> On Fri, Apr 07, 2006 at 06:50:15PM +0300, Imre Deak wrote:
> > On Fri, 2006-04-07 at 16:00 +0100, ext Jonathan McDowell wrote:
> > > Currently if we look for more than one instance of a boot tag and it's
> > > defined in the board config rather than passed by the boot loader we can
> > > get stuck in a loop where the same tag is processed many times. This can
> > > be seen with the gpio-switch driver, for example. The patch below fixes
> > > this by respecting the skip parameter to get_config when we're scanning
> > > the board config structure.
> > > 
> > > Signed-Off-By: Jonathan McDowell <noodles@earth.li>
> > > 
> > > diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
> > > index 0625df5..39ab667 100644
> > > --- a/arch/arm/plat-omap/common.c
> > > +++ b/arch/arm/plat-omap/common.c
> > > @@ -114,8 +114,12 @@ static const void *get_config(u16 tag, s
> > >  	 * in the kernel. */
> > >  	for (i = 0; i < omap_board_config_size; i++) {
> > >  		if (omap_board_config[i].tag == tag) {
> > > -			kinfo = &omap_board_config[i];
> > > -			break;
> > > +			if (skip == 0) {
> > > +				kinfo = &omap_board_config[i];
> > > +				break;
> > > +			} else {
> > > +				skip--;
> > > +			}
> > >  		}
> > >  	}
> > >  	if (kinfo == NULL)
> > 
> > Should the case be handled when a configuration is present in both the
> > boot tag and the board config? If so, we should stop parsing after the
> > boot tags whenever the requested tag is found, even if there are less
> > then 'skip' instances of it, and return NULL.
> 
> If a config option that can appear multiple times (ie when the skip
> option would apply) is present in both the boot tag and board config I'd
> expect both to have effect. You might have some instances of a device
> that exist on all variants of the board (that would be in the board
> config) and then some additional instances that would be controlled by
> the boot tags passed by the boot loader. Though equally I can see the
> argument for the other way when you'd want to override the defaults. :)
> 

Ok, I didn't consider the semantic where we merge the two
configurations. And since it was the implied behavior so far, fixing it
with your patch seems to be the right thing.

--Imre