| Message ID | 1332493721-28309-4-git-send-email-peppe.cavallaro@st.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 3/23/2012 2:38 PM, Giuseppe CAVALLARO wrote: > [snip] > > > - priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); > - if (priv->rx_coe) > - pr_info(" RX Checksum Offload Engine supported\n"); > + if (priv->plat->rx_coe) > + pr_info(" RX Checksum Offload Engine supported (type %d)\n", > + priv->plat->rx_coe); > if (priv->plat->tx_coe) > pr_info(" TX Checksum insertion supported\n"); > rx_coe needs to be enabled. Earlier it was being done. Any specific reasons to remove this. Instead this code needs to be moved post mac reset has been done. Regards Deepak -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/24/2012 10:21 AM, Deepak SIKRI wrote: > > > > On 3/23/2012 2:38 PM, Giuseppe CAVALLARO wrote: >> [snip] >> >> >> - priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); >> - if (priv->rx_coe) >> - pr_info(" RX Checksum Offload Engine supported\n"); >> + if (priv->plat->rx_coe) >> + pr_info(" RX Checksum Offload Engine supported (type %d)\n", >> + priv->plat->rx_coe); >> if (priv->plat->tx_coe) >> pr_info(" TX Checksum insertion supported\n"); >> > > rx_coe needs to be enabled. Earlier it was being done. Any specific > reasons to remove this. > Instead this code needs to be moved post mac reset has been done. Hello Deepak sorry for this delay. I've not clear at all your question. The driver well uses the rx_coe as briefly described below: probe funct |__ hw_init |_ check the RX type from HW cap reg |__ Override the rx_coe if required After that the rx_coe is used and passed to the core as expected. In case of there is no HW cap register so the rx_coe from platform will be used. Peppe > > Regards > Deepak > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/2/2012 4:37 PM, Giuseppe CAVALLARO wrote: > On 3/24/2012 10:21 AM, Deepak SIKRI wrote: >> >> >> On 3/23/2012 2:38 PM, Giuseppe CAVALLARO wrote: >>> [snip] >>> >>> >>> - priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); >>> - if (priv->rx_coe) >>> - pr_info(" RX Checksum Offload Engine supported\n"); >>> + if (priv->plat->rx_coe) >>> + pr_info(" RX Checksum Offload Engine supported (type %d)\n", >>> + priv->plat->rx_coe); >>> if (priv->plat->tx_coe) >>> pr_info(" TX Checksum insertion supported\n"); >>> >> rx_coe needs to be enabled. Earlier it was being done. Any specific >> reasons to remove this. >> Instead this code needs to be moved post mac reset has been done. > Hello Deepak > > sorry for this delay. > > I've not clear at all your question. > The driver well uses the rx_coe as briefly described below: > > probe funct > |__ hw_init > |_ check the RX type from HW cap reg > |__ Override the rx_coe if required > > After that the rx_coe is used and passed to the core as expected. > In case of there is no HW cap register so the rx_coe from platform will > be used. > > Peppe In the same patch, this portion of the code has been removed. -static int dwmac1000_rx_coe_supported(void __iomem *ioaddr) -{ - u32 value = readl(ioaddr + GMAC_CONTROL); - - value |= GMAC_CONTROL_IPC; - writel(value, ioaddr + GMAC_CONTROL); - - value = readl(ioaddr + GMAC_CONTROL); - - return !!(value& GMAC_CONTROL_IPC); -} Earlier this was taking care of setting the IP Checksum offloading feature in case its available. This code has to be present, as I do not see any other location where the IPC bit is being programmed. Also, the location of setting the IPC should be post the mac has been reset. I hope this clears the things a bit. Sorry for the miscommunication. Rgds Deepak >> Regards >> Deepak >> > . > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Deepak, On 4/2/2012 6:18 PM, Deepak SIKRI wrote: > On 4/2/2012 4:37 PM, Giuseppe CAVALLARO wrote: [snip] >> I've not clear at all your question. >> The driver well uses the rx_coe as briefly described below: >> >> probe funct >> |__ hw_init >> |_ check the RX type from HW cap reg >> |__ Override the rx_coe if required >> >> After that the rx_coe is used and passed to the core as expected. >> In case of there is no HW cap register so the rx_coe from platform will >> be used. >> >> Peppe > > In the same patch, this portion of the code has been removed. > > -static int dwmac1000_rx_coe_supported(void __iomem *ioaddr) > -{ > - u32 value = readl(ioaddr + GMAC_CONTROL); > - > - value |= GMAC_CONTROL_IPC; > - writel(value, ioaddr + GMAC_CONTROL); > - > - value = readl(ioaddr + GMAC_CONTROL); > - > - return !!(value& GMAC_CONTROL_IPC); > -} > > Earlier this was taking care of setting the IP Checksum offloading feature > in case its available. This code has to be present, as I do not see any > other location where the IPC bit is being programmed. > > Also, the location of setting the IPC should be post the mac has been > reset. Previously, the stmmac called the dwmac1000_rx_coe_supported to verify it could do the CSUM in Hw. If true the driver used the type 2 by default. I've voluntarily removed this function because not necessary anymore. In fact, YOU improved the rx_coe from the platform. If it is passed as STMMAC_RX_COE_NONE then it means the driver is not able to perform any csum for the incoming frames. This is actually used on old gmac/mac cores. In new cores, the HW cap register will be used to manage and fix this logic. I could restore the core you are mentioning but just to do another safety check at run-time in case of the user provided a broken setting from the platform and there is not the HW cap register. Hmm, I do not know if this actually could help indeed... just an extra check IMHO. > > I hope this clears the things a bit. Sorry for the miscommunication. No problem for the miscommunication ;-) Let me know Ciao Peppe > > Rgds > Deepak > > > > > > >>> Regards >>> Deepak >>> >> . >> > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ciao Peppe, On 4/3/2012 12:19 PM, Giuseppe CAVALLARO wrote: > > [snip] >> reset. > Previously, the stmmac called the dwmac1000_rx_coe_supported to verify > it could do the CSUM in Hw. If true the driver used the type 2 by default. > > I've voluntarily removed this function because not necessary anymore. > In fact, YOU improved the rx_coe from the platform. If it is passed as > STMMAC_RX_COE_NONE then it means the driver is not able to perform any > csum for the incoming frames. This is actually used on old gmac/mac > cores. In new cores, the HW cap register will be used to manage and fix > this logic. > I could restore the core you are mentioning but just to do another > safety check at run-time in case of the user provided a broken setting > from the platform and there is not the HW cap register. Hmm, I do not > know if this actually could help indeed... just an extra check IMHO. These are the updates required in the code. In function dwmac1000_core_init(), you may need to set the IPC bit based on the fact that if rx_coe has been setup through the platform code. This bit set is a must for the checksum offload to be enabled. Rest of the code looks good. Deepak >> I hope this clears the things a bit. Sorry for the miscommunication. > No problem for the miscommunication ;-) > > Let me know > Ciao > Peppe > >> Rgds >> Deepak >> >> >> >> >> >> >>>> Regards >>>> Deepak >>>> >>> . >>> >> > . > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/3/2012 9:56 AM, deepaksi wrote: > Ciao Peppe, > > On 4/3/2012 12:19 PM, Giuseppe CAVALLARO wrote: >> >> [snip] >>> reset. >> Previously, the stmmac called the dwmac1000_rx_coe_supported to verify >> it could do the CSUM in Hw. If true the driver used the type 2 by >> default. >> >> I've voluntarily removed this function because not necessary anymore. >> In fact, YOU improved the rx_coe from the platform. If it is passed as >> STMMAC_RX_COE_NONE then it means the driver is not able to perform any >> csum for the incoming frames. This is actually used on old gmac/mac >> cores. In new cores, the HW cap register will be used to manage and fix >> this logic. >> I could restore the core you are mentioning but just to do another >> safety check at run-time in case of the user provided a broken setting >> from the platform and there is not the HW cap register. Hmm, I do not >> know if this actually could help indeed... just an extra check IMHO. > > These are the updates required in the code. > > In function dwmac1000_core_init(), you may need to set the IPC bit based > on the fact that > if rx_coe has been setup through the platform code. > This bit set is a must for the checksum offload to be enabled. Rest of > the code looks good. > Great Deepak, sorry I had not understood your good question and thx for you call. I was wrong and the IPC Checksum Offload has to be set in the MAC Configuration Register (*). static int dwmac1000_rx_coe_supported(void __iomem *ioaddr) { u32 value = readl(ioaddr + GMAC_CONTROL); | |==> :-) (*) value |= GMAC_CONTROL_IPC; | writel(value, ioaddr + GMAC_CONTROL); | NOTE ===>>> I'm going to remove the extra check below because useless as I had told in my previous email. Is it ok for you? Let me know. value = readl(ioaddr + GMAC_CONTROL); return !!(value& GMAC_CONTROL_IPC); } I'll rework it and re-send all the patches with this fix soon. ciao Peppe > Deepak > > > > > >>> I hope this clears the things a bit. Sorry for the miscommunication. >> No problem for the miscommunication ;-) >> >> Let me know >> Ciao >> Peppe >> >>> Rgds >>> Deepak >>> >>> >>> >>> >>> >>> >>>>> Regards >>>>> Deepak >>>>> >>>> . >>>> >>> >> . >> > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 0319d64..386b100 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -228,7 +228,7 @@ struct stmmac_desc_ops { int (*get_rx_owner) (struct dma_desc *p); void (*set_rx_owner) (struct dma_desc *p); /* Get the receive frame size */ - int (*get_rx_frame_len) (struct dma_desc *p); + int (*get_rx_frame_len) (struct dma_desc *p, int rx_coe_type); /* Return the reception status looking at the RDES1 */ int (*rx_status) (void *data, struct stmmac_extra_stats *x, struct dma_desc *p); @@ -261,8 +261,6 @@ struct stmmac_dma_ops { struct stmmac_ops { /* MAC core initialization */ void (*core_init) (void __iomem *ioaddr) ____cacheline_aligned; - /* Support checksum offload engine */ - int (*rx_coe) (void __iomem *ioaddr); /* Dump MAC registers */ void (*dump_regs) (void __iomem *ioaddr); /* Handle extra events on specific interrupts hw dependent */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c index b1c48b9..342fe8d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c @@ -46,18 +46,6 @@ static void dwmac1000_core_init(void __iomem *ioaddr) #endif } -static int dwmac1000_rx_coe_supported(void __iomem *ioaddr) -{ - u32 value = readl(ioaddr + GMAC_CONTROL); - - value |= GMAC_CONTROL_IPC; - writel(value, ioaddr + GMAC_CONTROL); - - value = readl(ioaddr + GMAC_CONTROL); - - return !!(value & GMAC_CONTROL_IPC); -} - static void dwmac1000_dump_regs(void __iomem *ioaddr) { int i; @@ -211,7 +199,6 @@ static void dwmac1000_irq_status(void __iomem *ioaddr) static const struct stmmac_ops dwmac1000_ops = { .core_init = dwmac1000_core_init, - .rx_coe = dwmac1000_rx_coe_supported, .dump_regs = dwmac1000_dump_regs, .host_irq_status = dwmac1000_irq_status, .set_filter = dwmac1000_set_filter, diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c index 138fb8d..9faf010 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c @@ -43,11 +43,6 @@ static void dwmac100_core_init(void __iomem *ioaddr) #endif } -static int dwmac100_rx_coe_supported(void __iomem *ioaddr) -{ - return 0; -} - static void dwmac100_dump_mac_regs(void __iomem *ioaddr) { pr_info("\t----------------------------------------------\n" @@ -160,7 +155,6 @@ static void dwmac100_pmt(void __iomem *ioaddr, unsigned long mode) static const struct stmmac_ops dwmac100_ops = { .core_init = dwmac100_core_init, - .rx_coe = dwmac100_rx_coe_supported, .dump_regs = dwmac100_dump_mac_regs, .host_irq_status = dwmac100_irq_status, .set_filter = dwmac100_set_filter, diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c index ad1b627..2fc8ef9 100644 --- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c +++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c @@ -22,6 +22,7 @@ Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> *******************************************************************************/ +#include <linux/stmmac.h> #include "common.h" #include "descs_com.h" @@ -309,9 +310,17 @@ static void enh_desc_close_tx_desc(struct dma_desc *p) p->des01.etx.interrupt = 1; } -static int enh_desc_get_rx_frame_len(struct dma_desc *p) +static int enh_desc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type) { - return p->des01.erx.frame_length; + /* The type-1 checksum offload engines append the checksum at + * the end of frame and the two bytes of checksum are added in + * the length. + * Adjust for that in the framelen for type-1 checksum offload + * engines. */ + if (rx_coe_type == STMMAC_RX_COE_TYPE1) + return p->des01.erx.frame_length - 2; + else + return p->des01.erx.frame_length; } const struct stmmac_desc_ops enh_desc_ops = { diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c index 25953bb..68962c5 100644 --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c @@ -22,6 +22,7 @@ Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> *******************************************************************************/ +#include <linux/stmmac.h> #include "common.h" #include "descs_com.h" @@ -201,9 +202,17 @@ static void ndesc_close_tx_desc(struct dma_desc *p) p->des01.tx.interrupt = 1; } -static int ndesc_get_rx_frame_len(struct dma_desc *p) +static int ndesc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type) { - return p->des01.rx.frame_length; + /* The type-1 checksum offload engines append the checksum at + * the end of frame and the two bytes of checksum are added in + * the length. + * Adjust for that in the framelen for type-1 checksum offload + * engines. */ + if (rx_coe_type == STMMAC_RX_COE_TYPE1) + return p->des01.rx.frame_length - 2; + else + return p->des01.rx.frame_length; } const struct stmmac_desc_ops ndesc_ops = { diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index b4b095f..b65d787 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -56,8 +56,6 @@ struct stmmac_priv { struct stmmac_extra_stats xstats; struct napi_struct napi; - - int rx_coe; int no_csum_insertion; struct phy_device *phydev; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 860519c..a5a150d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1282,7 +1282,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) struct sk_buff *skb; int frame_len; - frame_len = priv->hw->desc->get_rx_frame_len(p); + frame_len = priv->hw->desc->get_rx_frame_len(p, + priv->plat->rx_coe); /* ACS is set; GMAC core strips PAD/FCS for IEEE 802.3 * Type frames (LLC/LLC-SNAP) */ if (unlikely(status != llc_snap)) @@ -1318,7 +1319,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) #endif skb->protocol = eth_type_trans(skb, priv->dev); - if (unlikely(!priv->rx_coe)) { + if (unlikely(!priv->plat->rx_coe)) { /* No RX COE for old mac10/100 devices */ skb_checksum_none_assert(skb); netif_receive_skb(skb); @@ -1465,8 +1466,10 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev, { struct stmmac_priv *priv = netdev_priv(dev); - if (!priv->rx_coe) + if (priv->plat->rx_coe == STMMAC_RX_COE_NONE) features &= ~NETIF_F_RXCSUM; + else if (priv->plat->rx_coe == STMMAC_RX_COE_TYPE1) + features &= ~NETIF_F_IPV6_CSUM; if (!priv->plat->tx_coe) features &= ~NETIF_F_ALL_CSUM; @@ -1769,17 +1772,24 @@ static int stmmac_hw_init(struct stmmac_priv *priv) * register (if supported). */ priv->plat->enh_desc = priv->dma_cap.enh_desc; - priv->plat->tx_coe = priv->dma_cap.tx_coe; priv->plat->pmt = priv->dma_cap.pmt_remote_wake_up; + + priv->plat->tx_coe = priv->dma_cap.tx_coe; + + if (priv->dma_cap.rx_coe_type2) + priv->plat->rx_coe = STMMAC_RX_COE_TYPE2; + else if (priv->dma_cap.rx_coe_type1) + priv->plat->rx_coe = STMMAC_RX_COE_TYPE1; + } else pr_info(" No HW DMA feature register supported"); /* Select the enhnaced/normal descriptor structures */ stmmac_selec_desc_mode(priv); - priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); - if (priv->rx_coe) - pr_info(" RX Checksum Offload Engine supported\n"); + if (priv->plat->rx_coe) + pr_info(" RX Checksum Offload Engine supported (type %d)\n", + priv->plat->rx_coe); if (priv->plat->tx_coe) pr_info(" TX Checksum insertion supported\n");