2018-03-23 13:45:56

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 0/9] staging: fsl-dpaa2/eth: Cleanup

A set of cleanup patches with limited functional impact.

Ioana Radulescu (9):
staging: fsl-dpaa2/eth: Use generic irq handler
staging: fsl-dpaa2/eth: Move print message
staging: fsl-dpaa2/eth: Remove unused field
staging: fsl-dpaa2/eth: Remove packed attribute
staging: fsl-dpaa2/eth: Add DPNI version check
staging: fsl-dpaa2/eth: Change link settings on the fly
staging: fsl-dpaa2/eth: Cleanup TX frame freeing code
staging: fsl-dpaa2/eth: Fix SGT allocation
staging: fsl-dpaa2/eth: Change max number of Tx queues

drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 47 ++++++++++++----------
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 11 ++---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c | 31 +++++++-------
3 files changed, 46 insertions(+), 43 deletions(-)

--
2.7.4



2018-03-23 13:46:17

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 7/9] staging: fsl-dpaa2/eth: Cleanup TX frame freeing code

Cleanup code in free_tx_fd() that deals with S/G frames:
- remove local variables that aren't really needed
- in the frame sw annotation area, store the actual SG table
buffer size, which is needed on free, rather then recompute
it based on number of S/G entries

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 17 +++++------------
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 2 +-
2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index f58c85a..9994922 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -406,7 +406,7 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,
swa->skb = skb;
swa->scl = scl;
swa->num_sg = num_sg;
- swa->num_dma_bufs = num_dma_bufs;
+ swa->sgt_size = sgt_buf_size;

/* Separately map the SGT buffer */
addr = dma_map_single(dev, sgt_buf, sgt_buf_size, DMA_BIDIRECTIONAL);
@@ -489,9 +489,6 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
dma_addr_t fd_addr;
struct sk_buff **skbh, *skb;
unsigned char *buffer_start;
- int unmap_size;
- struct scatterlist *scl;
- int num_sg, num_dma_bufs;
struct dpaa2_eth_swa *swa;
u8 fd_format = dpaa2_fd_get_format(fd);

@@ -510,18 +507,14 @@ static void free_tx_fd(const struct dpaa2_eth_priv *priv,
} else if (fd_format == dpaa2_fd_sg) {
swa = (struct dpaa2_eth_swa *)skbh;
skb = swa->skb;
- scl = swa->scl;
- num_sg = swa->num_sg;
- num_dma_bufs = swa->num_dma_bufs;

/* Unmap the scatterlist */
- dma_unmap_sg(dev, scl, num_sg, DMA_BIDIRECTIONAL);
- kfree(scl);
+ dma_unmap_sg(dev, swa->scl, swa->num_sg, DMA_BIDIRECTIONAL);
+ kfree(swa->scl);

/* Unmap the SGT buffer */
- unmap_size = priv->tx_data_offset +
- sizeof(struct dpaa2_sg_entry) * (1 + num_dma_bufs);
- dma_unmap_single(dev, fd_addr, unmap_size, DMA_BIDIRECTIONAL);
+ dma_unmap_single(dev, fd_addr, swa->sgt_size,
+ DMA_BIDIRECTIONAL);
} else {
netdev_dbg(priv->net_dev, "Invalid FD format\n");
return;
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index 5ac014f..fc9a255 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -108,7 +108,7 @@ struct dpaa2_eth_swa {
struct sk_buff *skb;
struct scatterlist *scl;
int num_sg;
- int num_dma_bufs;
+ int sgt_size;
};

/* Annotation valid bits in FD FRC */
--
2.7.4


2018-03-23 13:46:38

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 4/9] staging: fsl-dpaa2/eth: Remove packed attribute

Structure dpaa2_fas is naturally aligned, so no need to use
the __packed attribute explicitly.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index 63ea64b..c4816e6 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -142,7 +142,7 @@ struct dpaa2_fas {
u8 ppid;
__le16 ifpid;
__le32 status;
-} __packed;
+};

/* Frame annotation status word is located in the first 8 bytes
* of the buffer's hardware annoatation area
--
2.7.4


2018-03-23 13:46:44

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 8/9] staging: fsl-dpaa2/eth: Fix SGT allocation

We mistakenly allocate space for too many entries in the
scatter-gather table of multi buffer egress frames.

While it doesn't have a negative impact from a functional
point of view, it wastes resources so fix it.

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 9994922..14bd8fe 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -373,7 +373,7 @@ static int build_sg_fd(struct dpaa2_eth_priv *priv,

/* Prepare the HW SGT structure */
sgt_buf_size = priv->tx_data_offset +
- sizeof(struct dpaa2_sg_entry) * (1 + num_dma_bufs);
+ sizeof(struct dpaa2_sg_entry) * num_dma_bufs;
sgt_buf = netdev_alloc_frag(sgt_buf_size + DPAA2_ETH_TX_BUF_ALIGN);
if (unlikely(!sgt_buf)) {
err = -ENOMEM;
--
2.7.4


2018-03-23 13:47:22

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 9/9] staging: fsl-dpaa2/eth: Change max number of Tx queues

We use DPAA2_ETH_MAX_TX_QUEUES to dimension the array holding
information on Tx queues. At most, we can have one queue per cpu.

Until now we used the NR_CPUS macro to set the upper limit on number
of Tx queues. However, the platforms that the DPAA2 Ethernet driver
supports have at most 16 cores, whereas NR_CPUS is Kconfigurable and
can be much higher.

Avoid allocating memory we'll never use, by setting
DPAA2_ETH_MAX_TX_QUEUES to 16. Same for DPAA2_ETH_MAX_DPCONS.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index fc9a255..54cea2f 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -251,11 +251,11 @@ struct dpaa2_eth_ch_stats {

/* Maximum number of queues associated with a DPNI */
#define DPAA2_ETH_MAX_RX_QUEUES 16
-#define DPAA2_ETH_MAX_TX_QUEUES NR_CPUS
+#define DPAA2_ETH_MAX_TX_QUEUES 16
#define DPAA2_ETH_MAX_QUEUES (DPAA2_ETH_MAX_RX_QUEUES + \
DPAA2_ETH_MAX_TX_QUEUES)

-#define DPAA2_ETH_MAX_DPCONS NR_CPUS
+#define DPAA2_ETH_MAX_DPCONS 16

enum dpaa2_eth_fq_type {
DPAA2_RX_FQ = 0,
--
2.7.4


2018-03-23 13:47:51

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 5/9] staging: fsl-dpaa2/eth: Add DPNI version check

The DPAA2 Ethernet driver assumes the DPNI objects that it
uses to build network interfaces have a minimum supported
API version, but until now it did nothing to enforce this
requirement.

Add a check at probe time to make sure the DPNI object is
compatible with the set of MC commands we intend to use
on it.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 15 +++++++++++++++
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 10 ++++++++++
drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c | 11 ++---------
3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index 9fb88f2..f58c85a 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -1846,6 +1846,21 @@ static int setup_dpni(struct fsl_mc_device *ls_dev)
return err;
}

