RE: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment

2017-10-30 Thread Bogdan Purcareata 
> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Monday, October 30, 2017 10:56 AM
> To: Bogdan Purcareata <bogdan.purcare...@nxp.com>
> Cc: de...@driverdev.osuosl.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment
> 
> On Fri, Oct 27, 2017 at 02:44:37PM +, Bogdan Purcareata wrote:
> > > -Original Message-
> > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > > Sent: Friday, October 27, 2017 5:30 PM
> > > To: Bogdan Purcareata <bogdan.purcare...@nxp.com>
> > > Cc: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
> > > gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> > > de...@driverdev.osuosl.org
> > > Subject: Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer 
> > > alignment
> > >
> > > On Fri, Oct 27, 2017 at 02:11:35PM +, Bogdan Purcareata wrote:
> > > > @@ -93,10 +100,10 @@
> > > >   * buffers large enough to allow building an skb around them and also
> > > account
> > > >   * for alignment restrictions
> > > >   */
> > > > -#define DPAA2_ETH_BUF_RAW_SIZE \
> > > > +#define DPAA2_ETH_BUF_RAW_SIZE(priv) \
> > > > (DPAA2_ETH_RX_BUF_SIZE + \
> > > > SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> > > > -   DPAA2_ETH_RX_BUF_ALIGN)
> > > > +   (priv)->rx_buf_align)
> > > >
> > >
> > > Not related to this patch, but this macro is ugly.  It would be better
> > > as function.
> >
> > Okay, will change the macros to inline functions in v2, where applicable.
> >
> 
> You didn't need to do that, because I said it was "not related to this
> change".  I try not to make people redo paches for stuff like this.  But
> thanks, it looks nicer now.

I agree with you, it does look better with inline functions. I know the change
wasn't absolutely necessary, but the code is easier on the eyes in v2, so I
thought it was good enough reason to go for it.

Thanks for the feedback! :)
Bogdan P.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment

2017-10-30 Thread Dan Carpenter 
On Fri, Oct 27, 2017 at 02:44:37PM +, Bogdan Purcareata wrote:
> > -Original Message-
> > From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> > Sent: Friday, October 27, 2017 5:30 PM
> > To: Bogdan Purcareata <bogdan.purcare...@nxp.com>
> > Cc: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
> > gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> > de...@driverdev.osuosl.org
> > Subject: Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment
> > 
> > On Fri, Oct 27, 2017 at 02:11:35PM +, Bogdan Purcareata wrote:
> > > @@ -93,10 +100,10 @@
> > >   * buffers large enough to allow building an skb around them and also
> > account
> > >   * for alignment restrictions
> > >   */
> > > -#define DPAA2_ETH_BUF_RAW_SIZE \
> > > +#define DPAA2_ETH_BUF_RAW_SIZE(priv) \
> > >   (DPAA2_ETH_RX_BUF_SIZE + \
> > >   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> > > - DPAA2_ETH_RX_BUF_ALIGN)
> > > + (priv)->rx_buf_align)
> > >
> > 
> > Not related to this patch, but this macro is ugly.  It would be better
> > as function.
> 
> Okay, will change the macros to inline functions in v2, where applicable.
> 

You didn't need to do that, because I said it was "not related to this
change".  I try not to make people redo paches for stuff like this.  But
thanks, it looks nicer now.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment

2017-10-27 Thread Bogdan Purcareata 
> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Friday, October 27, 2017 5:30 PM
> To: Bogdan Purcareata <bogdan.purcare...@nxp.com>
> Cc: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
> gre...@linuxfoundation.org; linux-ker...@vger.kernel.org;
> de...@driverdev.osuosl.org
> Subject: Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment
> 
> On Fri, Oct 27, 2017 at 02:11:35PM +, Bogdan Purcareata wrote:
> > @@ -93,10 +100,10 @@
> >   * buffers large enough to allow building an skb around them and also
> account
> >   * for alignment restrictions
> >   */
> > -#define DPAA2_ETH_BUF_RAW_SIZE \
> > +#define DPAA2_ETH_BUF_RAW_SIZE(priv) \
> > (DPAA2_ETH_RX_BUF_SIZE + \
> > SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> > -   DPAA2_ETH_RX_BUF_ALIGN)
> > +   (priv)->rx_buf_align)
> >
> 
> Not related to this patch, but this macro is ugly.  It would be better
> as function.

Okay, will change the macros to inline functions in v2, where applicable.

Thank you!
Bogdan P.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment

2017-10-27 Thread Dan Carpenter 
On Fri, Oct 27, 2017 at 02:11:35PM +, Bogdan Purcareata wrote:
> @@ -93,10 +100,10 @@
>   * buffers large enough to allow building an skb around them and also account
>   * for alignment restrictions
>   */
> -#define DPAA2_ETH_BUF_RAW_SIZE \
> +#define DPAA2_ETH_BUF_RAW_SIZE(priv) \
>   (DPAA2_ETH_RX_BUF_SIZE + \
>   SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
> - DPAA2_ETH_RX_BUF_ALIGN)
> + (priv)->rx_buf_align)
>  

Not related to this patch, but this macro is ugly.  It would be better
as function.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/5] staging: fsl-dpaa2/eth: Change RX buffer alignment

