[PATCH net 0/8] net: enetc: fix some known issues

[PATCH net 0/8] net: enetc: fix some known issues

[Wei Fang]

There are some issues with the enetc driver, some of which are specific
to the LS1028A platform, and some of which were introduced recently when
i.MX95 ENETC support was added, so this patch set aims to clean up those
issues.

Wei Fang (8):
  net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()
  net: enetc: correct the tx_swbd statistics
  net: enetc: correct the xdp_tx statistics
  net: enetc: VFs do not support HWTSTAMP_TX_ONESTEP_SYNC
  net: enetc: update UDP checksum when updating originTimestamp field
  net: enetc: add missing enetc4_link_deinit()
  net: enetc: remove the mm_lock from the ENETC v4 driver
  net: enetc: correct the EMDIO base offset for ENETC v4

 drivers/net/ethernet/freescale/enetc/enetc.c  | 53 +++++++++++++++----
 .../net/ethernet/freescale/enetc/enetc4_hw.h  |  3 ++
 .../net/ethernet/freescale/enetc/enetc4_pf.c  |  2 +-
 .../ethernet/freescale/enetc/enetc_ethtool.c  |  7 ++-
 .../freescale/enetc/enetc_pf_common.c         | 10 +++-
 5 files changed, 60 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

When the DMA mapping error occurs, it will attempt to free 'count + 1'
tx_swbd instead of 'count', so fix this off-by-one issue.

Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 6a6fc819dfde..f7bc2fc33a76 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -372,13 +372,13 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 dma_err:
 	dev_err(tx_ring->dev, "DMA map error");
 
-	do {
+	while (count--) {
 		tx_swbd = &tx_ring->tx_swbd[i];
 		enetc_free_tx_frame(tx_ring, tx_swbd);
 		if (i == 0)
 			i = tx_ring->bd_count;
 		i--;
-	} while (count--);
+	};
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Michal Swiatkowski]

On Mon, Feb 17, 2025 at 05:38:59PM +0800, Wei Fang wrote:
> When the DMA mapping error occurs, it will attempt to free 'count + 1'
> tx_swbd instead of 'count', so fix this off-by-one issue.
> 
> Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet drivers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 6a6fc819dfde..f7bc2fc33a76 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -372,13 +372,13 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  dma_err:
>  	dev_err(tx_ring->dev, "DMA map error");
>  
> -	do {
> +	while (count--) {
>  		tx_swbd = &tx_ring->tx_swbd[i];
>  		enetc_free_tx_frame(tx_ring, tx_swbd);
>  		if (i == 0)
>  			i = tx_ring->bd_count;
>  		i--;
> -	} while (count--);
> +	};

In enetc_lso_hw_offload() this is fixed by --count instead of changing
to while and count--, maybe follow this scheme, or event better call
helper function to fix in one place.

The same problem is probably in enetc_map_tx_tso_buffs().

Thanks
Michal

>  
>  	return 0;
>  }
> -- 
> 2.34.1
> 

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

> > Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet
> drivers")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/enetc/enetc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> > index 6a6fc819dfde..f7bc2fc33a76 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > @@ -372,13 +372,13 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >  dma_err:
> >  	dev_err(tx_ring->dev, "DMA map error");
> >
> > -	do {
> > +	while (count--) {
> >  		tx_swbd = &tx_ring->tx_swbd[i];
> >  		enetc_free_tx_frame(tx_ring, tx_swbd);
> >  		if (i == 0)
> >  			i = tx_ring->bd_count;
> >  		i--;
> > -	} while (count--);
> > +	};
> 
> In enetc_lso_hw_offload() this is fixed by --count instead of changing
> to while and count--, maybe follow this scheme, or event better call
> helper function to fix in one place.

The situation is slightly different in enetc_map_tx_buffs(), the count
may be 0 when the error occurs. But in enetc_lso_hw_offload(), the
count will not be 0 when the error occurs.

> 
> The same problem is probably in enetc_map_tx_tso_buffs().
> 