+ /* Check if we can work with this DPNI object */
+ err = dpni_get_api_version(priv->mc_io, 0, &priv->dpni_ver_major,
+ &priv->dpni_ver_minor);
+ if (err) {
+ dev_err(dev, "dpni_get_api_version() failed\n");
+ goto close;
+ }
+ if (dpaa2_eth_cmp_dpni_ver(priv, DPNI_VER_MAJOR, DPNI_VER_MINOR) < 0) {
+ dev_err(dev, "DPNI version %u.%u not supported, need >= %u.%u\n",
+ priv->dpni_ver_major, priv->dpni_ver_minor,
+ DPNI_VER_MAJOR, DPNI_VER_MINOR);
+ err = -ENOTSUPP;
+ goto close;
+ }
+
ls_dev->mc_io = priv->mc_io;
ls_dev->mc_handle = priv->mc_token;

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index c4816e6..5ac014f 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -311,6 +311,8 @@ struct dpaa2_eth_priv {
struct dpaa2_eth_channel *channel[DPAA2_ETH_MAX_DPCONS];

struct dpni_attr dpni_attrs;
+ u16 dpni_ver_major;
+ u16 dpni_ver_minor;
u16 tx_data_offset;

struct fsl_mc_device *dpbp_dev;
@@ -354,6 +356,14 @@ struct dpaa2_eth_priv {
extern const struct ethtool_ops dpaa2_ethtool_ops;
extern const char dpaa2_eth_drv_version[];

+static inline int dpaa2_eth_cmp_dpni_ver(struct dpaa2_eth_priv *priv,
+ u16 ver_major, u16 ver_minor)
+{
+ if (priv->dpni_ver_major == ver_major)
+ return priv->dpni_ver_minor - ver_minor;
+ return priv->dpni_ver_major - ver_major;
+}
+
/* Hardware only sees DPAA2_ETH_RX_BUF_SIZE, but the skb built around
* the buffer also needs space for its shared info struct, and we need
* to allocate enough to accommodate hardware alignment restrictions
diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
index 070a3f2..dfbfa94 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
@@ -78,20 +78,13 @@ static void dpaa2_eth_get_drvinfo(struct net_device *net_dev,
struct ethtool_drvinfo *drvinfo)
{
struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
- u16 fw_major, fw_minor;
- int err;

strlcpy(drvinfo->driver, KBUILD_MODNAME, sizeof(drvinfo->driver));
strlcpy(drvinfo->version, dpaa2_eth_drv_version,
sizeof(drvinfo->version));

- err = dpni_get_api_version(priv->mc_io, 0, &fw_major, &fw_minor);
- if (!err)
- snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
- "%u.%u", fw_major, fw_minor);
- else
- strlcpy(drvinfo->fw_version, "N/A",
- sizeof(drvinfo->fw_version));
+ snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
+ "%u.%u", priv->dpni_ver_major, priv->dpni_ver_minor);

strlcpy(drvinfo->bus_info, dev_name(net_dev->dev.parent->parent),
sizeof(drvinfo->bus_info));
--
2.7.4


2018-03-23 13:47:55

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message

Let the driver remove() function print an informative message
after it finishes removing the network interface, not at an
arbitrary point during cleanup.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index beb5959..9fb88f2 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -2550,7 +2550,6 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
priv = netdev_priv(net_dev);

unregister_netdev(net_dev);
- dev_info(net_dev->dev.parent, "Removed interface %s\n", net_dev->name);

if (priv->do_link_poll)
kthread_stop(priv->poll_thread);
@@ -2571,6 +2570,8 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
dev_set_drvdata(dev, NULL);
free_netdev(net_dev);

+ dev_info(net_dev->dev.parent, "Removed interface %s\n", net_dev->name);
+
return 0;
}

--
2.7.4


2018-03-23 13:48:17

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 3/9] staging: fsl-dpaa2/eth: Remove unused field

Remove dpio_id field in struct dpaa2_eth_channel, which wasn't
used anywhere in the code.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
index b8990cf..63ea64b 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
@@ -285,7 +285,6 @@ struct dpaa2_eth_channel {
struct fsl_mc_device *dpcon;
int dpcon_id;
int ch_id;
- int dpio_id;
struct napi_struct napi;
struct dpaa2_io *dpio;
struct dpaa2_io_store *store;
--
2.7.4


2018-03-23 13:48:27

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 6/9] staging: fsl-dpaa2/eth: Change link settings on the fly

Newer MC versions allow us to change link settings while the
interface is up. Only check interface status if we are using
an old version.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
index dfbfa94..bfc8b64 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-ethtool.c
@@ -119,6 +119,8 @@ dpaa2_eth_get_link_ksettings(struct net_device *net_dev,
return err;
}

+#define DPNI_DYNAMIC_LINK_SET_VER_MAJOR 7
+#define DPNI_DYNAMIC_LINK_SET_VER_MINOR 1
static int
dpaa2_eth_set_link_ksettings(struct net_device *net_dev,
const struct ethtool_link_ksettings *link_settings)
@@ -127,15 +129,16 @@ dpaa2_eth_set_link_ksettings(struct net_device *net_dev,
struct dpaa2_eth_priv *priv = netdev_priv(net_dev);
int err = 0;

- netdev_dbg(net_dev, "Setting link parameters...");
-
- /* Due to a temporary MC limitation, the DPNI must be down
+ /* If using an older MC version, the DPNI must be down
* in order to be able to change link settings. Taking steps to let
* the user know that.
*/
- if (netif_running(net_dev)) {
- netdev_info(net_dev, "Sorry, interface must be brought down first.\n");
- return -EACCES;
+ if (dpaa2_eth_cmp_dpni_ver(priv, DPNI_DYNAMIC_LINK_SET_VER_MAJOR,
+ DPNI_DYNAMIC_LINK_SET_VER_MINOR) < 0) {
+ if (netif_running(net_dev)) {
+ netdev_info(net_dev, "Interface must be brought down first.\n");
+ return -EACCES;
+ }
}

cfg.rate = link_settings->base.speed;
--
2.7.4


2018-03-23 13:49:48

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: [PATCH 1/9] staging: fsl-dpaa2/eth: Use generic irq handler

For the link state interrupt, we used a dummy non-threaded
irq handler, which had the same implementation as the generic
irq_default_primary_handler() function.

Give up on using our own irq handler and let the kernel use
the generic one instead.

Signed-off-by: Ioana Radulescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
index ac1a750..beb5959 100644
--- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
+++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
@@ -2317,11 +2317,6 @@ static int poll_link_state(void *arg)
return 0;
}

-static irqreturn_t dpni_irq0_handler(int irq_num, void *arg)
-{
- return IRQ_WAKE_THREAD;
-}
-
static irqreturn_t dpni_irq0_handler_thread(int irq_num, void *arg)
{
u32 status = ~0;
@@ -2356,8 +2351,7 @@ static int setup_irqs(struct fsl_mc_device *ls_dev)

irq = ls_dev->irqs[0];
err = devm_request_threaded_irq(&ls_dev->dev, irq->msi_desc->irq,
- dpni_irq0_handler,
- dpni_irq0_handler_thread,
+ NULL, dpni_irq0_handler_thread,
IRQF_NO_SUSPEND | IRQF_ONESHOT,
dev_name(&ls_dev->dev), &ls_dev->dev);
if (err < 0) {
--
2.7.4


2018-03-23 15:04:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message

On Fri, Mar 23, 2018 at 08:44:06AM -0500, Ioana Radulescu wrote:
> Let the driver remove() function print an informative message
> after it finishes removing the network interface, not at an
> arbitrary point during cleanup.
>
> Signed-off-by: Ioana Radulescu <[email protected]>
> ---
> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> index beb5959..9fb88f2 100644
> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> @@ -2550,7 +2550,6 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
> priv = netdev_priv(net_dev);
>
> unregister_netdev(net_dev);
> - dev_info(net_dev->dev.parent, "Removed interface %s\n", net_dev->name);
>
> if (priv->do_link_poll)
> kthread_stop(priv->poll_thread);
> @@ -2571,6 +2570,8 @@ static int dpaa2_eth_remove(struct fsl_mc_device *ls_dev)
> dev_set_drvdata(dev, NULL);
> free_netdev(net_dev);
>
> + dev_info(net_dev->dev.parent, "Removed interface %s\n", net_dev->name);

Why is this even needed? I'll take it but spamming kernel logs for no
reason is not a good idea :)

thanks,

greg k-h

2018-03-23 15:06:17

by Ioana Ciocoi Radulescu

[permalink] [raw]
Subject: RE: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message

> -----Original Message-----
> From: Greg KH [mailto:[email protected]]
> Sent: Friday, March 23, 2018 5:01 PM
> To: Ruxandra Ioana Ciocoi Radulescu <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message
>
> On Fri, Mar 23, 2018 at 08:44:06AM -0500, Ioana Radulescu wrote:
> > Let the driver remove() function print an informative message
> > after it finishes removing the network interface, not at an
> > arbitrary point during cleanup.
> >
> > Signed-off-by: Ioana Radulescu <[email protected]>
> > ---
> > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > index beb5959..9fb88f2 100644
> > --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > @@ -2550,7 +2550,6 @@ static int dpaa2_eth_remove(struct
> fsl_mc_device *ls_dev)
> > priv = netdev_priv(net_dev);
> >
> > unregister_netdev(net_dev);
> > - dev_info(net_dev->dev.parent, "Removed interface %s\n",
> net_dev->name);
> >
> > if (priv->do_link_poll)
> > kthread_stop(priv->poll_thread);
> > @@ -2571,6 +2570,8 @@ static int dpaa2_eth_remove(struct
> fsl_mc_device *ls_dev)
> > dev_set_drvdata(dev, NULL);
> > free_netdev(net_dev);
> >
> > + dev_info(net_dev->dev.parent, "Removed interface %s\n",
> net_dev->name);
>
> Why is this even needed? I'll take it but spamming kernel logs for no
> reason is not a good idea :)

Fair point, should I make it dev_dbg instead?

Thanks,
Ioana

2018-03-23 15:08:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message

On Fri, Mar 23, 2018 at 03:03:29PM +0000, Ruxandra Ioana Ciocoi Radulescu wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:[email protected]]
> > Sent: Friday, March 23, 2018 5:01 PM
> > To: Ruxandra Ioana Ciocoi Radulescu <[email protected]>
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH 2/9] staging: fsl-dpaa2/eth: Move print message
> >
> > On Fri, Mar 23, 2018 at 08:44:06AM -0500, Ioana Radulescu wrote:
> > > Let the driver remove() function print an informative message
> > > after it finishes removing the network interface, not at an
> > > arbitrary point during cleanup.
> > >
> > > Signed-off-by: Ioana Radulescu <[email protected]>
> > > ---
> > > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > > index beb5959..9fb88f2 100644
> > > --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > > +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> > > @@ -2550,7 +2550,6 @@ static int dpaa2_eth_remove(struct
> > fsl_mc_device *ls_dev)
> > > priv = netdev_priv(net_dev);
> > >
> > > unregister_netdev(net_dev);
> > > - dev_info(net_dev->dev.parent, "Removed interface %s\n",
> > net_dev->name);
> > >
> > > if (priv->do_link_poll)
> > > kthread_stop(priv->poll_thread);
> > > @@ -2571,6 +2570,8 @@ static int dpaa2_eth_remove(struct
> > fsl_mc_device *ls_dev)
> > > dev_set_drvdata(dev, NULL);
> > > free_netdev(net_dev);
> > >
> > > + dev_info(net_dev->dev.parent, "Removed interface %s\n",
> > net_dev->name);
> >
> > Why is this even needed? I'll take it but spamming kernel logs for no
> > reason is not a good idea :)
>
> Fair point, should I make it dev_dbg instead?

Yeah, that's a lot nicer, thanks.

greg k-h