One bug fix, one small functional change and a couple of
cleanup patches.
Ioana Radulescu (5):
staging: fsl-dpaa2/eth: Fix DMA mapping direction
staging: fsl-dpaa2/eth: Remove obsolete reference
staging: fsl-dpaa2/eth: Remove pointless instruction
staging: fsl-dpaa2/eth: MTU cleanup
staging: fsl-dpaa2/eth: Remove Rx frame size check
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 34 ++++++--------------------
1 file changed, 8 insertions(+), 26 deletions(-)
--
2.7.4
Most Ethernet drivers don't enforce the MTU value as upper limit
for ingress frames. We too support receiving frames larger than
MTU, so allow that.
Remove our ndo_change_mtu implementation, letting the default
stack implementation handle things. Also, set the max frame length
allowed by hardware only once at probe time, with the largest
possible value.
Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 24e069c..4ae2371 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -1243,25 +1243,6 @@ static void dpaa2_eth_get_stats(struct net_device *net_dev,
}
}
-static int dpaa2_eth_change_mtu(struct net_device *net_dev, int mtu)
-{
- struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
- int err;
-
- /* Set the maximum Rx frame length to match the transmit side;
- * account for L2 headers when computing the MFL
- */
- err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token,
- (u16)DPAA2_ETH_L2_MAX_FRM(mtu));
- if (err) {
- netdev_err(net_dev, "dpni_set_max_frame_length() failed\n");
- return err;
- }
-
- net_dev->mtu = mtu;
- return 0;
-}
-
/* Copy mac unicast addresses from @net_dev to @priv.
* Its sole purpose is to make dpaa2_eth_set_rx_mode() more readable.
*/
@@ -1469,7 +1450,6 @@ static const struct net_device_ops dpaa2_eth_ops = {
.ndo_init = dpaa2_eth_init,
.ndo_set_mac_address = dpaa2_eth_set_addr,
.ndo_get_stats64 = dpaa2_eth_get_stats,
- .ndo_change_mtu = dpaa2_eth_change_mtu,
.ndo_set_rx_mode = dpaa2_eth_set_rx_mode,
.ndo_set_features = dpaa2_eth_set_features,
.ndo_do_ioctl = dpaa2_eth_ioctl,
@@ -2385,6 +2365,12 @@ static int netdev_init(struct net_device *net_dev)
/* Set MTU upper limit; lower limit is 68B (default value) */
net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
+ err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token,
+ (u16)DPAA2_ETH_MFL);
+ if (err) {
+ dev_err(dev, "dpni_set_max_frame_length() failed\n");
+ return err;
+ }
/* Set actual number of queues in the net device */
num_queues = dpaa2_eth_queue_count(priv);
--
2.7.4
Commit 2b7c86eb7bf3 ("staging: fsl-dpaa2/eth: Don't enable FAS on Tx")
removed the status field from the TX confirm frame annotation,
but a reference to it remained in the description of free_tx_fd().
Remove it.
Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 537d5bb..6331e8d 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -522,8 +522,6 @@ static int build_single_fd(struct dpaa2_eth_priv *priv,
* back-pointed to is also freed.
* This can be called either from dpaa2_eth_tx_conf() or on the error path of
* dpaa2_eth_tx().
- * Optionally, return the frame annotation status word (FAS), which needs
- * to be checked if we're on the confirmation path.
*/
static void free_tx_fd(const struct dpaa2_eth_priv *priv,
const struct dpaa2_fd *fd)
--
2.7.4
Don't set the lower MTU limit explicitly, since we use
the default value anyway.
Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 439b260..24e069c 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -2383,8 +2383,7 @@ static int netdev_init(struct net_device *net_dev)
return err;
}
- /* Set MTU limits */
- net_dev->min_mtu = 68;
+ /* Set MTU upper limit; lower limit is 68B (default value) */
net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
/* Set actual number of queues in the net device */
--
2.7.4
We don't need to call dev_set_drvdata(dev, NULL) on driver
remove since core kernel code also performs this step.
Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 6331e8d..439b260 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -2676,7 +2676,6 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
fsl_mc_portal_free(priv->mc_io);
- dev_set_drvdata(dev, NULL);
free_netdev(net_dev);
dev_dbg(net_dev->dev.parent, "Removed interface %s\n", net_dev->name);
--
2.7.4
We are using DMA_FROM_DEVICE when mapping RX frame buffers,
but DMA_BIDIRECTIONAL for unmap. Fix the direction for DMA
unmapping operation.
Fixes: 87eb55e418b7 ("staging: fsl-dpaa2/eth: Fix potential endless loop")
Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 3963717..537d5bb 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -767,7 +767,7 @@ static void free_bufs(struct dpaa2_eth_priv *priv, u64 *buf_array, int count)
for (i = 0; i < count; i++) {
vaddr = dpaa2_iova_to_virt(priv->iommu_domain, buf_array[i]);
dma_unmap_single(dev, buf_array[i], DPAA2_ETH_RX_BUF_SIZE,
- DMA_BIDIRECTIONAL);
+ DMA_FROM_DEVICE);
skb_free_frag(vaddr);
}
}
--
2.7.4
On Mon, Jul 09, 2018 at 10:01:11AM -0500, Ioana Radulescu wrote:
> @@ -2385,6 +2365,12 @@ static int netdev_init(struct net_device *net_dev)
>
> /* Set MTU upper limit; lower limit is 68B (default value) */
> net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
> + err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token,
> + (u16)DPAA2_ETH_MFL);
The cast was there in the original code so this is not a comment on this
particular patch (which seems fine) but there is no need to cast.
Generally it's best to avoid unnecessary casts. As a human reader, I
find the cast confusing. It indicates that DPAA2_ETH_MFL somehow
requires special handling. Perhaps it's negative or we are trying to
truncate away the high bits. But neither of those things really make
sense.
From a static analysis perspective if DPAA2_ETH_MFL doesn't fit nicely
then we would want to generate a warning. But the cast explicitly
disables the check.
regards,
dan carpenter
> -----Original Message-----
> From: Dan Carpenter [mailto:[email protected]]
> Sent: Monday, July 9, 2018 10:28 PM
> To: Ioana Ciocoi Radulescu <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; Ioana Ciornei <[email protected]>
> Subject: Re: [PATCH 5/5] staging: fsl-dpaa2/eth: Remove Rx frame size check
>
> On Mon, Jul 09, 2018 at 10:01:11AM -0500, Ioana Radulescu wrote:
> > @@ -2385,6 +2365,12 @@ static int netdev_init(struct net_device
> *net_dev)
> >
> > /* Set MTU upper limit; lower limit is 68B (default value) */
> > net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
> > + err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token,
> > + (u16)DPAA2_ETH_MFL);
>
> The cast was there in the original code so this is not a comment on this
> particular patch (which seems fine) but there is no need to cast.
>
> Generally it's best to avoid unnecessary casts. As a human reader, I
> find the cast confusing. It indicates that DPAA2_ETH_MFL somehow
> requires special handling. Perhaps it's negative or we are trying to
> truncate away the high bits. But neither of those things really make
> sense.
>
> From a static analysis perspective if DPAA2_ETH_MFL doesn't fit nicely
> then we would want to generate a warning. But the cast explicitly
> disables the check.
I really don't remember why the cast was there in the first place.
It doesn't look like it's needed anymore, DPAA2_ETH_MFL has a
positive value (around 10K) that fits just fine inside a u16.
I see Greg already applied the patch, so I'll send a separate one to
remove the cast.
Thanks,
Ioana
On Tue, Jul 10, 2018 at 03:55:32PM +0000, Ioana Ciocoi Radulescu wrote:
> > -----Original Message-----
> > From: Dan Carpenter [mailto:[email protected]]
> > Sent: Monday, July 9, 2018 10:28 PM
> > To: Ioana Ciocoi Radulescu <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; Ioana Ciornei <[email protected]>
> > Subject: Re: [PATCH 5/5] staging: fsl-dpaa2/eth: Remove Rx frame size check
> >
> > On Mon, Jul 09, 2018 at 10:01:11AM -0500, Ioana Radulescu wrote:
> > > @@ -2385,6 +2365,12 @@ static int netdev_init(struct net_device
> > *net_dev)
> > >
> > > /* Set MTU upper limit; lower limit is 68B (default value) */
> > > net_dev->max_mtu = DPAA2_ETH_MAX_MTU;
> > > + err = dpni_set_max_frame_length(priv->mc_io, 0, priv->mc_token,
> > > + (u16)DPAA2_ETH_MFL);
> >
> > The cast was there in the original code so this is not a comment on this
> > particular patch (which seems fine) but there is no need to cast.
> >
> > Generally it's best to avoid unnecessary casts. As a human reader, I
> > find the cast confusing. It indicates that DPAA2_ETH_MFL somehow
> > requires special handling. Perhaps it's negative or we are trying to
> > truncate away the high bits. But neither of those things really make
> > sense.
> >
> > From a static analysis perspective if DPAA2_ETH_MFL doesn't fit nicely
> > then we would want to generate a warning. But the cast explicitly
> > disables the check.
>
> I really don't remember why the cast was there in the first place.
> It doesn't look like it's needed anymore, DPAA2_ETH_MFL has a
> positive value (around 10K) that fits just fine inside a u16.
>
> I see Greg already applied the patch, so I'll send a separate one to
> remove the cast.
Yeah. I wasn't saying redo the patch. That was there in the original
and it's not like it's a bug or anything.
regards,
dan carpenter