Re: Fwd: [PATCH v4 11/18] cxl: Separate bare-metal fields in adapter and AFU data structures

[Manoj Kumar <manoj@linux.vnet.ibm.com>]

Christophe, Fred: Perhaps none of these comments below are specific
to your patch, but clarification would help the next reviewer.

--
Manoj Kumar

> Subject: [PATCH v4 11/18] cxl: Separate bare-metal fields in adapter and

>
> -    WARN_ON(afu->spa_size > 0x100000); /* Max size supported by the
> hardware */
> +    WARN_ON(afu->native->spa_size > 0x100000); /* Max size supported by
> the hardware */

Would prefer to see a MACRO defined, instead of the literal 0x1000000


>
>       cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x0000000000000000);

Same as above.


>       p1n_base = p1_base(dev) + 0x10000 + (afu->slice * p1n_size);

Same as above.

>
> @@ -621,7 +622,7 @@ static int cxl_read_afu_descriptor(struct cxl_afu *afu)
>       afu->pp_size = AFUD_PPPSA_LEN(val) * 4096;

Both val and pp_size are 64bit quantities. Not clear how the overflow
during multiplication is going to be handled.


>       afu->crs_len = AFUD_CR_LEN(val) * 256;

What do the 4096 and 256 represent?


>       /* Convert everything to bytes, because there is NO WAY I'd look
> at the
>        * code a month later and forget what units these are in ;-) */
> -    adapter->ps_off = ps_off * 64 * 1024;
> +    adapter->native->ps_off = ps_off * 64 * 1024;
>       adapter->ps_size = ps_size * 64 * 1024;
> -    adapter->afu_desc_off = afu_desc_off * 64 * 1024;
> -    adapter->afu_desc_size = afu_desc_size *64 * 1024;
> +    adapter->native->afu_desc_off = afu_desc_off * 64 * 1024;
> +    adapter->native->afu_desc_size = afu_desc_size * 64 * 1024;

Is this (64k) page size related?