[Skiboot] [PATCH] capi: Keep the current mmio windows in the mbt cache table.

[Vaibhav Jain]

Hi Christophe,

The flow looks good to me but some minor comments regarding code
re-organization.

Christophe Lombard <clombard at linux.vnet.ibm.com> writes:

> --- a/hw/phb4.c
> +++ b/hw/phb4.c
<snip>
>  	for (i = 0; i < p->mbt_size; i++) {
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][0]);
> -		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[i][1]);
> +		/* search if the capi mmio window is already present */
> +		if ((p->mbt_cache[i][0] == mbt_0) &&
> +		    (p->mbt_cache[i][1] == mbt_1)) {
> +			 entc = i;
> +			 break;
> +		}
> +		/* search a free entry */
> +		if ((p->mbt_cache[i][0] == 0) &&
> +		    (p->mbt_cache[i][1] == 0)) {
> +			if (entf == -1)
> +				entf = i;
> +		}
'if (entf == -1 && (p->mbt_cache[i][0] == 0) && (p->mbt_cache[i][1] == 0))'

> +	}
> +
> +	if ((entc == -1) && (entf == -1)) {
> +		/* this case should never happen */
> +		PHBERR(p, "CAPP: Failed to add CAPI mmio window\n");
> +	} else if (entc == -1) {
> +		/* no capi mmio window found, so add it */
> +		p->mbt_cache[entf][0] = mbt_0;
> +		p->mbt_cache[entf][1] = mbt_1;
> +
> +		phb4_ioda_sel(p, IODA3_TBL_MBT, 0, true);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][0]);
> +		out_be64(p->regs + PHB_IODA_DATA0, p->mbt_cache[entf][1]);
>  	}

You can get rid of variable 'entc' entirely as its value can be drived from
loop index 'i'. This should also simplify the loop and the 'if-else' ladder
afterwards:

if (entf >= 0 && i == p->mbt_size) /* Free entry */
else if (i == p->mbt_size) /* mbt cache full */
else /* duplicate entry */

or

if (i < p->mbt_size) /* duplicate entry */
else if (entf >= 0) /* Free Entry */
else /* mbt cache full */

-- 
Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.