2017-10-27 Thread Bogdan Purcareata 
The WRIOP hardware block v1.0.0 (found on LS2080A board)
requires data in RX buffers to be aligned to 256B, but
newer revisions (e.g. on LS2088A, LS1088A) only require
64B alignment.

Check WRIOP version and decide at runtime which alignment
requirement to configure for ingress buffers.

Signed-off-by: Bogdan Purcareata 
Signed-off-by: Ioana Radulescu 
---
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 18 ++
 drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 14 +++---
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c 
b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index d68c1f5..29b4928 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -766,11 +766,11 @@ static int add_bufs(struct dpaa2_eth_priv *priv, u16 bpid)
/* Allocate buffer visible to WRIOP + skb shared info +
 * alignment padding
 */
-   buf = napi_alloc_frag(DPAA2_ETH_BUF_RAW_SIZE);
+   buf = napi_alloc_frag(DPAA2_ETH_BUF_RAW_SIZE(priv));
if (unlikely(!buf))
goto err_alloc;
 
-   buf = PTR_ALIGN(buf, DPAA2_ETH_RX_BUF_ALIGN);
+   buf = PTR_ALIGN(buf, priv->rx_buf_align);
 
addr = dma_map_single(dev, buf, DPAA2_ETH_RX_BUF_SIZE,
  DMA_FROM_DEVICE);
@@ -781,7 +781,7 @@ static int add_bufs(struct dpaa2_eth_priv *priv, u16 bpid)
 
/* tracing point */
trace_dpaa2_eth_buf_seed(priv->net_dev,
-buf, DPAA2_ETH_BUF_RAW_SIZE,
+buf, DPAA2_ETH_BUF_RAW_SIZE(priv),
 addr, DPAA2_ETH_RX_BUF_SIZE,
 bpid);
}
@@ -1782,11 +1782,21 @@ static int set_buffer_layout(struct dpaa2_eth_priv 
*priv)
struct dpni_buffer_layout buf_layout = {0};
int err;
 
+   /* We need to check for WRIOP version 1.0.0, but depending on the MC
+* version, this number is not always provided correctly on rev1.
+* We need to check for both alternatives in this situation.
+*/
+   if (priv->dpni_attrs.wriop_version == DPAA2_WRIOP_VERSION(0, 0, 0) ||
+   priv->dpni_attrs.wriop_version == DPAA2_WRIOP_VERSION(1, 0, 0))
+   priv->rx_buf_align = DPAA2_ETH_RX_BUF_ALIGN_REV1;
+   else
+   priv->rx_buf_align = DPAA2_ETH_RX_BUF_ALIGN;
+
/* rx buffer */
buf_layout.pass_parser_result = true;
buf_layout.pass_frame_status = true;
buf_layout.private_data_size = DPAA2_ETH_SWA_SIZE;
-   buf_layout.data_align = DPAA2_ETH_RX_BUF_ALIGN;
+   buf_layout.data_align = priv->rx_buf_align;
buf_layout.options = DPNI_BUF_LAYOUT_OPT_PARSER_RESULT |
 DPNI_BUF_LAYOUT_OPT_FRAME_STATUS |
 DPNI_BUF_LAYOUT_OPT_PRIVATE_DATA_SIZE |
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h 
b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index bfbabae..374a99a 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -45,6 +45,8 @@
 
 #include "dpaa2-eth-trace.h"
 
+#define DPAA2_WRIOP_VERSION(x, y, z) ((x) << 10 | (y) << 5 | (z) << 0)
+
 #define DPAA2_ETH_STORE_SIZE   16
 
 /* Maximum number of scatter-gather entries in an ingress frame,
@@ -85,7 +87,12 @@
  */
 #define DPAA2_ETH_RX_BUF_SIZE  2048
 #define DPAA2_ETH_TX_BUF_ALIGN 64
-#define DPAA2_ETH_RX_BUF_ALIGN 256
+/* Due to a limitation in WRIOP 1.0.0, the RX buffer data must be aligned
+ * to 256B. For newer revisions, the requirement is only for 64B alignment
+ */
+#define DPAA2_ETH_RX_BUF_ALIGN_REV1256
+#define DPAA2_ETH_RX_BUF_ALIGN 64
+
 #define DPAA2_ETH_NEEDED_HEADROOM(p_priv) \
((p_priv)->tx_data_offset + DPAA2_ETH_TX_BUF_ALIGN)
 
@@ -93,10 +100,10 @@
  * buffers large enough to allow building an skb around them and also account
  * for alignment restrictions
  */
-#define DPAA2_ETH_BUF_RAW_SIZE \
+#define DPAA2_ETH_BUF_RAW_SIZE(priv) \
(DPAA2_ETH_RX_BUF_SIZE + \
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) + \
-   DPAA2_ETH_RX_BUF_ALIGN)
+   (priv)->rx_buf_align)
 
 /* We are accommodating a skb backpointer and some S/G info
  * in the frame's software annotation. The hardware
@@ -318,6 +325,7 @@ struct dpaa2_eth_priv {
struct iommu_domain *iommu_domain;
 
u16 tx_qdid;
+   u16 rx_buf_align;
struct fsl_mc_io *mc_io;
/* Cores which have an affine DPIO/DPCON.
 * This is the cpu set on which Rx and Tx conf frames are processed
-- 
2.7.4

___
devel mailing list