I think there is no such problem in enetc_map_tx_tso_buffs(),
because the index 'i' has been increased before the error occurs,
but the count is not increased, so the actual 'count' is count + 1.

Re: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Michal Swiatkowski]

On Tue, Feb 18, 2025 at 02:11:12AM +0000, Wei Fang wrote:
> > > Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet
> > drivers")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > ---
> > >  drivers/net/ethernet/freescale/enetc/enetc.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> > b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > index 6a6fc819dfde..f7bc2fc33a76 100644
> > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > @@ -372,13 +372,13 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> > *tx_ring, struct sk_buff *skb)
> > >  dma_err:
> > >  	dev_err(tx_ring->dev, "DMA map error");
> > >
> > > -	do {
> > > +	while (count--) {
> > >  		tx_swbd = &tx_ring->tx_swbd[i];
> > >  		enetc_free_tx_frame(tx_ring, tx_swbd);
> > >  		if (i == 0)
> > >  			i = tx_ring->bd_count;
> > >  		i--;
> > > -	} while (count--);
> > > +	};
> > 
> > In enetc_lso_hw_offload() this is fixed by --count instead of changing
> > to while and count--, maybe follow this scheme, or event better call
> > helper function to fix in one place.
> 
> The situation is slightly different in enetc_map_tx_buffs(), the count
> may be 0 when the error occurs. But in enetc_lso_hw_offload(), the
> count will not be 0 when the error occurs.

Right, didn't see that, sorry.

> 
> > 
> > The same problem is probably in enetc_map_tx_tso_buffs().
> > 
> 
> I think there is no such problem in enetc_map_tx_tso_buffs(),
> because the index 'i' has been increased before the error occurs,
> but the count is not increased, so the actual 'count' is count + 1.
>

But you can reach the error code jumping to label "err_chained_bd" where
both i and count is increased. Isn't it a problem?

BTW, not increasing i and count in one place looks like unnecessary
complication ;) .

Thanks,
Michal

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

> -----Original Message-----
> From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Sent: 2025年2月18日 13:45
> To: Wei Fang <wei.fang@nxp.com>
> Cc: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>; Claudiu Manoil
> <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>;
> Clark Wang <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; Ioana Ciornei <ioana.ciornei@nxp.com>; Y.B. Lu
> <yangbo.lu@nxp.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> imx@lists.linux.dev; stable@vger.kernel.org
> Subject: Re: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> enetc_map_tx_buffs()
> 
> On Tue, Feb 18, 2025 at 02:11:12AM +0000, Wei Fang wrote:
> > > > Fixes: d4fd0404c1c9 ("enetc: Introduce basic PF and VF ENETC ethernet
> > > drivers")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > > > ---
> > > >  drivers/net/ethernet/freescale/enetc/enetc.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > index 6a6fc819dfde..f7bc2fc33a76 100644
> > > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > @@ -372,13 +372,13 @@ static int enetc_map_tx_buffs(struct
> enetc_bdr
> > > *tx_ring, struct sk_buff *skb)
> > > >  dma_err:
> > > >  	dev_err(tx_ring->dev, "DMA map error");
> > > >
> > > > -	do {
> > > > +	while (count--) {
> > > >  		tx_swbd = &tx_ring->tx_swbd[i];
> > > >  		enetc_free_tx_frame(tx_ring, tx_swbd);
> > > >  		if (i == 0)
> > > >  			i = tx_ring->bd_count;
> > > >  		i--;
> > > > -	} while (count--);
> > > > +	};
> > >
> > > In enetc_lso_hw_offload() this is fixed by --count instead of changing
> > > to while and count--, maybe follow this scheme, or event better call
> > > helper function to fix in one place.
> >
> > The situation is slightly different in enetc_map_tx_buffs(), the count
> > may be 0 when the error occurs. But in enetc_lso_hw_offload(), the
> > count will not be 0 when the error occurs.
> 
> Right, didn't see that, sorry.
> 
> >
> > >
> > > The same problem is probably in enetc_map_tx_tso_buffs().
> > >
> >
> > I think there is no such problem in enetc_map_tx_tso_buffs(),
> > because the index 'i' has been increased before the error occurs,
> > but the count is not increased, so the actual 'count' is count + 1.
> >
> 
> But you can reach the error code jumping to label "err_chained_bd" where
> both i and count is increased. Isn't it a problem?
> 
You are right, I ignored this label. I will add a separate patch to fix this issue.
Thanks.

> BTW, not increasing i and count in one place looks like unnecessary
> complication ;) .
> 

