| Message ID | 1330692928-30330-5-git-send-email-deepak.sikri@st.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Deepak Sikri <deepak.sikri@st.com> Date: Fri, 2 Mar 2012 18:25:26 +0530 > +static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, > + u32 mac_id) Poorly formatted, this should be: static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, u32 mac_id) > + /* > + * The type-1 checksume offload engines append > + * the checksum at the end of frame, and the /* Format comments * like this. */ /* * Not * like this. */ -- 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 David, On Mon, Mar 05, 2012 at 09:51:55AM +0800, David Miller wrote: > From: Deepak Sikri <deepak.sikri@st.com> > > + /* > > + * The type-1 checksume offload engines append > > + * the checksum at the end of frame, and the > > /* Format comments > * like this. > */ > > /* > * Not > * like this. > */ Linux Documentation/CodingStyle mentions following about comments in chapter 8. The preferred style for long (multi-line) comments is: /* * This is the preferred style for multi-line * comments in the Linux kernel source code. * Please use it consistently. * * Description: A column of asterisks on the left side, * with beginning and ending almost-blank lines. */ Are we missing something ? -- regards Shiraz -- 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
From: Shiraz Hashim <shiraz.hashim@st.com> Date: Mon, 5 Mar 2012 09:31:25 +0530 > Are we missing something ? It wastes precious screen real-estate with that first line with zero text in it, therefore I don't want it used in any code I accept. -- 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 3/2/2012 1:55 PM, Deepak Sikri wrote: > The patch adds in the following support. > 1. The stmmac core supporting Type-1 checksum offload engine & prior to > Revision 3.5, append the HW cecksum at the end of payload in case the Rx > checksum engine was used to offload the HW checksum. > There cores did not provide the HW register interface to read the device > capabilities, and hence the plat code provides the core checksum capabilties > that help to identify type-1 cores, and adjust the frame length. > > 2. Also, the following Frame status checks with the Full checksum offload > Type-2 engine enabled are not backward compatible and are reserved for > STMMAC core revisions prior to 3.5. > Bit18(Eth Frame) Bit27(Header Csum Error) Bit28 (Payload Csum Err) > 0 1 1 > 0 1 0 Type 2 has been introduced starting from the 3.30a and Type 1 maintained for back-ward compatibility because only available in 3.30. If we want to actually support Type 1 (I've no HW where test) I guess we need to review this patch. First problem I can see in the patch is that, in case of type 1, we have to properly set the device hw features because full IPC offload is not supported (e.g. IPv6). This only is true for type 2. I've just looked at all the MAC data-books starting from the 3.30a to 3.60a and I have seen that all the MACs treat the Receive Checksum Offload Engine in the same way. I mean, the cores don't append any payload csum bytes in case of type 2. This is always done for type 1! Frankly, I prefer to have no define like GMAC_VERSION_35. I always tried to avoid it :-)... unless there is some critical reason and we actually need it. Pls, see my comments comments inline below. > These conditions have been treated as no HW checksum support for stmmac core > prior to rev-3.5. > > Signed-off-by: Deepak Sikri <deepak.sikri@st.com> > --- > drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- > drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 17 +++++++++++------ > drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 2 +- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++++++++++++- > 4 files changed, 26 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h > index d0b814e..12d1565 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/common.h > +++ b/drivers/net/ethernet/stmicro/stmmac/common.h > @@ -230,7 +230,7 @@ struct stmmac_desc_ops { > int (*get_rx_frame_len) (struct dma_desc *p); > /* Return the reception status looking at the RDES1 */ > int (*rx_status) (void *data, struct stmmac_extra_stats *x, > - struct dma_desc *p); > + struct dma_desc *p, u32 mac_id); > }; > > struct stmmac_dma_ops { > diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c > index d879763..42026f6 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" > > @@ -106,7 +107,9 @@ static int enh_desc_get_tx_len(struct dma_desc *p) > return p->des01.etx.buffer1_size; > } > > -static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) > +#define GMAC_VERSION_35 0x35 > +static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, > + u32 mac_id) > { > int ret = good_frame; > u32 status = (type << 2 | ipc_err << 1 | payload_err) & 0x7; > @@ -141,16 +144,16 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) > } else if (status == 0x1) { > CHIP_DBG(KERN_ERR > "RX Des0 status: IPv4/6 unsupported IP PAYLOAD.\n"); > - ret = discard_frame; > + ret = (mac_id >= GMAC_VERSION_35) ? discard_frame : csum_none; > } else if (status == 0x3) { > CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); > - ret = discard_frame; > + ret = (mac_id >= GMAC_VERSION_35) ? discard_frame : csum_none; > } > return ret; > } The enh_desc_coe_rdes0 parses the Receive Descriptor 0 When COE (Type 2). It should be onlyinvoked on full csum case. We also should discard the frames on the latest two cases I mean when: - IPv4/IPv6 Type frame with no IP header checksum error and the payload check bypassed, due to an unsupported payload - A Type frame that is neither IPv4 or IPv6 (the Checksum Offload engine bypasses checksum completely.) Also from all the Synopsys databooks I cannot see any difference in the Table 7.2 when treat the RDES0 bits 0, 5, 7. So I expect to have no check for the GMAC_VERSION_35 inside the enh desc file. > static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, > - struct dma_desc *p) > + struct dma_desc *p, u32 mac_id) > { > int ret = good_frame; > struct net_device_stats *stats = (struct net_device_stats *)data; > @@ -195,9 +198,11 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, > /* After a payload csum error, the ES bit is set. > * It doesn't match with the information reported into the databook. > * At any rate, we need to understand if the CSUM hw computation is ok > - * and report this info to the upper layers. */ > + * and report this info to the upper layers. > + */ This is useless change in the patch that should be removed in the final version. > ret = enh_desc_coe_rdes0(p->des01.erx.ipc_csum_error, > - p->des01.erx.frame_type, p->des01.erx.payload_csum_error); > + p->des01.erx.frame_type, > + p->des01.erx.payload_csum_error, mac_id); > > if (unlikely(p->des01.erx.dribbling)) { > CHIP_DBG(KERN_ERR "GMAC RX: dribbling error\n"); > diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c > index fda5d2b..057a728 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c > +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c > @@ -72,7 +72,7 @@ static int ndesc_get_tx_len(struct dma_desc *p) > * In case of success, it returns good_frame because the GMAC device > * is supposed to be able to compute the csum in HW. */ > static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x, > - struct dma_desc *p) > + struct dma_desc *p, u32 mac_id) > { > int ret = good_frame; > struct net_device_stats *stats = (struct net_device_stats *)data; > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 001b8f3..3bcdc97 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1103,6 +1103,7 @@ static int stmmac_open(struct net_device *dev) > priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); > if (priv->rx_coe) > pr_info(" Checksum Offload Engine supported\n"); > + do not add useless spaces. > if (priv->plat->tx_coe) > pr_info(" Checksum insertion supported\n"); Here I expect to see more information about the RX COE ;-) I'd like to see: pr_info(" Checksum Offload Engine supported (type %d)\n", ....); > > @@ -1436,7 +1437,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) > > /* read the status of the incoming frame */ > status = (priv->hw->desc->rx_status(&priv->dev->stats, > - &priv->xstats, p)); > + &priv->xstats, p, > + priv->hw->synopsys_uid & 0xff)); > if (unlikely(status == discard_frame)) > priv->dev->stats.rx_errors++; > else { > @@ -1444,6 +1446,16 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) > int frame_len; > > frame_len = priv->hw->desc->get_rx_frame_len(p); > + /* > + * The type-1 checksume offload engines append > + * the checksum at the end of frame, and the > + * the two bytes of checksum are added in > + * length. > + * Adjust for that in the framelen for type-1 > + * checksum offload engines. > + */ > + if (priv->plat->csum_off_engine_type == STMMAC_CSUM_T1) > + frame_len -= 2; I'd like to have this inside the core. I mean, get_rx_frame_len returns the len - 2 in case of type 1. > /* ACS is set; GMAC core strips PAD/FCS for IEEE 802.3 > * Type frames (LLC/LLC-SNAP) */ > if (unlikely(status != llc_snap)) -- 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
Hi Peppe, > Type 2 has been introduced starting from the 3.30a and Type 1 maintained > for back-ward compatibility because only available in 3.30. > > If we want to actually support Type 1 (I've no HW where test) I guess we > need to review this patch. > > First problem I can see in the patch is that, in case of type 1, we have > to properly set the device hw features because full IPC offload is not > supported (e.g. IPv6). This only is true for type 2. > > I've just looked at all the MAC data-books starting from the 3.30a to > 3.60a and I have seen that all the MACs treat the Receive Checksum > Offload Engine in the same way. I mean, the cores don't append any > payload csum bytes in case of type 2. This is always done for type 1! > > Frankly, I prefer to have no define like GMAC_VERSION_35. > I always tried to avoid it :-)... unless there is some critical reason > and we actually need it. Pls, see my comments comments inline below. There are two issues to be addresses. 1. Issue-1 : For the Type-1 Rx COE, the frame length has to be adjusted by 2. 2. The two frame status conditions that have been marked as csum_none for the versions 3.3a and earlier. I would take them along the review comments below >> } else if (status == 0x1) { >> CHIP_DBG(KERN_ERR >> "RX Des0 status: IPv4/6 unsupported IP PAYLOAD.\n"); >> - ret = discard_frame; >> + ret = (mac_id>= GMAC_VERSION_35) ? discard_frame : csum_none; >> } else if (status == 0x3) { >> CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); >> - ret = discard_frame; >> + ret = (mac_id>= GMAC_VERSION_35) ? discard_frame : csum_none; >> } >> return ret; >> } > The enh_desc_coe_rdes0 parses the Receive Descriptor 0 When COE (Type > 2). It should be onlyinvoked on full csum case. > We also should discard the frames on the latest two cases I mean when: > > - IPv4/IPv6 Type frame with no IP header checksum error and the payload > check bypassed, due to an unsupported payload > > - A Type frame that is neither IPv4 or IPv6 (the Checksum Offload engine > bypasses checksum completely.) > > Also from all the Synopsys databooks I cannot see any difference in the > Table 7.2 when treat the RDES0 bits 0, 5, 7. > > So I expect to have no check for the GMAC_VERSION_35 inside the enh desc > file. I can cross verify this on the SPEAr test platform which we had been using. We had faced some issue related to this before also. >> static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, >> - struct dma_desc *p) >> + struct dma_desc *p, u32 mac_id) >> { >> int ret = good_frame; >> struct net_device_stats *stats = (struct net_device_stats *)data; >> @@ -195,9 +198,11 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, >> /* After a payload csum error, the ES bit is set. >> * It doesn't match with the information reported into the databook. >> * At any rate, we need to understand if the CSUM hw computation is ok >> - * and report this info to the upper layers. */ >> + * and report this info to the upper layers. >> + */ > This is useless change in the patch that should be removed in the final > version. ok > if (priv->rx_coe) > pr_info(" Checksum Offload Engine supported\n"); > + > do not add useless spaces. ok >> if (priv->plat->tx_coe) >> pr_info(" Checksum insertion supported\n"); > Here I expect to see more information about the RX COE ;-) > > I'd like to see: > pr_info(" Checksum Offload Engine supported (type %d)\n", ....); Sure, will do that >> >> @@ -1436,7 +1437,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) >> >> /* read the status of the incoming frame */ >> status = (priv->hw->desc->rx_status(&priv->dev->stats, >> - &priv->xstats, p)); >> + &priv->xstats, p, >> + priv->hw->synopsys_uid& 0xff)); >> if (unlikely(status == discard_frame)) >> priv->dev->stats.rx_errors++; >> else { >> @@ -1444,6 +1446,16 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) >> int frame_len; >> >> frame_len = priv->hw->desc->get_rx_frame_len(p); >> + /* >> + * The type-1 checksume offload engines append >> + * the checksum at the end of frame, and the >> + * the two bytes of checksum are added in >> + * length. >> + * Adjust for that in the framelen for type-1 >> + * checksum offload engines. >> + */ >> + if (priv->plat->csum_off_engine_type == STMMAC_CSUM_T1) >> + frame_len -= 2; > I'd like to have this inside the core. I mean, get_rx_frame_len returns > the len - 2 in case of type 1. Thats fine. Will do that 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
Hi David, On 3/5/2012 7:21 AM, David Miller wrote: > From: Deepak Sikri<deepak.sikri@st.com> > Date: Fri, 2 Mar 2012 18:25:26 +0530 > >> +static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, >> + u32 mac_id) > Poorly formatted, this should be: Sorry for that.. Will rectify that > static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, > u32 mac_id) > > >> + /* >> + * The type-1 checksume offload engines append >> + * the checksum at the end of frame, and the > /* Format comments > * like this. > */ > > /* > * Not > * like this. > */ > Sure.. will do that 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/7/2012 9:25 AM, Deepak SIKRI wrote: > Hi Peppe, > > >> Type 2 has been introduced starting from the 3.30a and Type 1 maintained >> for back-ward compatibility because only available in 3.30. >> >> If we want to actually support Type 1 (I've no HW where test) I guess we >> need to review this patch. >> >> First problem I can see in the patch is that, in case of type 1, we have >> to properly set the device hw features because full IPC offload is not >> supported (e.g. IPv6). This only is true for type 2. >> >> I've just looked at all the MAC data-books starting from the 3.30a to >> 3.60a and I have seen that all the MACs treat the Receive Checksum >> Offload Engine in the same way. I mean, the cores don't append any >> payload csum bytes in case of type 2. This is always done for type 1! >> >> Frankly, I prefer to have no define like GMAC_VERSION_35. >> I always tried to avoid it :-)... unless there is some critical reason >> and we actually need it. Pls, see my comments comments inline below. > There are two issues to be addresses. > 1. Issue-1 : For the Type-1 Rx COE, the frame length has to be adjusted > by 2. agree but this could directly be done in get_rx_frame_len > 2. The two frame status conditions that have been marked as csum_none > for the versions 3.3a and > earlier. Earlier MACs have no Type 2 and the status condition enh_desc_coe_rdes0 only is for MAC that has Type 2 > > I would take them along the review comments below > > > >>> } else if (status == 0x1) { >>> CHIP_DBG(KERN_ERR >>> "RX Des0 status: IPv4/6 unsupported IP PAYLOAD.\n"); >>> - ret = discard_frame; >>> + ret = (mac_id>= GMAC_VERSION_35) ? discard_frame : csum_none; >>> } else if (status == 0x3) { >>> CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); >>> - ret = discard_frame; >>> + ret = (mac_id>= GMAC_VERSION_35) ? discard_frame : csum_none; >>> } >>> return ret; >>> } >> The enh_desc_coe_rdes0 parses the Receive Descriptor 0 When COE (Type >> 2). It should be onlyinvoked on full csum case. >> We also should discard the frames on the latest two cases I mean when: >> >> - IPv4/IPv6 Type frame with no IP header checksum error and the payload >> check bypassed, due to an unsupported payload >> >> - A Type frame that is neither IPv4 or IPv6 (the Checksum Offload engine >> bypasses checksum completely.) >> >> Also from all the Synopsys databooks I cannot see any difference in the >> Table 7.2 when treat the RDES0 bits 0, 5, 7. >> >> So I expect to have no check for the GMAC_VERSION_35 inside the enh desc >> file. > > I can cross verify this on the SPEAr test platform which we had been > using. We had faced some issue > related to this before also. > >>> static int enh_desc_get_rx_status(void *data, struct >>> stmmac_extra_stats *x, >>> - struct dma_desc *p) >>> + struct dma_desc *p, u32 mac_id) >>> { >>> int ret = good_frame; >>> struct net_device_stats *stats = (struct net_device_stats *)data; >>> @@ -195,9 +198,11 @@ static int enh_desc_get_rx_status(void *data, >>> struct stmmac_extra_stats *x, >>> /* After a payload csum error, the ES bit is set. >>> * It doesn't match with the information reported into the >>> databook. >>> * At any rate, we need to understand if the CSUM hw >>> computation is ok >>> - * and report this info to the upper layers. */ >>> + * and report this info to the upper layers. >>> + */ >> This is useless change in the patch that should be removed in the final >> version. > > ok > >> if (priv->rx_coe) >> pr_info(" Checksum Offload Engine supported\n"); >> + >> do not add useless spaces. > > ok > >>> if (priv->plat->tx_coe) >>> pr_info(" Checksum insertion supported\n"); >> Here I expect to see more information about the RX COE ;-) >> >> I'd like to see: >> pr_info(" Checksum Offload Engine supported (type %d)\n", ....); > > Sure, will do that :-) > >>> >>> @@ -1436,7 +1437,8 @@ static int stmmac_rx(struct stmmac_priv *priv, >>> int limit) >>> >>> /* read the status of the incoming frame */ >>> status = (priv->hw->desc->rx_status(&priv->dev->stats, >>> - &priv->xstats, p)); >>> + &priv->xstats, p, >>> + priv->hw->synopsys_uid& 0xff)); >>> if (unlikely(status == discard_frame)) >>> priv->dev->stats.rx_errors++; >>> else { >>> @@ -1444,6 +1446,16 @@ static int stmmac_rx(struct stmmac_priv *priv, >>> int limit) >>> int frame_len; >>> >>> frame_len = priv->hw->desc->get_rx_frame_len(p); >>> + /* >>> + * The type-1 checksume offload engines append >>> + * the checksum at the end of frame, and the >>> + * the two bytes of checksum are added in >>> + * length. >>> + * Adjust for that in the framelen for type-1 >>> + * checksum offload engines. >>> + */ >>> + if (priv->plat->csum_off_engine_type == STMMAC_CSUM_T1) >>> + frame_len -= 2; >> I'd like to have this inside the core. I mean, get_rx_frame_len returns >> the len - 2 in case of type 1. > > Thats fine. Will do that Thanks so much 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
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index d0b814e..12d1565 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -230,7 +230,7 @@ struct stmmac_desc_ops { int (*get_rx_frame_len) (struct dma_desc *p); /* Return the reception status looking at the RDES1 */ int (*rx_status) (void *data, struct stmmac_extra_stats *x, - struct dma_desc *p); + struct dma_desc *p, u32 mac_id); }; struct stmmac_dma_ops { diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c index d879763..42026f6 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" @@ -106,7 +107,9 @@ static int enh_desc_get_tx_len(struct dma_desc *p) return p->des01.etx.buffer1_size; } -static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) +#define GMAC_VERSION_35 0x35 +static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err, + u32 mac_id) { int ret = good_frame; u32 status = (type << 2 | ipc_err << 1 | payload_err) & 0x7; @@ -141,16 +144,16 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err) } else if (status == 0x1) { CHIP_DBG(KERN_ERR "RX Des0 status: IPv4/6 unsupported IP PAYLOAD.\n"); - ret = discard_frame; + ret = (mac_id >= GMAC_VERSION_35) ? discard_frame : csum_none; } else if (status == 0x3) { CHIP_DBG(KERN_ERR "RX Des0 status: No IPv4, IPv6 frame.\n"); - ret = discard_frame; + ret = (mac_id >= GMAC_VERSION_35) ? discard_frame : csum_none; } return ret; } static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, - struct dma_desc *p) + struct dma_desc *p, u32 mac_id) { int ret = good_frame; struct net_device_stats *stats = (struct net_device_stats *)data; @@ -195,9 +198,11 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x, /* After a payload csum error, the ES bit is set. * It doesn't match with the information reported into the databook. * At any rate, we need to understand if the CSUM hw computation is ok - * and report this info to the upper layers. */ + * and report this info to the upper layers. + */ ret = enh_desc_coe_rdes0(p->des01.erx.ipc_csum_error, - p->des01.erx.frame_type, p->des01.erx.payload_csum_error); + p->des01.erx.frame_type, + p->des01.erx.payload_csum_error, mac_id); if (unlikely(p->des01.erx.dribbling)) { CHIP_DBG(KERN_ERR "GMAC RX: dribbling error\n"); diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c index fda5d2b..057a728 100644 --- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c +++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c @@ -72,7 +72,7 @@ static int ndesc_get_tx_len(struct dma_desc *p) * In case of success, it returns good_frame because the GMAC device * is supposed to be able to compute the csum in HW. */ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x, - struct dma_desc *p) + struct dma_desc *p, u32 mac_id) { int ret = good_frame; struct net_device_stats *stats = (struct net_device_stats *)data; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 001b8f3..3bcdc97 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1103,6 +1103,7 @@ static int stmmac_open(struct net_device *dev) priv->rx_coe = priv->hw->mac->rx_coe(priv->ioaddr); if (priv->rx_coe) pr_info(" Checksum Offload Engine supported\n"); + if (priv->plat->tx_coe) pr_info(" Checksum insertion supported\n"); @@ -1436,7 +1437,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) /* read the status of the incoming frame */ status = (priv->hw->desc->rx_status(&priv->dev->stats, - &priv->xstats, p)); + &priv->xstats, p, + priv->hw->synopsys_uid & 0xff)); if (unlikely(status == discard_frame)) priv->dev->stats.rx_errors++; else { @@ -1444,6 +1446,16 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit) int frame_len; frame_len = priv->hw->desc->get_rx_frame_len(p); + /* + * The type-1 checksume offload engines append + * the checksum at the end of frame, and the + * the two bytes of checksum are added in + * length. + * Adjust for that in the framelen for type-1 + * checksum offload engines. + */ + if (priv->plat->csum_off_engine_type == STMMAC_CSUM_T1) + frame_len -= 2; /* ACS is set; GMAC core strips PAD/FCS for IEEE 802.3 * Type frames (LLC/LLC-SNAP) */ if (unlikely(status != llc_snap))
The patch adds in the following support. 1. The stmmac core supporting Type-1 checksum offload engine & prior to Revision 3.5, append the HW cecksum at the end of payload in case the Rx checksum engine was used to offload the HW checksum. There cores did not provide the HW register interface to read the device capabilities, and hence the plat code provides the core checksum capabilties that help to identify type-1 cores, and adjust the frame length. 2. Also, the following Frame status checks with the Full checksum offload Type-2 engine enabled are not backward compatible and are reserved for STMMAC core revisions prior to 3.5. Bit18(Eth Frame) Bit27(Header Csum Error) Bit28 (Payload Csum Err) 0 1 1 0 1 0 These conditions have been treated as no HW checksum support for stmmac core prior to rev-3.5. Signed-off-by: Deepak Sikri <deepak.sikri@st.com> --- drivers/net/ethernet/stmicro/stmmac/common.h | 2 +- drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 17 +++++++++++------ drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++++++++++++- 4 files changed, 26 insertions(+), 9 deletions(-)