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

2016-02-22 Thread Manoj Kumar

On 2/22/2016 11:57 AM, Frederic Barrat wrote:

Manoj,

Point taken. Those constants are all defined in the architecture
document (CAIA). We should probably use more macros there.
However, since those were not introduced by this patch, I'll put it in
my todo list for the future, but don't intend to address it in this
patchset.

   Fred


Fred:

I am fine with this approach.

--
Manoj Kumar

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2016-02-22 Thread Frederic Barrat

Manoj,

Point taken. Those constants are all defined in the architecture 
document (CAIA). We should probably use more macros there.
However, since those were not introduced by this patch, I'll put it in 
my todo list for the future, but don't intend to address it in this 
patchset.


  Fred

Le 22/02/2016 02:14, Manoj Kumar a écrit :

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 > 0x10); /* Max size supported by the
hardware */
+WARN_ON(afu->native->spa_size > 0x10); /* Max size supported by
the hardware */


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




  cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);


Same as above.



  p1n_base = p1_base(dev) + 0x1 + (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?




___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2016-02-21 Thread Manoj Kumar

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 > 0x10); /* Max size supported by the
hardware */
+WARN_ON(afu->native->spa_size > 0x10); /* Max size supported by
the hardware */


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




  cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);


Same as above.



  p1n_base = p1_base(dev) + 0x1 + (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?


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev