Re: [PATCH] net: sxgbe: Added tail point update
Byungho An bh74...@samsung.com : Florian Fainelli wrote: [...] I think that at some point you should revisit your abstraction, all the patches that I see do take a void __iomem * argument as the first function argument, you should probably use your driver private context here. The day you support a different version of the hardware, there might be other differences that need to be taken care of. I agree with you, it would be more robust for different version of the hardware and make simple function argument. But most functions of this driver deal with read/write register using ioaddr. It does not imply that they should urge on ioaddr. drivers/net/ethernet/broadcom/tg3.c gets it right and there is no reason why sxgbe should be different. As of now I think it is enough but I'll consider it later. In the meantime: 1) more use-once function pointers are added 2) their parameter list keeps growing Please reconsider sooner than later. -- Ueimor -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] net: sxgbe: Added tail point update
Francois Romieu wrote : Byungho An bh74...@samsung.com : Florian Fainelli wrote: [...] I think that at some point you should revisit your abstraction, all the patches that I see do take a void __iomem * argument as the first function argument, you should probably use your driver private context here. The day you support a different version of the hardware, there might be other differences that need to be taken care of. I agree with you, it would be more robust for different version of the hardware and make simple function argument. But most functions of this driver deal with read/write register using ioaddr. It does not imply that they should urge on ioaddr. drivers/net/ethernet/broadcom/tg3.c gets it right and there is no reason why sxgbe should be different. I fully understood. As of now I think it is enough but I'll consider it later. In the meantime: 1) more use-once function pointers are added 2) their parameter list keeps growing Please reconsider sooner than later. I'll revisit above things sooner (after this patch set). Thanks. -- Ueimor -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: sxgbe: Added tail point update
2014-05-07 1:14 GMT-07:00 Byungho An bh74...@samsung.com: This patch adds tail point update function for rx path after rx_refill function. It indicates tail point for rx dma. Signed-off-by: Byungho An bh74...@samsung.com --- drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c | 14 +- drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h |4 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c |5 + 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c index 49240c9..249b0e0 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c @@ -112,7 +112,7 @@ static void sxgbe_dma_channel_init(void __iomem *ioaddr, int cha_num, dma_addr = dma_rx + ((r_rsize - 1) * SXGBE_DESC_SIZE_BYTES); writel(lower_32_bits(dma_addr), - ioaddr + SXGBE_DMA_CHA_RXDESC_LADD_REG(cha_num)); + ioaddr + SXGBE_DMA_CHA_RXDESC_TAILPTR_REG(cha_num)); /* program the ring sizes */ writel(t_rsize - 1, ioaddr + SXGBE_DMA_CHA_TXDESC_RINGLEN_REG(cha_num)); writel(r_rsize - 1, ioaddr + SXGBE_DMA_CHA_RXDESC_RINGLEN_REG(cha_num)); @@ -370,6 +370,17 @@ static void sxgbe_enable_tso(void __iomem *ioaddr, u8 chan_num) writel(ctrl, ioaddr + SXGBE_DMA_CHA_TXCTL_REG(chan_num)); } +static void sxgbe_dma_update_rxdesc_tail_ptr(void __iomem *ioaddr, u8 chan_num, + dma_addr_t dma_rx_phy, int cur_rx, + int rxsize) I think that at some point you should revisit your abstraction, all the patches that I see do take a void __iomem * argument as the first function argument, you should probably use your driver private context here. The day you support a different version of the hardware, there might be other differences that need to be taken care of. +{ + u32 reg_val; + + reg_val = (dma_rx_phy 0x) + ((cur_rx % rxsize) * + SXGBE_DESC_SIZE_BYTES); + writel(reg_val, ioaddr + SXGBE_DMA_CHA_RXDESC_TAILPTR_REG(chan_num)); +} + static const struct sxgbe_dma_ops sxgbe_dma_ops = { .init = sxgbe_dma_init, .cha_init = sxgbe_dma_channel_init, @@ -386,6 +397,7 @@ static const struct sxgbe_dma_ops sxgbe_dma_ops = { .rx_dma_int_status = sxgbe_rx_dma_int_status, .rx_watchdog= sxgbe_dma_rx_watchdog, .enable_tso = sxgbe_enable_tso, + .update_rxdesc_tail_ptr = sxgbe_dma_update_rxdesc_tail_ptr, }; const struct sxgbe_dma_ops *sxgbe_get_dma_ops(void) diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h index 843fa9b..a06e01e 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h @@ -43,6 +43,10 @@ struct sxgbe_dma_ops { void (*rx_watchdog)(void __iomem *ioaddr, u32 riwt); /* Enable TSO for each DMA channel */ void (*enable_tso)(void __iomem *ioaddr, u8 chan_num); + void (*update_rxdesc_tail_ptr)(void __iomem *ioaddr, u8 chan_num, + dma_addr_t dma_rx, int r_rentry, + int r_rsize); + }; const struct sxgbe_dma_ops *sxgbe_get_dma_ops(void); diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c index 93bf151..7dc3449 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c @@ -1521,6 +1521,7 @@ static int sxgbe_rx(struct sxgbe_priv_data *priv, int limit) skb_put(skb, frame_len); skb-ip_summed = checksum; + skb-protocol = eth_type_trans(skb, priv-dev); if (checksum == CHECKSUM_NONE) netif_receive_skb(skb); else @@ -1530,6 +1531,10 @@ static int sxgbe_rx(struct sxgbe_priv_data *priv, int limit) } sxgbe_rx_refill(priv); + priv-hw-dma-update_rxdesc_tail_ptr(priv-ioaddr, qnum, + priv-rxq[qnum]-dma_rx_phy, + priv-rxq[qnum]-cur_rx, + priv-dma_rx_size); return count; } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Florian -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at
RE: [PATCH] net: sxgbe: Added tail point update
Florian Fainelli wrote: 2014-05-07 1:14 GMT-07:00 Byungho An bh74...@samsung.com: This patch adds tail point update function for rx path after rx_refill function. It indicates tail point for rx dma. Signed-off-by: Byungho An bh74...@samsung.com --- drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c | 14 +- drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.h |4 drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c |5 + 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c index 49240c9..249b0e0 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_dma.c @@ -112,7 +112,7 @@ static void sxgbe_dma_channel_init(void __iomem *ioaddr, int cha_num, dma_addr = dma_rx + ((r_rsize - 1) * SXGBE_DESC_SIZE_BYTES); writel(lower_32_bits(dma_addr), - ioaddr + SXGBE_DMA_CHA_RXDESC_LADD_REG(cha_num)); + ioaddr + SXGBE_DMA_CHA_RXDESC_TAILPTR_REG(cha_num)); /* program the ring sizes */ writel(t_rsize - 1, ioaddr + SXGBE_DMA_CHA_TXDESC_RINGLEN_REG(cha_num)); writel(r_rsize - 1, ioaddr + SXGBE_DMA_CHA_RXDESC_RINGLEN_REG(cha_num)); @@ -370,6 +370,17 @@ static void sxgbe_enable_tso(void __iomem *ioaddr, u8 chan_num) writel(ctrl, ioaddr + SXGBE_DMA_CHA_TXCTL_REG(chan_num)); } +static void sxgbe_dma_update_rxdesc_tail_ptr(void __iomem *ioaddr, u8 chan_num, + dma_addr_t dma_rx_phy, int cur_rx, + int rxsize) I think that at some point you should revisit your abstraction, all the patches that I see do take a void __iomem * argument as the first function argument, you should probably use your driver private context here. The day you support a different version of the hardware, there might be other differences that need to be taken care of. I agree with you, it would be more robust for different version of the hardware and make simple function argument. But most functions of this driver deal with read/write register using ioaddr. As of now I think it is enough but I'll consider it later. [snip] -- Florian -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html