[Skiboot] [PATCH V2 2/6] phb3: Load CAPP ucode to both CAPP units on Naples

[Stewart Smith]

Philippe Bergheaud <felix at linux.vnet.ibm.com> writes:
> diff --git a/hw/phb3.c b/hw/phb3.c
> index adff5bc..087f3e0 100644
> --- a/hw/phb3.c
> +++ b/hw/phb3.c
> @@ -2314,6 +2314,10 @@ static struct {
>
>  #define CAPP_UCODE_MAX_SIZE 0x20000
>
> +#define CAPP_UCODE_LOADED(chip, p) \
> +	((p->rev < PHB3_REV_NAPLES_DD10) ? chip->capp_ucode_loaded : \
> +	 (chip->capp_ucode_loaded & (1 << p->index)))
> +

Why not just treat it as a bitmap for everything? Sure, for not Naples,
it'll just be one bit, but that's okay and probably simpler to
understand.

Also, are we *sure* that rev < PHB3_REV_NAPLES_DD10 is going to always
be true? Do we have docs somewhere saying how the revision numbers are
allocated? I couldn't find it from a quick look at docs...

> @@ -2346,12 +2350,18 @@ static int64_t capp_load_ucode(struct phb3 *p)
>  	struct capp_ucode_data *data;
>  	struct capp_lid_hdr *lid;
>  	uint64_t rc, val, addr;
> -	uint32_t chunk_count, offset;
> +	uint32_t chunk_count, offset, reg_offset;
>  	int i;
>
> -	if (chip->capp_ucode_loaded)
> +	if (CAPP_UCODE_LOADED(chip, p))
>  		return OPAL_SUCCESS;
>
> +	/* Return if PHB not attached to a CAPP unit */
> +	if (p->rev == PHB3_REV_NAPLES_DD10 && p->index > 1)
> +		return OPAL_HARDWARE;
> +	if (p->index > 2)
> +		return OPAL_HARDWARE;

Should we have a MAX_PHBs define somewhere?


-- 
Stewart Smith
OPAL Architect, IBM.