| Message ID | 1442928363-6219-3-git-send-email-igvtee@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tuesday 22 September 2015 21:25:59 michael lee wrote: > From: michael lee <igvtee@gmail.com> > > Signed-off-by: Michael Lee <igvtee@gmail.com> > --- Sorry, missed your patch when I've send the fix [1] for my report [2]. John was kind enough to point me to your patch. > diff --git a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h > index 78e04b0..8861762 100644 > --- a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h > +++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h > @@ -310,7 +310,8 @@ enum fe_work_flag { > /* rxd2 */ > #define RX_DMA_DONE BIT(31) > #define RX_DMA_LSO BIT(30) > -#define RX_DMA_PLEN0(_x) (((_x) >> 16) & 0x3fff) > +#define RX_DMA_PLEN0_GET(_x) (((_x) >> 16) & 0x3fff) > +#define RX_DMA_PLEN0_SET(_x) (((_x) & 0x3fff) << 16) > #define RX_DMA_TAG BIT(15) > /* rxd3 */ > #define RX_DMA_TPID(_x) (((_x) >> 16) & 0xffff) I am personally against changing the name from RX_DMA_PLEN0 to RX_DMA_PLEN0_SET after I read the rest of the header. For example check the TX DMA macros: #define TX_DMA_PLEN0(_x) (((_x) & TX_DMA_BUF_LEN) << 16) #define TX_DMA_PLEN1(_x) ((_x) & TX_DMA_BUF_LEN) #define TX_DMA_GET_PLEN0(_x) (((_x) >> 16 ) & TX_DMA_BUF_LEN) #define TX_DMA_GET_PLEN1(_x) ((_x) & TX_DMA_BUF_LEN) You can see that the set functions have no _SET in it but the _GET functions have. So I would say it is better to follow the current coding/naming style instead of introducing a second one. Kind regards, Sven [1] https://patchwork.ozlabs.org/patch/522722/ [2] http://news.gmane.org/gmane.comp.embedded.openwrt.devel
On Monday 05 October 2015 12:31:44 Sven Eckelmann wrote: > [1] https://patchwork.ozlabs.org/patch/522722/ > [2] http://news.gmane.org/gmane.comp.embedded.openwrt.devel Grml, the second link should have been http://thread.gmane.org/gmane.comp.embedded.openwrt.devel/34868 Kind regards, Sven
diff --git a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.c b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.c index db6b197..4b31b56 100644 --- a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.c +++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.c @@ -266,7 +266,8 @@ static int fe_alloc_rx(struct fe_priv *priv) ring->rx_dma[i].rxd1 = (unsigned int) dma_addr; if (priv->flags & FE_FLAG_RX_SG_DMA) - ring->rx_dma[i].rxd2 = RX_DMA_PLEN0(ring->rx_buf_size); + ring->rx_dma[i].rxd2 = + RX_DMA_PLEN0_SET(ring->rx_buf_size); else ring->rx_dma[i].rxd2 = RX_DMA_LSO; } @@ -849,7 +850,7 @@ static int fe_poll_rx(struct napi_struct *napi, int budget, dma_unmap_single(&netdev->dev, trxd.rxd1, ring->rx_buf_size, DMA_FROM_DEVICE); - pktlen = RX_DMA_PLEN0(trxd.rxd2); + pktlen = RX_DMA_PLEN0_GET(trxd.rxd2); skb->dev = netdev; skb_put(skb, pktlen); if (trxd.rxd4 & checksum_bit) { @@ -871,7 +872,7 @@ static int fe_poll_rx(struct napi_struct *napi, int budget, release_desc: if (priv->flags & FE_FLAG_RX_SG_DMA) - rxd->rxd2 = RX_DMA_PLEN0(ring->rx_buf_size); + rxd->rxd2 = RX_DMA_PLEN0_SET(ring->rx_buf_size); else rxd->rxd2 = RX_DMA_LSO; diff --git a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h index 78e04b0..8861762 100644 --- a/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h +++ b/target/linux/ramips/files/drivers/net/ethernet/ralink/ralink_soc_eth.h @@ -310,7 +310,8 @@ enum fe_work_flag { /* rxd2 */ #define RX_DMA_DONE BIT(31) #define RX_DMA_LSO BIT(30) -#define RX_DMA_PLEN0(_x) (((_x) >> 16) & 0x3fff) +#define RX_DMA_PLEN0_GET(_x) (((_x) >> 16) & 0x3fff) +#define RX_DMA_PLEN0_SET(_x) (((_x) & 0x3fff) << 16) #define RX_DMA_TAG BIT(15) /* rxd3 */ #define RX_DMA_TPID(_x) (((_x) >> 16) & 0xffff)