Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1338389 > unrolled thread

[PATCH v2 0/3] net: ti: netcp: restore get/set_pad_info() functionality

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] net: ti: netcp: restore get/set_pad_info() functionality Murali Karicheri <m-karicheri2@ti.com> - 2016-02-19 19:00 +0100
    [PATCH v2 2/3] soc: ti: knav_dma: rename pad in struct knav_dma_desc to sw_data Murali Karicheri <m-karicheri2@ti.com> - 2016-02-19 19:00 +0100
      Re: [PATCH v2 2/3] soc: ti: knav_dma: rename pad in struct knav_dma_desc to sw_data Arnd Bergmann <arnd@arndb.de> - 2016-02-19 22:00 +0100
    [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc Murali Karicheri <m-karicheri2@ti.com> - 2016-02-19 19:00 +0100
      Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc Arnd Bergmann <arnd@arndb.de> - 2016-02-19 22:00 +0100
        Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc Arnd Bergmann <arnd@arndb.de> - 2016-02-19 23:30 +0100
          Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in  dma desc Murali Karicheri <m-karicheri2@ti.com> - 2016-02-22 18:10 +0100
        Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in  dma desc Murali Karicheri <m-karicheri2@ti.com> - 2016-02-19 23:30 +0100
    Re: [PATCH v2 0/3] net: ti: netcp: restore get/set_pad_info()  functionality David Miller <davem@davemloft.net> - 2016-02-22 04:10 +0100

#1338389 — [PATCH v2 0/3] net: ti: netcp: restore get/set_pad_info() functionality

This series fixes a regression and add some improvements for the ease
of maintainance. Incorporated comments against v1.

Changelogs:
 
 v2 : combined 2-3 into one patch as this involves a header change
      fixed a parse warning in 3/4 per comment from Arnd.
      Removed Sign-off from Arnd against 1/4
      added comments in 3/3 to alert on the usage of sw data per review
      comments
 v1 : added 2-4 to accomodate feedback received from review
 v0 : initial version to fix the regression (From Grygorii) 

Murali Karicheri (3):
  net: ti: netcp: restore get/set_pad_info() functionality
  soc: ti: knav_dma: rename pad in struct knav_dma_desc to sw_data
  net: netcp: rework the code for get/set sw_data in dma desc

 drivers/net/ethernet/ti/netcp_core.c | 105 ++++++++++++++++++++---------------
 include/linux/soc/ti/knav_dma.h      |   4 +-
 2 files changed, 64 insertions(+), 45 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1338391 — [PATCH v2 2/3] soc: ti: knav_dma: rename pad in struct knav_dma_desc to sw_data

Rename the pad to sw_data as per description of this field in the hardware
spec(refer sprugr9 from www.ti.com). Latest version of the document is
at http://www.ti.com/lit/ug/sprugr9h/sprugr9h.pdf and section 3.1
Host Packet Descriptor describes this field.

Define and use a constant for the size of sw_data field similar to
other fields in the struct for desc and document the sw_data field
in the header. As the sw_data is not touched by hw, it's type can be
changed to u32.

Rename the helpers to match with the updated dma desc field sw_data.

Cc: Wingman Kwok <w-kwok2@ti.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Grygorii Strashko <grygorii.strashko@ti.com>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 40 +++++++++++++++++++-----------------
 include/linux/soc/ti/knav_dma.h      |  4 +++-
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 0b26e52..84bab29 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -117,10 +117,11 @@ static void get_pkt_info(dma_addr_t *buff, u32 *buff_len, dma_addr_t *ndesc,
 	*ndesc = le32_to_cpu(desc->next_desc);
 }
 
-static void get_pad_info(u32 *pad0, u32 *pad1, struct knav_dma_desc *desc)
+static void get_sw_data(u32 *data0, u32 *data1, struct knav_dma_desc *desc)
 {
-	*pad0 = le32_to_cpu(desc->pad[0]);
-	*pad1 = le32_to_cpu(desc->pad[1]);
+	/* No Endian conversion needed as this data is untouched by hw */
+	*data0 = desc->sw_data[0];
+	*data1 = desc->sw_data[1];
 }
 
 static void get_org_pkt_info(dma_addr_t *buff, u32 *buff_len,
@@ -153,10 +154,11 @@ static void set_desc_info(u32 desc_info, u32 pkt_info,
 	desc->packet_info = cpu_to_le32(pkt_info);
 }
 
-static void set_pad_info(u32 pad0, u32 pad1, struct knav_dma_desc *desc)
+static void set_sw_data(u32 data0, u32 data1, struct knav_dma_desc *desc)
 {
-	desc->pad[0] = cpu_to_le32(pad0);
-	desc->pad[1] = cpu_to_le32(pad1);
+	/* No Endian conversion needed as this data is untouched by hw */
+	desc->sw_data[0] = data0;
+	desc->sw_data[1] = data1;
 }
 
 static void set_org_pkt_info(dma_addr_t buff, u32 buff_len,
@@ -581,12 +583,12 @@ static void netcp_free_rx_desc_chain(struct netcp_intf *netcp,
 			break;
 		}
 		get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
-		get_pad_info((u32 *)&buf_ptr, &buf_len, ndesc);
+		get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
 		dma_unmap_page(netcp->dev, dma_buf, PAGE_SIZE, DMA_FROM_DEVICE);
 		__free_page(buf_ptr);
 		knav_pool_desc_put(netcp->rx_pool, desc);
 	}
-	get_pad_info((u32 *)&buf_ptr, &buf_len, desc);
+	get_sw_data((u32 *)&buf_ptr, &buf_len, desc);
 
 	if (buf_ptr)
 		netcp_frag_free(buf_len <= PAGE_SIZE, buf_ptr);
@@ -639,7 +641,7 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 	}
 
 	get_pkt_info(&dma_buff, &buf_len, &dma_desc, desc);
-	get_pad_info((u32 *)&org_buf_ptr, &org_buf_len, desc);
+	get_sw_data((u32 *)&org_buf_ptr, &org_buf_len, desc);
 
 	if (unlikely(!org_buf_ptr)) {
 		dev_err(netcp->ndev_dev, "NULL bufptr in desc\n");
@@ -672,7 +674,7 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 		}
 
 		get_pkt_info(&dma_buff, &buf_len, &dma_desc, ndesc);
-		get_pad_info((u32 *)&page, &tmp, ndesc);
+		get_sw_data((u32 *)&page, &tmp, ndesc);
 
 		if (likely(dma_buff && buf_len && page)) {
 			dma_unmap_page(netcp->dev, dma_buff, PAGE_SIZE,
@@ -761,7 +763,7 @@ static void netcp_free_rx_buf(struct netcp_intf *netcp, int fdq)
 		}
 
 		get_org_pkt_info(&dma, &buf_len, desc);
-		get_pad_info((u32 *)&buf_ptr, &tmp, desc);
+		get_sw_data((u32 *)&buf_ptr, &tmp, desc);
 
 		if (unlikely(!dma)) {
 			dev_err(netcp->ndev_dev, "NULL orig_buff in desc\n");
@@ -813,7 +815,7 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 	struct page *page;
 	dma_addr_t dma;
 	void *bufptr;
-	u32 pad[2];
+	u32 sw_data[2];
 
 	/* Allocate descriptor */
 	hwdesc = knav_pool_desc_get(netcp->rx_pool);
@@ -830,7 +832,7 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 				SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
 		bufptr = netdev_alloc_frag(primary_buf_len);
-		pad[1] = primary_buf_len;
+		sw_data[1] = primary_buf_len;
 
 		if (unlikely(!bufptr)) {
 			dev_warn_ratelimited(netcp->ndev_dev,
@@ -842,7 +844,7 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 		if (unlikely(dma_mapping_error(netcp->dev, dma)))
 			goto fail;
 
-		pad[0] = (u32)bufptr;
+		sw_data[0] = (u32)bufptr;
 	} else {
 		/* Allocate a secondary receive queue entry */
 		page = alloc_page(GFP_ATOMIC | GFP_DMA | __GFP_COLD);
@@ -852,8 +854,8 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 		}
 		buf_len = PAGE_SIZE;
 		dma = dma_map_page(netcp->dev, page, 0, buf_len, DMA_TO_DEVICE);
-		pad[0] = (u32)page;
-		pad[1] = 0;
+		sw_data[0] = (u32)page;
+		sw_data[1] = 0;
 	}
 
 	desc_info =  KNAV_DMA_DESC_PS_INFO_IN_DESC;
@@ -863,7 +865,7 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 	pkt_info |= (netcp->rx_queue_id & KNAV_DMA_DESC_RETQ_MASK) <<
 		    KNAV_DMA_DESC_RETQ_SHIFT;
 	set_org_pkt_info(dma, buf_len, hwdesc);
-	set_pad_info(pad[0], pad[1], hwdesc);
+	set_sw_data(sw_data[0], sw_data[1], hwdesc);
 	set_desc_info(desc_info, pkt_info, hwdesc);
 
 	/* Push to FDQs */
@@ -969,7 +971,7 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
 			continue;
 		}
 
-		get_pad_info((u32 *)&skb, &tmp, desc);
+		get_sw_data((u32 *)&skb, &tmp, desc);
 		netcp_free_tx_desc_chain(netcp, desc, dma_sz);
 		if (!skb) {
 			dev_err(netcp->ndev_dev, "No skb in Tx desc\n");
@@ -1174,7 +1176,7 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
 	}
 
 	set_words(&tmp, 1, &desc->packet_info);
-	set_words((u32 *)&skb, 1, &desc->pad[0]);
+	set_sw_data((u32)skb, 0, desc);
 
 	if (tx_pipe->flags & SWITCH_TO_PORT_IN_TAGINFO) {
 		tmp = tx_pipe->switch_to_port;
diff --git a/include/linux/soc/ti/knav_dma.h b/include/linux/soc/ti/knav_dma.h
index 343c13a..35cb926 100644
--- a/include/linux/soc/ti/knav_dma.h
+++ b/include/linux/soc/ti/knav_dma.h
@@ -44,6 +44,7 @@
 
 #define KNAV_DMA_NUM_EPIB_WORDS			4
 #define KNAV_DMA_NUM_PS_WORDS			16
+#define KNAV_DMA_NUM_SW_DATA_WORDS		4
 #define KNAV_DMA_FDQ_PER_CHAN			4
 
 /* Tx channel scheduling priority */
@@ -142,6 +143,7 @@ struct knav_dma_cfg {
  * @orig_buff:			buff pointer since 'buff' can be overwritten
  * @epib:			Extended packet info block
  * @psdata:			Protocol specific
+ * @sw_data:			Software private data not touched by h/w
  */
 struct knav_dma_desc {
 	__le32	desc_info;
@@ -154,7 +156,7 @@ struct knav_dma_desc {
 	__le32	orig_buff;
 	__le32	epib[KNAV_DMA_NUM_EPIB_WORDS];
 	__le32	psdata[KNAV_DMA_NUM_PS_WORDS];
-	__le32	pad[4];
+	u32	sw_data[KNAV_DMA_NUM_SW_DATA_WORDS];
 } ____cacheline_aligned;
 
 #if IS_ENABLED(CONFIG_KEYSTONE_NAVIGATOR_DMA)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1338461 — Re: [PATCH v2 2/3] soc: ti: knav_dma: rename pad in struct knav_dma_desc to sw_data

On Friday 19 February 2016 12:58:43 Murali Karicheri wrote:
> Rename the pad to sw_data as per description of this field in the hardware
> spec(refer sprugr9 from www.ti.com). Latest version of the document is
> at http://www.ti.com/lit/ug/sprugr9h/sprugr9h.pdf and section 3.1
> Host Packet Descriptor describes this field.
> 
> Define and use a constant for the size of sw_data field similar to
> other fields in the struct for desc and document the sw_data field
> in the header. As the sw_data is not touched by hw, it's type can be
> changed to u32.
> 
> Rename the helpers to match with the updated dma desc field sw_data.
> 
> Cc: Wingman Kwok <w-kwok2@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Grygorii Strashko <grygorii.strashko@ti.com>
> CC: David Laight <David.Laight@ACULAB.COM>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

[toc] | [prev] | [next] | [standalone]

#1338393 — [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc

SW data field in descriptor can be used by software to hold private
data for the driver. As there are 4 words available for this purpose,
use separate macros to place it or retrieve the same to/from
descriptors. Also do type cast of data types accordingly.

Cc: Wingman Kwok <w-kwok2@ti.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Grygorii Strashko <grygorii.strashko@ti.com>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 72 +++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 84bab29..029841f 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -117,13 +117,18 @@ static void get_pkt_info(dma_addr_t *buff, u32 *buff_len, dma_addr_t *ndesc,
 	*ndesc = le32_to_cpu(desc->next_desc);
 }
 
-static void get_sw_data(u32 *data0, u32 *data1, struct knav_dma_desc *desc)
+static u32 get_sw_data(int index, struct knav_dma_desc *desc)
 {
 	/* No Endian conversion needed as this data is untouched by hw */
-	*data0 = desc->sw_data[0];
-	*data1 = desc->sw_data[1];
+	return desc->sw_data[index];
 }
 
+/* use these macros to get sw data */
+#define GET_SW_DATA0(desc) get_sw_data(0, desc)
+#define GET_SW_DATA1(desc) get_sw_data(1, desc)
+#define GET_SW_DATA2(desc) get_sw_data(2, desc)
+#define GET_SW_DATA3(desc) get_sw_data(3, desc)
+
 static void get_org_pkt_info(dma_addr_t *buff, u32 *buff_len,
 			     struct knav_dma_desc *desc)
 {
@@ -154,13 +159,18 @@ static void set_desc_info(u32 desc_info, u32 pkt_info,
 	desc->packet_info = cpu_to_le32(pkt_info);
 }
 
-static void set_sw_data(u32 data0, u32 data1, struct knav_dma_desc *desc)
+static void set_sw_data(int index, u32 data, struct knav_dma_desc *desc)
 {
 	/* No Endian conversion needed as this data is untouched by hw */
-	desc->sw_data[0] = data0;
-	desc->sw_data[1] = data1;
+	desc->sw_data[index] = data;
 }
 
+/* use these macros to set sw data */
+#define SET_SW_DATA0(data, desc) set_sw_data(0, data, desc)
+#define SET_SW_DATA1(data, desc) set_sw_data(1, data, desc)
+#define SET_SW_DATA2(data, desc) set_sw_data(2, data, desc)
+#define SET_SW_DATA3(data, desc) set_sw_data(3, data, desc)
+
 static void set_org_pkt_info(dma_addr_t buff, u32 buff_len,
 			     struct knav_dma_desc *desc)
 {
@@ -583,12 +593,20 @@ static void netcp_free_rx_desc_chain(struct netcp_intf *netcp,
 			break;
 		}
 		get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
-		get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
+		/* warning!!!! We are retrieving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
+		buf_ptr = (void *)GET_SW_DATA0(ndesc);
+		buf_len = (int)GET_SW_DATA1(desc);
 		dma_unmap_page(netcp->dev, dma_buf, PAGE_SIZE, DMA_FROM_DEVICE);
 		__free_page(buf_ptr);
 		knav_pool_desc_put(netcp->rx_pool, desc);
 	}
-	get_sw_data((u32 *)&buf_ptr, &buf_len, desc);
+	/* warning!!!! We are retrieving the virtual ptr in the sw_data
+	 * field as a 32bit value. Will not work on 64bit machines
+	 */
+	buf_ptr = (void *)GET_SW_DATA0(desc);
+	buf_len = (int)GET_SW_DATA1(desc);
 
 	if (buf_ptr)
 		netcp_frag_free(buf_len <= PAGE_SIZE, buf_ptr);
@@ -628,7 +646,6 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 	struct netcp_packet p_info;
 	struct sk_buff *skb;
 	void *org_buf_ptr;
-	u32 tmp;
 
 	dma_desc = knav_queue_pop(netcp->rx_queue, &dma_sz);
 	if (!dma_desc)
@@ -641,7 +658,11 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 	}
 
 	get_pkt_info(&dma_buff, &buf_len, &dma_desc, desc);
-	get_sw_data((u32 *)&org_buf_ptr, &org_buf_len, desc);
+	/* warning!!!! We are retrieving the virtual ptr in the sw_data
+	 * field as a 32bit value. Will not work on 64bit machines
+	 */
+	org_buf_ptr = (void *)GET_SW_DATA0(desc);
+	org_buf_len = (int)GET_SW_DATA1(desc);
 
 	if (unlikely(!org_buf_ptr)) {
 		dev_err(netcp->ndev_dev, "NULL bufptr in desc\n");
@@ -674,7 +695,10 @@ static int netcp_process_one_rx_packet(struct netcp_intf *netcp)
 		}
 
 		get_pkt_info(&dma_buff, &buf_len, &dma_desc, ndesc);
-		get_sw_data((u32 *)&page, &tmp, ndesc);
+		/* warning!!!! We are retrieving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
+		page = (struct page *)GET_SW_DATA0(desc);
 
 		if (likely(dma_buff && buf_len && page)) {
 			dma_unmap_page(netcp->dev, dma_buff, PAGE_SIZE,
@@ -752,7 +776,6 @@ static void netcp_free_rx_buf(struct netcp_intf *netcp, int fdq)
 	unsigned int buf_len, dma_sz;
 	dma_addr_t dma;
 	void *buf_ptr;
-	u32 tmp;
 
 	/* Allocate descriptor */
 	while ((dma = knav_queue_pop(netcp->rx_fdq[fdq], &dma_sz))) {
@@ -763,7 +786,10 @@ static void netcp_free_rx_buf(struct netcp_intf *netcp, int fdq)
 		}
 
 		get_org_pkt_info(&dma, &buf_len, desc);
-		get_sw_data((u32 *)&buf_ptr, &tmp, desc);
+		/* warning!!!! We are retrieving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
+		buf_ptr = (void *)GET_SW_DATA0(desc);
 
 		if (unlikely(!dma)) {
 			dev_err(netcp->ndev_dev, "NULL orig_buff in desc\n");
@@ -844,6 +870,9 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 		if (unlikely(dma_mapping_error(netcp->dev, dma)))
 			goto fail;
 
+		/* warning!!!! We are saving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
 		sw_data[0] = (u32)bufptr;
 	} else {
 		/* Allocate a secondary receive queue entry */
@@ -854,6 +883,9 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 		}
 		buf_len = PAGE_SIZE;
 		dma = dma_map_page(netcp->dev, page, 0, buf_len, DMA_TO_DEVICE);
+		/* warning!!!! We are saving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
 		sw_data[0] = (u32)page;
 		sw_data[1] = 0;
 	}
@@ -865,7 +897,8 @@ static int netcp_allocate_rx_buf(struct netcp_intf *netcp, int fdq)
 	pkt_info |= (netcp->rx_queue_id & KNAV_DMA_DESC_RETQ_MASK) <<
 		    KNAV_DMA_DESC_RETQ_SHIFT;
 	set_org_pkt_info(dma, buf_len, hwdesc);
-	set_sw_data(sw_data[0], sw_data[1], hwdesc);
+	SET_SW_DATA0(sw_data[0], hwdesc);
+	SET_SW_DATA1(sw_data[1], hwdesc);
 	set_desc_info(desc_info, pkt_info, hwdesc);
 
 	/* Push to FDQs */
@@ -958,7 +991,6 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
 	unsigned int dma_sz;
 	dma_addr_t dma;
 	int pkts = 0;
-	u32 tmp;
 
 	while (budget--) {
 		dma = knav_queue_pop(netcp->tx_compl_q, &dma_sz);
@@ -971,7 +1003,10 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
 			continue;
 		}
 
-		get_sw_data((u32 *)&skb, &tmp, desc);
+		/* warning!!!! We are retrieving the virtual ptr in the sw_data
+		 * field as a 32bit value. Will not work on 64bit machines
+		 */
+		skb = (struct sk_buff *)GET_SW_DATA0(desc);
 		netcp_free_tx_desc_chain(netcp, desc, dma_sz);
 		if (!skb) {
 			dev_err(netcp->ndev_dev, "No skb in Tx desc\n");
@@ -1176,7 +1211,10 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
 	}
 
 	set_words(&tmp, 1, &desc->packet_info);
-	set_sw_data((u32)skb, 0, desc);
+	/* warning!!!! We are saving the virtual ptr in the sw_data
+	 * field as a 32bit value. Will not work on 64bit machines
+	 */
+	SET_SW_DATA0((u32)skb, desc);
 
 	if (tx_pipe->flags & SWITCH_TO_PORT_IN_TAGINFO) {
 		tmp = tx_pipe->switch_to_port;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1338462 — Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc

On Friday 19 February 2016 12:58:44 Murali Karicheri wrote:
> SW data field in descriptor can be used by software to hold private
> data for the driver. As there are 4 words available for this purpose,
> use separate macros to place it or retrieve the same to/from
> descriptors. Also do type cast of data types accordingly.
> 
> Cc: Wingman Kwok <w-kwok2@ti.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Grygorii Strashko <grygorii.strashko@ti.com>
> CC: David Laight <David.Laight@ACULAB.COM>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

Looks ok in principle.

Acked-by: Arnd Bergmann <arnd@arndb.de>

>  		get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
> -		get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
> +		/* warning!!!! We are retrieving the virtual ptr in the sw_data
> +		 * field as a 32bit value. Will not work on 64bit machines
> +		 */
> +		buf_ptr = (void *)GET_SW_DATA0(ndesc);
> +		buf_len = (int)GET_SW_DATA1(desc);

I would have abstracted the retrieval of a pointer again,
and added the comment in the helper function once, it doesn't
really need to be duplicated everywhere.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1338531 — Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc

On Friday 19 February 2016 17:21:57 Murali Karicheri wrote:
> >>              get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
> >> -            get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
> >> +            /* warning!!!! We are retrieving the virtual ptr in the sw_data
> >> +             * field as a 32bit value. Will not work on 64bit machines
> >> +             */
> >> +            buf_ptr = (void *)GET_SW_DATA0(ndesc);
> >> +            buf_len = (int)GET_SW_DATA1(desc);
> > 
> > I would have abstracted the retrieval of a pointer again,
> > and added the comment in the helper function once, it doesn't
> > really need to be duplicated everywhere.
> > 
> Arnd,
> 
> I thought about it to add it to the API. API currently set buffer
> and ptr. It would be an issue only if store/retrieve ptr in/from the sw_data.
> So for the comment to be really useful to someone who is changing the code,
> doesn't it make sense to add it at the point of invocation as done in this
> patch? No?
> 

Up to you, it was just an idea and you have my Ack either way.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1339736 — Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc

On 02/19/2016 05:25 PM, Arnd Bergmann wrote:
> On Friday 19 February 2016 17:21:57 Murali Karicheri wrote:
>>>>              get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
>>>> -            get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
>>>> +            /* warning!!!! We are retrieving the virtual ptr in the sw_data
>>>> +             * field as a 32bit value. Will not work on 64bit machines
>>>> +             */
>>>> +            buf_ptr = (void *)GET_SW_DATA0(ndesc);
>>>> +            buf_len = (int)GET_SW_DATA1(desc);
>>>
>>> I would have abstracted the retrieval of a pointer again,
>>> and added the comment in the helper function once, it doesn't
>>> really need to be duplicated everywhere.
>>>
>> Arnd,
>>
>> I thought about it to add it to the API. API currently set buffer
>> and ptr. It would be an issue only if store/retrieve ptr in/from the sw_data.
>> So for the comment to be really useful to someone who is changing the code,
>> doesn't it make sense to add it at the point of invocation as done in this
>> patch? No?
>>
> 
> Up to you, it was just an idea and you have my Ack either way.
> 
> 	Arnd
> 
Ok. Thanks

-- 
Murali Karicheri
Linux Kernel, Keystone

[toc] | [prev] | [next] | [standalone]

#1338534 — Re: [PATCH v2 3/3] net: netcp: rework the code for get/set sw_data in dma desc

On 02/19/2016 03:55 PM, Arnd Bergmann wrote:
> On Friday 19 February 2016 12:58:44 Murali Karicheri wrote:
>> SW data field in descriptor can be used by software to hold private
>> data for the driver. As there are 4 words available for this purpose,
>> use separate macros to place it or retrieve the same to/from
>> descriptors. Also do type cast of data types accordingly.
>>
>> Cc: Wingman Kwok <w-kwok2@ti.com>
>> Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> CC: Arnd Bergmann <arnd@arndb.de>
>> CC: Grygorii Strashko <grygorii.strashko@ti.com>
>> CC: David Laight <David.Laight@ACULAB.COM>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> 
> Looks ok in principle.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
>>  		get_pkt_info(&dma_buf, &tmp, &dma_desc, ndesc);
>> -		get_sw_data((u32 *)&buf_ptr, &buf_len, ndesc);
>> +		/* warning!!!! We are retrieving the virtual ptr in the sw_data
>> +		 * field as a 32bit value. Will not work on 64bit machines
>> +		 */
>> +		buf_ptr = (void *)GET_SW_DATA0(ndesc);
>> +		buf_len = (int)GET_SW_DATA1(desc);
> 
> I would have abstracted the retrieval of a pointer again,
> and added the comment in the helper function once, it doesn't
> really need to be duplicated everywhere.
> 
Arnd,

I thought about it to add it to the API. API currently set buffer
and ptr. It would be an issue only if store/retrieve ptr in/from the sw_data.
So for the comment to be really useful to someone who is changing the code,
doesn't it make sense to add it at the point of invocation as done in this
patch? No?

Murali

> 	Arnd
> 


-- 
Murali Karicheri
Linux Kernel, Keystone

[toc] | [prev] | [next] | [standalone]

#1339103 — Re: [PATCH v2 0/3] net: ti: netcp: restore get/set_pad_info() functionality

From: Murali Karicheri <m-karicheri2@ti.com>
Date: Fri, 19 Feb 2016 12:58:41 -0500

> This series fixes a regression and add some improvements for the ease
> of maintainance. Incorporated comments against v1.
> 
> Changelogs:
>  
>  v2 : combined 2-3 into one patch as this involves a header change
>       fixed a parse warning in 3/4 per comment from Arnd.
>       Removed Sign-off from Arnd against 1/4
>       added comments in 3/3 to alert on the usage of sw data per review
>       comments
>  v1 : added 2-4 to accomodate feedback received from review
>  v0 : initial version to fix the regression (From Grygorii) 

Series applied, thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web