Re: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Jakub Kicinski]

On Mon, 17 Feb 2025 17:38:59 +0800 Wei Fang wrote:
> +	while (count--) {
>  		tx_swbd = &tx_ring->tx_swbd[i];
>  		enetc_free_tx_frame(tx_ring, tx_swbd);
>  		if (i == 0)
>  			i = tx_ring->bd_count;
>  		i--;
> -	} while (count--);
> +	};

I think this gives us:

drivers/net/ethernet/freescale/enetc/enetc.c:408:2-3: Unneeded semicolon
-- 
pw-bot: cr

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

> On Mon, 17 Feb 2025 17:38:59 +0800 Wei Fang wrote:
> > +	while (count--) {
> >  		tx_swbd = &tx_ring->tx_swbd[i];
> >  		enetc_free_tx_frame(tx_ring, tx_swbd);
> >  		if (i == 0)
> >  			i = tx_ring->bd_count;
> >  		i--;
> > -	} while (count--);
> > +	};
> 
> I think this gives us:
> 
> drivers/net/ethernet/freescale/enetc/enetc.c:408:2-3: Unneeded semicolon

Thanks, will fix it.

> --
> pw-bot: cr

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Claudiu Manoil]

> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Monday, February 17, 2025 11:39 AM
[...]
> Subject: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> enetc_map_tx_buffs()
> 
> When the DMA mapping error occurs, it will attempt to free 'count + 1'

Hi Wei,
Are you sure?

dma_err occurs before count is incremented, at least that's the design.

First step already contradicts your claim:
```
static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
{
[...]
	int i, count = 0;
[...]
	dma = dma_map_single(tx_ring->dev, skb->data, len, DMA_TO_DEVICE);
	if (unlikely(dma_mapping_error(tx_ring->dev, dma)))
		goto dma_err;

===> count is 0 on goto path!
[...]
	count++;
```

Regards,
Claudiu

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

> > -----Original Message-----
> > From: Wei Fang <wei.fang@nxp.com>
> > Sent: Monday, February 17, 2025 11:39 AM
> [...]
> > Subject: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> > enetc_map_tx_buffs()
> >
> > When the DMA mapping error occurs, it will attempt to free 'count + 1'
> 
> Hi Wei,
> Are you sure?
> 
> dma_err occurs before count is incremented, at least that's the design.
> 
> First step already contradicts your claim:
> ```
> static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
> { [...]
> 	int i, count = 0;
> [...]
> 	dma = dma_map_single(tx_ring->dev, skb->data, len, DMA_TO_DEVICE);
> 	if (unlikely(dma_mapping_error(tx_ring->dev, dma)))
> 		goto dma_err;
> 
> ===> count is 0 on goto path!
> [...]
> 	count++;
> ```
> 

Hi Claudiu,

I think it's fine in your case, the count is 0, and the tx_swbd is not set,
so it's unnecessary to call enetc_free_tx_frame() in the dma_err path.

But if the count is not zero, then that is the problem, for example, count
is 1, and then goto dma_err path, the it will attempt to free 2 tx_swbd.

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Claudiu Manoil]

> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Tuesday, February 18, 2025 4:03 AM
[,,,]
> Subject: RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> enetc_map_tx_buffs()
> 
> > > -----Original Message-----
> > > From: Wei Fang <wei.fang@nxp.com>
> > > Sent: Monday, February 17, 2025 11:39 AM
> > [...]
> > > Subject: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> > > enetc_map_tx_buffs()
> > >
> > > When the DMA mapping error occurs, it will attempt to free 'count + 1'
> >
> > Hi Wei,
> > Are you sure?
> >
> > dma_err occurs before count is incremented, at least that's the design.
> >
> > First step already contradicts your claim:
> > ```
> > static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
> > { [...]
> > 	int i, count = 0;
> > [...]
> > 	dma = dma_map_single(tx_ring->dev, skb->data, len,
> DMA_TO_DEVICE);
> > 	if (unlikely(dma_mapping_error(tx_ring->dev, dma)))
> > 		goto dma_err;
> >
> > ===> count is 0 on goto path!
> > [...]
> > 	count++;
> > ```
> >
> 
> Hi Claudiu,
> 
> I think it's fine in your case, the count is 0, and the tx_swbd is not set,
> so it's unnecessary to call enetc_free_tx_frame() in the dma_err path.
> 

enetc_free_tx_frame() call on dma_err path is still needed for count 0 because
it needs to free the skb.

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Wei Fang]

> > -----Original Message-----
> > From: Wei Fang <wei.fang@nxp.com>
> > Sent: Tuesday, February 18, 2025 4:03 AM
> [,,,]
> > Subject: RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> > enetc_map_tx_buffs()
> >
> > > > -----Original Message-----
> > > > From: Wei Fang <wei.fang@nxp.com>
> > > > Sent: Monday, February 17, 2025 11:39 AM
> > > [...]
> > > > Subject: [PATCH net 1/8] net: enetc: fix the off-by-one issue in
> > > > enetc_map_tx_buffs()
> > > >
> > > > When the DMA mapping error occurs, it will attempt to free 'count + 1'
> > >
> > > Hi Wei,
> > > Are you sure?
> > >
> > > dma_err occurs before count is incremented, at least that's the design.
> > >
> > > First step already contradicts your claim:
> > > ```
> > > static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff
> *skb)
> > > { [...]
> > > 	int i, count = 0;
> > > [...]
> > > 	dma = dma_map_single(tx_ring->dev, skb->data, len,
> > DMA_TO_DEVICE);
> > > 	if (unlikely(dma_mapping_error(tx_ring->dev, dma)))
> > > 		goto dma_err;
> > >
> > > ===> count is 0 on goto path!
> > > [...]
> > > 	count++;
> > > ```
> > >
> >
> > Hi Claudiu,
> >
> > I think it's fine in your case, the count is 0, and the tx_swbd is not set,
> > so it's unnecessary to call enetc_free_tx_frame() in the dma_err path.
> >
> 
> enetc_free_tx_frame() call on dma_err path is still needed for count 0 because
> it needs to free the skb.

First, the tx_swbd is not set when count is 0, so tx_swbd->skb is NULL when
the error occurs.

Second, the skb is freed in enetc_start_xmit() not enetc_free_tx_frame().

RE: [PATCH net 1/8] net: enetc: fix the off-by-one issue in enetc_map_tx_buffs()

[Claudiu Manoil]

> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Tuesday, February 18, 2025 11:18 AM
[...]
> > enetc_free_tx_frame() call on dma_err path is still needed for count 0
> > because it needs to free the skb.
> 
> First, the tx_swbd is not set when count is 0, so tx_swbd->skb is NULL when
> the error occurs.
> 
> Second, the skb is freed in enetc_start_xmit() not enetc_free_tx_frame().

Yeah, noticed too finally.
'do {} while()' is not ok on error path here.

[PATCH net 2/8] net: enetc: correct the tx_swbd statistics

[Wei Fang]

When creating a TSO header, if the skb is VLAN tagged, the extended BD
will be used and the 'count' should be increased by 2 instead of 1.
Otherwise, when an error occurs, less tx_swbd will be freed than the
actual number.

