Re: [PATCH net-next v3] net: stmmac: dwxgmac2: enable L2 split header support

[Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

> 
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> >> index 6719ac6e395b..65ac5403ec58 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
> >> @@ -292,8 +292,18 @@ static int dwxgmac2_get_rx_hash(struct dma_desc *p, u32 *hash,
> >>  
> >>  static void dwxgmac2_get_rx_header_len(struct dma_desc *p, unsigned int *len)
> >>  {
> >> -	if (le32_to_cpu(p->des3) & XGMAC_RDES3_L34T)
> >> +	u32 rdes3 = le32_to_cpu(p->des3);
> >> +
> >> +	/* when FD=1 and LD=0, HL is RDES2[9:0] */
> >> +	if (!(rdes3 & XGMAC_RDES3_LD)) {
> >> +		*len = le32_to_cpu(p->des2) & XGMAC_RDES2_HL;
> >> +		return;
> >> +	}
> >> +
> >> +	if (rdes3 & XGMAC_RDES3_L34T)
> > should we check FD=1 here? Since HL is not defined for the case {FD=0,LD=1},
> > right?
> >
> >>  		*len = le32_to_cpu(p->des2) & XGMAC_RDES2_HL;
> >> +	else if (rdes3 & XGMAC_RDES3_L2T)
> >> +		*len = (le32_to_cpu(p->des2) & XGMAC_RDES2_NONIPHL) >> 2;
> > same here.
> >
> > Regards,
> > Lorenzo
> >
> Hi Lorenzo,
> 
> Thanks for the review. dwxgmac2_get_rx_header_len() is only called from stmmac_rx_buf1_len(),
> after the sph_active && len early return, so with split header enabled it only runs on the
> first descriptor (FD=1). I don't think an extra FD check is needed, but I can add one if you'd prefer.

I guess this approach would be more robust.

Regards,
Lorenzo

> 
> Regards,
> Sagar
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]