[Skiboot] [PATCH 3/6] hw/npu2: Common NPU2 init routine between NVLink and OpenCAPI

[Alistair Popple]

Couple of minor comments below, but nothing major.

Reviewed-By: Alistair Popple <alistair at popple.id.au>

<snip>

> +void probe_npu2(void)
> +{
> +	struct proc_chip *chip = next_chip(NULL);
> +	struct npu2 *npu;
> +	struct dt_node *np;
> +	const char *zcal;
> +
> +	/* Abort if we're running on DD1 */
> +	if (chip &&
> +	    (chip->type == PROC_CHIP_P9_NIMBUS ||
> +	     chip->type == PROC_CHIP_P9_CUMULUS) &&
> +	    (chip->ec_level & 0xf0) == 0x10) {
> +		prlog(PR_INFO, "NPU2: DD1 not supported\n");
> +		return;
> +	}
> +
> +	/* Check for a zcal override */
> +	zcal = nvram_query("nv_zcal_override");
> +	if (zcal) {
> +		nv_zcal_nominal = atoi(zcal);
> +		prlog(PR_WARNING, "NPU2: Using ZCAL impedance override = %d\n", nv_zcal_nominal);
> +	}
> +
> +	dt_for_each_compatible(dt_root, np, "ibm,power9-npu") {
> +	        npu = setup_npu(np);
> +		if (npu) {

Not sure if you're just being defensive but setup_npu() can never return
npu == NULL (it asserts instead) so the check is somewhat superfluous.

> +			setup_devices(npu);
> +		}
> +	}
> +}

<snip>

>  /*
> diff --git a/include/npu2.h b/include/npu2.h
> index 10742031ec0f..9fdc438bb4c1 100644
> --- a/include/npu2.h
> +++ b/include/npu2.h
> @@ -147,6 +147,7 @@ struct npu2 {
>  	uint32_t	chip_id;
>  	uint64_t	xscom_base;
>  	void		*regs;
> +	void		*ocapi_global_mmio_base;

This is only one value so at this point it doesn't matter but if we start adding
more values to struct npu2 that are exclusive to NPU2 xor OCAPI it would be nice
to have these in different structs so it's obvious which ones are
initialised/used in each case.

>  	uint64_t	mm_base;
>  	uint64_t	mm_size;
>  	uint32_t	base_lsi;
> @@ -167,6 +168,7 @@ struct npu2 {
>  
>  	/* NVLink */
>  	struct phb	phb_nvlink;
> +	uint32_t	phb_index;
>  };