Fixes: fb8629e2cbfc ("net: enetc: add support for software TSO")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index f7bc2fc33a76..0a1cea368280 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -759,6 +759,7 @@ static int enetc_lso_hw_offload(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 {
 	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
+	bool ext_bd = skb_vlan_tag_present(skb);
 	int hdr_len, total_len, data_len;
 	struct enetc_tx_swbd *tx_swbd;
 	union enetc_tx_bd *txbd;
@@ -792,7 +793,7 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb
 		csum = enetc_tso_hdr_csum(&tso, skb, hdr, hdr_len, &pos);
 		enetc_map_tx_tso_hdr(tx_ring, skb, tx_swbd, txbd, &i, hdr_len, data_len);
 		bd_data_num = 0;
-		count++;
+		count += ext_bd ? 2 : 1;
 
 		while (data_len > 0) {
 			int size;
-- 
2.34.1

Re: [PATCH net 2/8] net: enetc: correct the tx_swbd statistics

[Michal Swiatkowski]

On Mon, Feb 17, 2025 at 05:39:00PM +0800, Wei Fang wrote:
> When creating a TSO header, if the skb is VLAN tagged, the extended BD
> will be used and the 'count' should be increased by 2 instead of 1.
> Otherwise, when an error occurs, less tx_swbd will be freed than the
> actual number.
> 
> Fixes: fb8629e2cbfc ("net: enetc: add support for software TSO")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index f7bc2fc33a76..0a1cea368280 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -759,6 +759,7 @@ static int enetc_lso_hw_offload(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  {
>  	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
> +	bool ext_bd = skb_vlan_tag_present(skb);
>  	int hdr_len, total_len, data_len;
>  	struct enetc_tx_swbd *tx_swbd;
>  	union enetc_tx_bd *txbd;
> @@ -792,7 +793,7 @@ static int enetc_map_tx_tso_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb
>  		csum = enetc_tso_hdr_csum(&tso, skb, hdr, hdr_len, &pos);
>  		enetc_map_tx_tso_hdr(tx_ring, skb, tx_swbd, txbd, &i, hdr_len, data_len);
>  		bd_data_num = 0;
> -		count++;
> +		count += ext_bd ? 2 : 1;

Looks fine, beside you need to fix the unroll like in patch 1.
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>


>  
>  		while (data_len > 0) {
>  			int size;
> -- 
> 2.34.1

[PATCH net 3/8] net: enetc: correct the xdp_tx statistics

[Wei Fang]

The 'xdp_tx' is used to count the number of XDP_TX frames sent, not the
number of Tx BDs.

Fixes: 7ed2bc80074e ("net: enetc: add support for XDP_TX")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 0a1cea368280..eb0d7ef73303 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1902,7 +1902,7 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
 				enetc_xdp_drop(rx_ring, orig_i, i);
 				tx_ring->stats.xdp_tx_drops++;
 			} else {
-				tx_ring->stats.xdp_tx += xdp_tx_bd_cnt;
+				tx_ring->stats.xdp_tx++;
 				rx_ring->xdp.xdp_tx_in_flight += xdp_tx_bd_cnt;
 				xdp_tx_frm_cnt++;
 				/* The XDP_TX enqueue was successful, so we
-- 
2.34.1

[PATCH net 4/8] net: enetc: VFs do not support HWTSTAMP_TX_ONESTEP_SYNC

[Wei Fang]

Actually ENETC VFs do not support HWTSTAMP_TX_ONESTEP_SYNC because only
ENETC PF can access PMa_SINGLE_STEP registers. And there will be a crash
if VFs are used to test one-step timestamp, the crash log as follows.

[  129.110909] Unable to handle kernel paging request at virtual address 00000000000080c0
[  129.287769] Call trace:
[  129.290219]  enetc_port_mac_wr+0x30/0xec (P)
[  129.294504]  enetc_start_xmit+0xda4/0xe74
[  129.298525]  enetc_xmit+0x70/0xec
[  129.301848]  dev_hard_start_xmit+0x98/0x118

Fixes: 41514737ecaa ("enetc: add get_ts_info interface for ethtool")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c         | 3 +++
 drivers/net/ethernet/freescale/enetc/enetc_ethtool.c | 7 +++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index eb0d7ef73303..6b84c5ac7c36 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -3229,6 +3229,9 @@ static int enetc_hwtstamp_set(struct net_device *ndev, struct ifreq *ifr)
 		new_offloads |= ENETC_F_TX_TSTAMP;
 		break;
 	case HWTSTAMP_TX_ONESTEP_SYNC:
+		if (!enetc_si_is_pf(priv->si))
+			return -EOPNOTSUPP;
+
 		new_offloads &= ~ENETC_F_TX_TSTAMP_MASK;
 		new_offloads |= ENETC_F_TX_ONESTEP_SYNC_TSTAMP;
 		break;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
index bf34b5bb1e35..ece3ae28ba82 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_ethtool.c
@@ -832,6 +832,7 @@ static int enetc_set_coalesce(struct net_device *ndev,
 static int enetc_get_ts_info(struct net_device *ndev,
 			     struct kernel_ethtool_ts_info *info)
 {
+	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 	int *phc_idx;
 
 	phc_idx = symbol_get(enetc_phc_index);
@@ -852,8 +853,10 @@ static int enetc_get_ts_info(struct net_device *ndev,
 				SOF_TIMESTAMPING_TX_SOFTWARE;
 
 	info->tx_types = (1 << HWTSTAMP_TX_OFF) |
-			 (1 << HWTSTAMP_TX_ON) |
-			 (1 << HWTSTAMP_TX_ONESTEP_SYNC);
+			 (1 << HWTSTAMP_TX_ON);
+
+	if (enetc_si_is_pf(priv->si))
+		info->tx_types |= (1 << HWTSTAMP_TX_ONESTEP_SYNC);
 
 	info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
 			   (1 << HWTSTAMP_FILTER_ALL);
-- 
2.34.1

[PATCH net 5/8] net: enetc: update UDP checksum when updating originTimestamp field

[Wei Fang]

There is an issue with one-step timestamp based on UDP/IP. The peer will
discard the sync packet because of the wrong UDP checksum. For ENETC v1,
the software needs to update the UDP checksum when updating the
originTimestamp field, so that the hardware can correctly update the UDP
checksum when updating the correction field. Otherwise, the UDP checksum
in the sync packet will be wrong.

Fixes: 7294380c5211 ("enetc: support PTP Sync packet one-step timestamping")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 41 ++++++++++++++++----
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 6b84c5ac7c36..cb05b881ba66 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -279,9 +279,11 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 		}
 
 		if (do_onestep_tstamp) {
-			u32 lo, hi, val;
-			u64 sec, nsec;
+			__be32 new_sec_l, new_nsec;
+			u32 lo, hi, nsec, val;
+			__be16 new_sec_h;
 			u8 *data;
+			u64 sec;
 
 			lo = enetc_rd_hot(hw, ENETC_SICTR0);
 			hi = enetc_rd_hot(hw, ENETC_SICTR1);
@@ -295,13 +297,38 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 			/* Update originTimestamp field of Sync packet
 			 * - 48 bits seconds field
 			 * - 32 bits nanseconds field
+			 *
+			 * In addition, the UDP checksum needs to be updated
+			 * by software after updating originTimestamp field,
+			 * otherwise the hardware will calculate the wrong
+			 * checksum when updating the correction field and
+			 * update it to the packet.
 			 */
 			data = skb_mac_header(skb);
-			*(__be16 *)(data + offset2) =
-				htons((sec >> 32) & 0xffff);
-			*(__be32 *)(data + offset2 + 2) =
-				htonl(sec & 0xffffffff);
-			*(__be32 *)(data + offset2 + 6) = htonl(nsec);
+			new_sec_h = htons((sec >> 32) & 0xffff);
+			new_sec_l = htonl(sec & 0xffffffff);
+			new_nsec = htonl(nsec);
+			if (udp) {
+				struct udphdr *uh = udp_hdr(skb);
+				__be32 old_sec_l, old_nsec;
+				__be16 old_sec_h;
+
+				old_sec_h = *(__be16 *)(data + offset2);
+				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
+							 new_sec_h, false);
+
+				old_sec_l = *(__be32 *)(data + offset2 + 2);
+				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
+							 new_sec_l, false);
+
+				old_nsec = *(__be32 *)(data + offset2 + 6);
+				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
+							 new_nsec, false);
+			}
+
+			*(__be16 *)(data + offset2) = new_sec_h;
+			*(__be32 *)(data + offset2 + 2) = new_sec_l;
+			*(__be32 *)(data + offset2 + 6) = new_nsec;
 
 			/* Configure single-step register */
 			val = ENETC_PM0_SINGLE_STEP_EN;
-- 
2.34.1

[PATCH net 6/8] net: enetc: add missing enetc4_link_deinit()

[Wei Fang]

The enetc4_link_init() is called when the PF driver probes to create
phylink and MDIO bus, but we forgot to call enetc4_link_deinit() to
free the phylink and MDIO bus when the driver was unbound. so add
missing enetc4_link_deinit() to enetc4_pf_netdev_destroy().

Fixes: 99100d0d9922 ("net: enetc: add preliminary support for i.MX95 ENETC PF")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index fc41078c4f5d..48861c8b499a 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -684,6 +684,7 @@ static void enetc4_pf_netdev_destroy(struct enetc_si *si)
 	struct net_device *ndev = si->ndev;
 
 	unregister_netdev(ndev);
+	enetc4_link_deinit(priv);
 	enetc_free_msix(priv);
 	free_netdev(ndev);
 }
-- 
2.34.1

[PATCH net 7/8] net: enetc: remove the mm_lock from the ENETC v4 driver

[Wei Fang]

Currently, the ENETC v4 driver has not added the MAC merge layer support
in the upstream, so the mm_lock is not initialized and used, so remove
the mm_lock from the driver.

Fixes: 99100d0d9922 ("net: enetc: add preliminary support for i.MX95 ENETC PF")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
index 48861c8b499a..73ac8c6afb3a 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
@@ -672,7 +672,6 @@ static int enetc4_pf_netdev_create(struct enetc_si *si)
 err_alloc_msix:
 err_config_si:
 err_clk_get:
-	mutex_destroy(&priv->mm_lock);
 	free_netdev(ndev);
 
 	return err;
-- 
2.34.1

[PATCH net 8/8] net: enetc: correct the EMDIO base offset for ENETC v4

[Wei Fang]

In addition to centrally managing external PHYs through EMIDO device,
each ENETC has a set of EMDIO registers to access and manage its own
external PHY. When adding i.MX95 ENETC support, the EMDIO base offset
was forgot to be updated, which will result in ENETC being unable to
manage its external PHY through its own EMDIO registers.

Fixes: 99100d0d9922 ("net: enetc: add preliminary support for i.MX95 ENETC PF")
Cc: stable@vger.kernel.org
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc4_hw.h       |  3 +++
 drivers/net/ethernet/freescale/enetc/enetc_pf_common.c | 10 +++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
index 695cb07c74bc..02d627e2cca6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
@@ -175,4 +175,7 @@
 #define   SSP_1G			2
 #define  PM_IF_MODE_ENA			BIT(15)
 
+/* Port external MDIO Base address, use to access off-chip PHY */
+#define ENETC4_EMDIO_BASE		0x5c00
+
 #endif
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 3fd9b0727875..13e2db561c22 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -154,6 +154,14 @@ void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
 }
 EXPORT_SYMBOL_GPL(enetc_pf_netdev_setup);
 
+static int enetc_get_mdio_base(struct enetc_si *si)
+{
+	if (is_enetc_rev1(si))
+		return ENETC_EMDIO_BASE;
+
+	return ENETC4_EMDIO_BASE;
+}
+
 static int enetc_mdio_probe(struct enetc_pf *pf, struct device_node *np)
 {
 	struct device *dev = &pf->si->pdev->dev;
@@ -173,7 +181,7 @@ static int enetc_mdio_probe(struct enetc_pf *pf, struct device_node *np)
 	bus->parent = dev;
 	mdio_priv = bus->priv;
 	mdio_priv->hw = &pf->si->hw;
-	mdio_priv->mdio_base = ENETC_EMDIO_BASE;
+	mdio_priv->mdio_base = enetc_get_mdio_base(pf->si);
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
 
 	err = of_mdiobus_register(bus, np);
-- 
2.34.1