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

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

[Ioana Radulescu]

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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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

Signed-off-by: Ioana Radulescu <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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

Signed-off-by: Ioana Radulescu <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ioana Radulescu]

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 <ruxandra.radulescu@nxp.com>
---
 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

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Greg KH]

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 <ruxandra.radulescu@nxp.com>
> ---
>  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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Ruxandra Ioana Ciocoi Radulescu]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Friday, March 23, 2018 5:01 PM
> To: Ruxandra Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>
> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> 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 <ruxandra.radulescu@nxp.com>
> > ---
> >  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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

[Greg KH]

On Fri, Mar 23, 2018 at 03:03:29PM +0000, Ruxandra Ioana Ciocoi Radulescu wrote:
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Friday, March 23, 2018 5:01 PM
> > To: Ruxandra Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>
> > Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> > 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 <ruxandra.radulescu@nxp.com>
> > > ---
> > >  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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel