All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5]  Fixes coding style in xilinx_emaclite.c
@ 2018-06-18 11:08 ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

This patchset fixes checkpatch and kernel-doc warnings in
xilinx emaclite driver. No functional change.

Radhey Shyam Pandey (5):
  net: emaclite: Use __func__ instead of hardcoded name
  net: emaclite: Balance braces in else statement
  net: emaclite: update kernel-doc comments
  net: emaclite: Fix block comments style
  net: emaclite: Remove unnecessary spaces

 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 101 +++++++++++++++-----------
 1 file changed, 58 insertions(+), 43 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 0/5]  Fixes coding style in xilinx_emaclite.c
@ 2018-06-18 11:08 ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset fixes checkpatch and kernel-doc warnings in
xilinx emaclite driver. No functional change.

Radhey Shyam Pandey (5):
  net: emaclite: Use __func__ instead of hardcoded name
  net: emaclite: Balance braces in else statement
  net: emaclite: update kernel-doc comments
  net: emaclite: Fix block comments style
  net: emaclite: Remove unnecessary spaces

 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 101 +++++++++++++++-----------
 1 file changed, 58 insertions(+), 43 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
  2018-06-18 11:08 ` Radhey Shyam Pandey
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

Switch hardcoded function name with a reference to __func__ making
the code more maintainable. Address below checkpatch warning:

WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
this function's name, in a string
+               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",

WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
this function's name, in a string
+               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 2a0c06e..0544134 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -757,7 +757,7 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
 	rc = xemaclite_readl(lp->base_addr + XEL_MDIORD_OFFSET);
 
 	dev_dbg(&lp->ndev->dev,
-		"xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
+		"%s(phy_id=%i, reg=%x) == %x\n", __func__,
 		phy_id, reg, rc);
 
 	return rc;
@@ -780,7 +780,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 	u32 ctrl_reg;
 
 	dev_dbg(&lp->ndev->dev,
-		"xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
+		"%s(phy_id=%i, reg=%x, val=%x)\n", __func__,
 		phy_id, reg, val);
 
 	if (xemaclite_mdio_wait(lp))
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

Switch hardcoded function name with a reference to __func__ making
the code more maintainable. Address below checkpatch warning:

WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
this function's name, in a string
+               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",

WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
this function's name, in a string
+               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 2a0c06e..0544134 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -757,7 +757,7 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
 	rc = xemaclite_readl(lp->base_addr + XEL_MDIORD_OFFSET);
 
 	dev_dbg(&lp->ndev->dev,
-		"xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
+		"%s(phy_id=%i, reg=%x) == %x\n", __func__,
 		phy_id, reg, rc);
 
 	return rc;
@@ -780,7 +780,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 	u32 ctrl_reg;
 
 	dev_dbg(&lp->ndev->dev,
-		"xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
+		"%s(phy_id=%i, reg=%x, val=%x)\n", __func__,
 		phy_id, reg, val);
 
 	if (xemaclite_mdio_wait(lp))
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
  2018-06-18 11:08 ` Radhey Shyam Pandey
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

Remove else as it is not required with if doing a return.
Fixes below checkpatch warning.

WARNING: else is not generally useful after a break or return

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0544134..8d84f58 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
 					(u8 *) lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
-		else {
-			dev->stats.tx_bytes += lp->deferred_skb->len;
-			dev_kfree_skb_irq(lp->deferred_skb);
-			lp->deferred_skb = NULL;
-			netif_trans_update(dev); /* prevent tx timeout */
-			netif_wake_queue(dev);
-		}
+		dev->stats.tx_bytes += lp->deferred_skb->len;
+		dev_kfree_skb_irq(lp->deferred_skb);
+		lp->deferred_skb = NULL;
+		netif_trans_update(dev); /* prevent tx timeout */
+		netif_wake_queue(dev);
 	}
 }
 
@@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
 {
 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
 
-	if (p) {
+	if (p)
 		return (bool)*p;
-	} else {
-		dev_warn(&ofdev->dev, "Parameter %s not found,"
+
+	dev_warn(&ofdev->dev, "Parameter %s not found,"
 			"defaulting to false\n", s);
-		return false;
-	}
+
+	return false;
 }
 
 static const struct net_device_ops xemaclite_netdev_ops;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

Remove else as it is not required with if doing a return.
Fixes below checkpatch warning.

WARNING: else is not generally useful after a break or return

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0544134..8d84f58 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
 					(u8 *) lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
-		else {
-			dev->stats.tx_bytes += lp->deferred_skb->len;
-			dev_kfree_skb_irq(lp->deferred_skb);
-			lp->deferred_skb = NULL;
-			netif_trans_update(dev); /* prevent tx timeout */
-			netif_wake_queue(dev);
-		}
+		dev->stats.tx_bytes += lp->deferred_skb->len;
+		dev_kfree_skb_irq(lp->deferred_skb);
+		lp->deferred_skb = NULL;
+		netif_trans_update(dev); /* prevent tx timeout */
+		netif_wake_queue(dev);
 	}
 }
 
@@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
 {
 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
 
-	if (p) {
+	if (p)
 		return (bool)*p;
-	} else {
-		dev_warn(&ofdev->dev, "Parameter %s not found,"
+
+	dev_warn(&ofdev->dev, "Parameter %s not found,"
 			"defaulting to false\n", s);
-		return false;
-	}
+
+	return false;
 }
 
 static const struct net_device_ops xemaclite_netdev_ops;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/5] net: emaclite: update kernel-doc comments
  2018-06-18 11:08 ` Radhey Shyam Pandey
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

This patch fixes below kernel-doc warnings:

Function parameter or member 'maxlen' not described in 'xemaclite_recv_data'
Function parameter or member 'address'not described in 'xemaclite_set_mac_address'
Excess function parameter 'addr' description in 'xemaclite_set_mac_address'
No description found for return value of 'xemaclite_interrupt'
No description found for return value of 'xemaclite_mdio_write'
Function parameter or member 'dev' not described in 'xemaclite_mdio_setup'
Excess function parameter 'ofdev' description in 'xemaclite_mdio_setup'
No description found for return value of 'xemaclite_open'
No description found for return value of 'xemaclite_close'
Excess function parameter 'match' description in 'xemaclite_of_probe'

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 8d84f58..0b41cc5 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -369,6 +369,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
  * xemaclite_recv_data - Receive a frame
  * @drvdata:	Pointer to the Emaclite device private data
  * @data:	Address where the data is to be received
+ * @maxlen:    Maximum supported ethernet packet length
  *
  * This function is intended to be called from the interrupt context or
  * with a wrapper which waits for the receive frame to be available.
@@ -488,7 +489,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
 /**
  * xemaclite_set_mac_address - Set the MAC address for this device
  * @dev:	Pointer to the network device instance
- * @addr:	Void pointer to the sockaddr structure
+ * @address:	Void pointer to the sockaddr structure
  *
  * This function copies the HW address from the sockaddr strucutre to the
  * net_device structure and updates the address in HW.
@@ -637,6 +638,8 @@ static void xemaclite_rx_handler(struct net_device *dev)
  * @dev_id:	Void pointer to the network device instance used as callback
  *		reference
  *
+ * Return:	IRQ_HANDLED
+ *
  * This function handles the Tx and Rx interrupts of the EmacLite device.
  */
 static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
@@ -770,6 +773,8 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
  *
  * This function waits till the device is ready to accept a new MDIO
  * request and then writes the val to the MDIO Write Data register.
+ *
+ * Return:      0 upon success or a negative error upon failure
  */
 static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 				u16 val)
@@ -803,7 +808,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 /**
  * xemaclite_mdio_setup - Register mii_bus for the Emaclite device
  * @lp:		Pointer to the Emaclite device private data
- * @ofdev:	Pointer to OF device structure
+ * @dev:	Pointer to OF device structure
  *
  * This function enables MDIO bus in the Emaclite device and registers a
  * mii_bus.
@@ -903,6 +908,9 @@ static void xemaclite_adjust_link(struct net_device *ndev)
  * This function sets the MAC address, requests an IRQ and enables interrupts
  * for the Emaclite device and starts the Tx queue.
  * It also connects to the phy device, if MDIO is included in Emaclite device.
+ *
+ * Return:	0 on success. -ENODEV, if PHY cannot be connected.
+ *		Non-zero error value on failure.
  */
 static int xemaclite_open(struct net_device *dev)
 {
@@ -973,6 +981,8 @@ static int xemaclite_open(struct net_device *dev)
  * This function stops the Tx queue, disables interrupts and frees the IRQ for
  * the Emaclite device.
  * It also disconnects the phy device associated with the Emaclite device.
+ *
+ * Return:	0, always.
  */
 static int xemaclite_close(struct net_device *dev)
 {
@@ -1064,7 +1074,6 @@ static const struct net_device_ops xemaclite_netdev_ops;
 /**
  * xemaclite_of_probe - Probe method for the Emaclite device.
  * @ofdev:	Pointer to OF device structure
- * @match:	Pointer to the structure used for matching a device
  *
  * This function probes for the Emaclite device in the device tree.
  * It initializes the driver data structure and the hardware, sets the MAC
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/5] net: emaclite: update kernel-doc comments
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

This patch fixes below kernel-doc warnings:

Function parameter or member 'maxlen' not described in 'xemaclite_recv_data'
Function parameter or member 'address'not described in 'xemaclite_set_mac_address'
Excess function parameter 'addr' description in 'xemaclite_set_mac_address'
No description found for return value of 'xemaclite_interrupt'
No description found for return value of 'xemaclite_mdio_write'
Function parameter or member 'dev' not described in 'xemaclite_mdio_setup'
Excess function parameter 'ofdev' description in 'xemaclite_mdio_setup'
No description found for return value of 'xemaclite_open'
No description found for return value of 'xemaclite_close'
Excess function parameter 'match' description in 'xemaclite_of_probe'

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 8d84f58..0b41cc5 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -369,6 +369,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
  * xemaclite_recv_data - Receive a frame
  * @drvdata:	Pointer to the Emaclite device private data
  * @data:	Address where the data is to be received
+ * @maxlen:    Maximum supported ethernet packet length
  *
  * This function is intended to be called from the interrupt context or
  * with a wrapper which waits for the receive frame to be available.
@@ -488,7 +489,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
 /**
  * xemaclite_set_mac_address - Set the MAC address for this device
  * @dev:	Pointer to the network device instance
- * @addr:	Void pointer to the sockaddr structure
+ * @address:	Void pointer to the sockaddr structure
  *
  * This function copies the HW address from the sockaddr strucutre to the
  * net_device structure and updates the address in HW.
@@ -637,6 +638,8 @@ static void xemaclite_rx_handler(struct net_device *dev)
  * @dev_id:	Void pointer to the network device instance used as callback
  *		reference
  *
+ * Return:	IRQ_HANDLED
+ *
  * This function handles the Tx and Rx interrupts of the EmacLite device.
  */
 static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
@@ -770,6 +773,8 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
  *
  * This function waits till the device is ready to accept a new MDIO
  * request and then writes the val to the MDIO Write Data register.
+ *
+ * Return:      0 upon success or a negative error upon failure
  */
 static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 				u16 val)
@@ -803,7 +808,7 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
 /**
  * xemaclite_mdio_setup - Register mii_bus for the Emaclite device
  * @lp:		Pointer to the Emaclite device private data
- * @ofdev:	Pointer to OF device structure
+ * @dev:	Pointer to OF device structure
  *
  * This function enables MDIO bus in the Emaclite device and registers a
  * mii_bus.
@@ -903,6 +908,9 @@ static void xemaclite_adjust_link(struct net_device *ndev)
  * This function sets the MAC address, requests an IRQ and enables interrupts
  * for the Emaclite device and starts the Tx queue.
  * It also connects to the phy device, if MDIO is included in Emaclite device.
+ *
+ * Return:	0 on success. -ENODEV, if PHY cannot be connected.
+ *		Non-zero error value on failure.
  */
 static int xemaclite_open(struct net_device *dev)
 {
@@ -973,6 +981,8 @@ static int xemaclite_open(struct net_device *dev)
  * This function stops the Tx queue, disables interrupts and frees the IRQ for
  * the Emaclite device.
  * It also disconnects the phy device associated with the Emaclite device.
+ *
+ * Return:	0, always.
  */
 static int xemaclite_close(struct net_device *dev)
 {
@@ -1064,7 +1074,6 @@ static const struct net_device_ops xemaclite_netdev_ops;
 /**
  * xemaclite_of_probe - Probe method for the Emaclite device.
  * @ofdev:	Pointer to OF device structure
- * @match:	Pointer to the structure used for matching a device
  *
  * This function probes for the Emaclite device in the device tree.
  * It initializes the driver data structure and the hardware, sets the MAC
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/5] net: emaclite: Fix block comments style
  2018-06-18 11:08 ` Radhey Shyam Pandey
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

This patch fixes below checkpatch warnings-

WARNING: Block comments use a trailing */ on a separate line
WARNING: Block comments use * on subsequent lines
WARNING: networking block comments don't use an empty /* line,
use /* Comment

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 34 +++++++++++++++++----------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0b41cc5..e8bb7b3 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -70,7 +70,8 @@
 #define XEL_TSR_XMIT_IE_MASK	 0x00000008	/* Tx interrupt enable bit */
 #define XEL_TSR_XMIT_ACTIVE_MASK 0x80000000	/* Buffer is active, SW bit
 						 * only. This is not documented
-						 * in the HW spec */
+						 * in the HW spec
+						 */
 
 /* Define for programming the MAC address into the EmacLite */
 #define XEL_TSR_PROG_MAC_ADDR	(XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_PROGRAM_MASK)
@@ -336,7 +337,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 			drvdata->next_tx_buf_to_use ^= XEL_BUFFER_OFFSET;
 	} else if (drvdata->tx_ping_pong != 0) {
 		/* If the expected buffer is full, try the other buffer,
-		 * if it is configured in HW */
+		 * if it is configured in HW
+		 */
 
 		addr = (void __iomem __force *)((u32 __force)addr ^
 						 XEL_BUFFER_OFFSET);
@@ -357,7 +359,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 	/* Update the Tx Status Register to indicate that there is a
 	 * frame to send. Set the XEL_TSR_XMIT_ACTIVE_MASK flag which
 	 * is used by the interrupt handler to check whether a frame
-	 * has been transmitted */
+	 * has been transmitted
+	 */
 	reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
 	reg_data |= (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_XMIT_ACTIVE_MASK);
 	xemaclite_writel(reg_data, addr + XEL_TSR_OFFSET);
@@ -395,7 +398,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 		/* The instance is out of sync, try other buffer if other
 		 * buffer is configured, return 0 otherwise. If the instance is
 		 * out of sync, do not update the 'next_rx_buf_to_use' since it
-		 * will correct on subsequent calls */
+		 * will correct on subsequent calls
+		 */
 		if (drvdata->rx_ping_pong != 0)
 			addr = (void __iomem __force *)((u32 __force)addr ^
 							 XEL_BUFFER_OFFSET);
@@ -409,13 +413,15 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 			return 0;	/* No data was available */
 	}
 
-	/* Get the protocol type of the ethernet frame that arrived */
+	/* Get the protocol type of the ethernet frame that arrived
+	 */
 	proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
 			XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
 			XEL_RPLR_LENGTH_MASK);
 
 	/* Check if received ethernet frame is a raw ethernet frame
-	 * or an IP packet or an ARP packet */
+	 * or an IP packet or an ARP packet
+	 */
 	if (proto_type > ETH_DATA_LEN) {
 
 		if (proto_type == ETH_P_IP) {
@@ -431,7 +437,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 			length = XEL_ARP_PACKET_SIZE + ETH_HLEN + ETH_FCS_LEN;
 		else
 			/* Field contains type other than IP or ARP, use max
-			 * frame size and let user parse it */
+			 * frame size and let user parse it
+			 */
 			length = ETH_FRAME_LEN + ETH_FCS_LEN;
 	} else
 		/* Use the length in the frame, plus the header and trailer */
@@ -601,11 +608,11 @@ static void xemaclite_rx_handler(struct net_device *dev)
 		return;
 	}
 
-	/*
-	 * A new skb should have the data halfword aligned, but this code is
+	/* A new skb should have the data halfword aligned, but this code is
 	 * here just in case that isn't true. Calculate how many
 	 * bytes we should reserve to get the data to start on a word
-	 * boundary */
+	 * boundary
+	 */
 	align = BUFFER_ALIGN(skb->data);
 	if (align)
 		skb_reserve(skb, align);
@@ -707,8 +714,8 @@ static int xemaclite_mdio_wait(struct net_local *lp)
 	unsigned long end = jiffies + 2;
 
 	/* wait for the MDIO interface to not be busy or timeout
-	   after some time.
-	*/
+	 * after some time.
+	 */
 	while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
 			XEL_MDIOCTRL_MDIOSTS_MASK) {
 		if (time_before_eq(end, jiffies)) {
@@ -1028,7 +1035,8 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
 	if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
 		/* If the Emaclite Tx buffer is busy, stop the Tx queue and
 		 * defer the skb for transmission during the ISR, after the
-		 * current transmission is complete */
+		 * current transmission is complete
+		 */
 		netif_stop_queue(dev);
 		lp->deferred_skb = new_skb;
 		/* Take the time stamp now, since we can't do this in an ISR. */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/5] net: emaclite: Fix block comments style
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

This patch fixes below checkpatch warnings-

WARNING: Block comments use a trailing */ on a separate line
WARNING: Block comments use * on subsequent lines
WARNING: networking block comments don't use an empty /* line,
use /* Comment

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 34 +++++++++++++++++----------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0b41cc5..e8bb7b3 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -70,7 +70,8 @@
 #define XEL_TSR_XMIT_IE_MASK	 0x00000008	/* Tx interrupt enable bit */
 #define XEL_TSR_XMIT_ACTIVE_MASK 0x80000000	/* Buffer is active, SW bit
 						 * only. This is not documented
-						 * in the HW spec */
+						 * in the HW spec
+						 */
 
 /* Define for programming the MAC address into the EmacLite */
 #define XEL_TSR_PROG_MAC_ADDR	(XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_PROGRAM_MASK)
@@ -336,7 +337,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 			drvdata->next_tx_buf_to_use ^= XEL_BUFFER_OFFSET;
 	} else if (drvdata->tx_ping_pong != 0) {
 		/* If the expected buffer is full, try the other buffer,
-		 * if it is configured in HW */
+		 * if it is configured in HW
+		 */
 
 		addr = (void __iomem __force *)((u32 __force)addr ^
 						 XEL_BUFFER_OFFSET);
@@ -357,7 +359,8 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 	/* Update the Tx Status Register to indicate that there is a
 	 * frame to send. Set the XEL_TSR_XMIT_ACTIVE_MASK flag which
 	 * is used by the interrupt handler to check whether a frame
-	 * has been transmitted */
+	 * has been transmitted
+	 */
 	reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
 	reg_data |= (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_XMIT_ACTIVE_MASK);
 	xemaclite_writel(reg_data, addr + XEL_TSR_OFFSET);
@@ -395,7 +398,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 		/* The instance is out of sync, try other buffer if other
 		 * buffer is configured, return 0 otherwise. If the instance is
 		 * out of sync, do not update the 'next_rx_buf_to_use' since it
-		 * will correct on subsequent calls */
+		 * will correct on subsequent calls
+		 */
 		if (drvdata->rx_ping_pong != 0)
 			addr = (void __iomem __force *)((u32 __force)addr ^
 							 XEL_BUFFER_OFFSET);
@@ -409,13 +413,15 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 			return 0;	/* No data was available */
 	}
 
-	/* Get the protocol type of the ethernet frame that arrived */
+	/* Get the protocol type of the ethernet frame that arrived
+	 */
 	proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
 			XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
 			XEL_RPLR_LENGTH_MASK);
 
 	/* Check if received ethernet frame is a raw ethernet frame
-	 * or an IP packet or an ARP packet */
+	 * or an IP packet or an ARP packet
+	 */
 	if (proto_type > ETH_DATA_LEN) {
 
 		if (proto_type == ETH_P_IP) {
@@ -431,7 +437,8 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 			length = XEL_ARP_PACKET_SIZE + ETH_HLEN + ETH_FCS_LEN;
 		else
 			/* Field contains type other than IP or ARP, use max
-			 * frame size and let user parse it */
+			 * frame size and let user parse it
+			 */
 			length = ETH_FRAME_LEN + ETH_FCS_LEN;
 	} else
 		/* Use the length in the frame, plus the header and trailer */
@@ -601,11 +608,11 @@ static void xemaclite_rx_handler(struct net_device *dev)
 		return;
 	}
 
-	/*
-	 * A new skb should have the data halfword aligned, but this code is
+	/* A new skb should have the data halfword aligned, but this code is
 	 * here just in case that isn't true. Calculate how many
 	 * bytes we should reserve to get the data to start on a word
-	 * boundary */
+	 * boundary
+	 */
 	align = BUFFER_ALIGN(skb->data);
 	if (align)
 		skb_reserve(skb, align);
@@ -707,8 +714,8 @@ static int xemaclite_mdio_wait(struct net_local *lp)
 	unsigned long end = jiffies + 2;
 
 	/* wait for the MDIO interface to not be busy or timeout
-	   after some time.
-	*/
+	 * after some time.
+	 */
 	while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
 			XEL_MDIOCTRL_MDIOSTS_MASK) {
 		if (time_before_eq(end, jiffies)) {
@@ -1028,7 +1035,8 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
 	if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
 		/* If the Emaclite Tx buffer is busy, stop the Tx queue and
 		 * defer the skb for transmission during the ISR, after the
-		 * current transmission is complete */
+		 * current transmission is complete
+		 */
 		netif_stop_queue(dev);
 		lp->deferred_skb = new_skb;
 		/* Take the time stamp now, since we can't do this in an ISR. */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/5] net: emaclite: Remove unnecessary spaces
  2018-06-18 11:08 ` Radhey Shyam Pandey
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

This patch fixes below checkpatch checks-

CHECK: spaces preferred around that '*' (ctx:VxV)
CHECK: No space is necessary after a cast

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index e8bb7b3..f62e4b6 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -95,11 +95,11 @@
 
 
 
-#define TX_TIMEOUT		(60*HZ)		/* Tx timeout is 60 seconds. */
+#define TX_TIMEOUT		(60 * HZ)	/* Tx timeout is 60 seconds. */
 #define ALIGNMENT		4
 
 /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */
-#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT)
+#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT)
 
 #ifdef __BIG_ENDIAN
 #define xemaclite_readl		ioread32be
@@ -239,8 +239,8 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
 
 		/* Set up to output the remaining data */
 		align_buffer = 0;
-		to_u8_ptr = (u8 *) &align_buffer;
-		from_u8_ptr = (u8 *) from_u16_ptr;
+		to_u8_ptr = (u8 *)&align_buffer;
+		from_u8_ptr = (u8 *)from_u16_ptr;
 
 		/* Output the remaining data */
 		for (; length > 0; length--)
@@ -273,7 +273,7 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
 	u32 align_buffer;
 
 	from_u32_ptr = src_ptr;
-	to_u16_ptr = (u16 *) dest_ptr;
+	to_u16_ptr = (u16 *)dest_ptr;
 
 	for (; length > 3; length -= 4) {
 		/* Copy each word into the temporary buffer */
@@ -289,9 +289,9 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
 		u8 *to_u8_ptr, *from_u8_ptr;
 
 		/* Set up to read the remaining data */
-		to_u8_ptr = (u8 *) to_u16_ptr;
+		to_u8_ptr = (u8 *)to_u16_ptr;
 		align_buffer = *from_u32_ptr++;
-		from_u8_ptr = (u8 *) &align_buffer;
+		from_u8_ptr = (u8 *)&align_buffer;
 
 		/* Read the remaining data */
 		for (; length > 0; length--)
@@ -351,7 +351,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 		return -1; /* Buffer was full, return failure */
 
 	/* Write the frame to the buffer */
-	xemaclite_aligned_write(data, (u32 __force *) addr, byte_count);
+	xemaclite_aligned_write(data, (u32 __force *)addr, byte_count);
 
 	xemaclite_writel((byte_count & XEL_TPLR_LENGTH_MASK),
 			 addr + XEL_TPLR_OFFSET);
@@ -448,7 +448,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 		length = maxlen;
 
 	/* Read from the EmacLite device */
-	xemaclite_aligned_read((u32 __force *) (addr + XEL_RXBUFF_OFFSET),
+	xemaclite_aligned_read((u32 __force *)(addr + XEL_RXBUFF_OFFSET),
 				data, length);
 
 	/* Acknowledge the frame */
@@ -479,7 +479,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
 	/* Determine the expected Tx buffer address */
 	addr = drvdata->base_addr + drvdata->next_tx_buf_to_use;
 
-	xemaclite_aligned_write(address_ptr, (u32 __force *) addr, ETH_ALEN);
+	xemaclite_aligned_write(address_ptr, (u32 __force *)addr, ETH_ALEN);
 
 	xemaclite_writel(ETH_ALEN, addr + XEL_TPLR_OFFSET);
 
@@ -574,7 +574,7 @@ static void xemaclite_tx_handler(struct net_device *dev)
 	dev->stats.tx_packets++;
 	if (lp->deferred_skb) {
 		if (xemaclite_send_data(lp,
-					(u8 *) lp->deferred_skb->data,
+					(u8 *)lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
 		dev->stats.tx_bytes += lp->deferred_skb->len;
@@ -619,7 +619,7 @@ static void xemaclite_rx_handler(struct net_device *dev)
 
 	skb_reserve(skb, 2);
 
-	len = xemaclite_recv_data(lp, (u8 *) skb->data, len);
+	len = xemaclite_recv_data(lp, (u8 *)skb->data, len);
 
 	if (!len) {
 		dev->stats.rx_errors++;
@@ -1032,7 +1032,7 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
 	new_skb = orig_skb;
 
 	spin_lock_irqsave(&lp->reset_lock, flags);
-	if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
+	if (xemaclite_send_data(lp, (u8 *)new_skb->data, len) != 0) {
 		/* If the Emaclite Tx buffer is busy, stop the Tx queue and
 		 * defer the skb for transmission during the ISR, after the
 		 * current transmission is complete
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/5] net: emaclite: Remove unnecessary spaces
@ 2018-06-18 11:08   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

This patch fixes below checkpatch checks-

CHECK: spaces preferred around that '*' (ctx:VxV)
CHECK: No space is necessary after a cast

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index e8bb7b3..f62e4b6 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -95,11 +95,11 @@
 
 
 
-#define TX_TIMEOUT		(60*HZ)		/* Tx timeout is 60 seconds. */
+#define TX_TIMEOUT		(60 * HZ)	/* Tx timeout is 60 seconds. */
 #define ALIGNMENT		4
 
 /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */
-#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT)
+#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32)adr)) % ALIGNMENT)
 
 #ifdef __BIG_ENDIAN
 #define xemaclite_readl		ioread32be
@@ -239,8 +239,8 @@ static void xemaclite_aligned_write(void *src_ptr, u32 *dest_ptr,
 
 		/* Set up to output the remaining data */
 		align_buffer = 0;
-		to_u8_ptr = (u8 *) &align_buffer;
-		from_u8_ptr = (u8 *) from_u16_ptr;
+		to_u8_ptr = (u8 *)&align_buffer;
+		from_u8_ptr = (u8 *)from_u16_ptr;
 
 		/* Output the remaining data */
 		for (; length > 0; length--)
@@ -273,7 +273,7 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
 	u32 align_buffer;
 
 	from_u32_ptr = src_ptr;
-	to_u16_ptr = (u16 *) dest_ptr;
+	to_u16_ptr = (u16 *)dest_ptr;
 
 	for (; length > 3; length -= 4) {
 		/* Copy each word into the temporary buffer */
@@ -289,9 +289,9 @@ static void xemaclite_aligned_read(u32 *src_ptr, u8 *dest_ptr,
 		u8 *to_u8_ptr, *from_u8_ptr;
 
 		/* Set up to read the remaining data */
-		to_u8_ptr = (u8 *) to_u16_ptr;
+		to_u8_ptr = (u8 *)to_u16_ptr;
 		align_buffer = *from_u32_ptr++;
-		from_u8_ptr = (u8 *) &align_buffer;
+		from_u8_ptr = (u8 *)&align_buffer;
 
 		/* Read the remaining data */
 		for (; length > 0; length--)
@@ -351,7 +351,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
 		return -1; /* Buffer was full, return failure */
 
 	/* Write the frame to the buffer */
-	xemaclite_aligned_write(data, (u32 __force *) addr, byte_count);
+	xemaclite_aligned_write(data, (u32 __force *)addr, byte_count);
 
 	xemaclite_writel((byte_count & XEL_TPLR_LENGTH_MASK),
 			 addr + XEL_TPLR_OFFSET);
@@ -448,7 +448,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
 		length = maxlen;
 
 	/* Read from the EmacLite device */
-	xemaclite_aligned_read((u32 __force *) (addr + XEL_RXBUFF_OFFSET),
+	xemaclite_aligned_read((u32 __force *)(addr + XEL_RXBUFF_OFFSET),
 				data, length);
 
 	/* Acknowledge the frame */
@@ -479,7 +479,7 @@ static void xemaclite_update_address(struct net_local *drvdata,
 	/* Determine the expected Tx buffer address */
 	addr = drvdata->base_addr + drvdata->next_tx_buf_to_use;
 
-	xemaclite_aligned_write(address_ptr, (u32 __force *) addr, ETH_ALEN);
+	xemaclite_aligned_write(address_ptr, (u32 __force *)addr, ETH_ALEN);
 
 	xemaclite_writel(ETH_ALEN, addr + XEL_TPLR_OFFSET);
 
@@ -574,7 +574,7 @@ static void xemaclite_tx_handler(struct net_device *dev)
 	dev->stats.tx_packets++;
 	if (lp->deferred_skb) {
 		if (xemaclite_send_data(lp,
-					(u8 *) lp->deferred_skb->data,
+					(u8 *)lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
 		dev->stats.tx_bytes += lp->deferred_skb->len;
@@ -619,7 +619,7 @@ static void xemaclite_rx_handler(struct net_device *dev)
 
 	skb_reserve(skb, 2);
 
-	len = xemaclite_recv_data(lp, (u8 *) skb->data, len);
+	len = xemaclite_recv_data(lp, (u8 *)skb->data, len);
 
 	if (!len) {
 		dev->stats.rx_errors++;
@@ -1032,7 +1032,7 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
 	new_skb = orig_skb;
 
 	spin_lock_irqsave(&lp->reset_lock, flags);
-	if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
+	if (xemaclite_send_data(lp, (u8 *)new_skb->data, len) != 0) {
 		/* If the Emaclite Tx buffer is busy, stop the Tx queue and
 		 * defer the skb for transmission during the ISR, after the
 		 * current transmission is complete
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
  2018-06-18 11:08   ` Radhey Shyam Pandey
@ 2018-06-19 21:36     ` Andy Shevchenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2018-06-19 21:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey
  Cc: David S. Miller, Andrew Lunn, Michal Simek, netdev,
	linux-arm Mailing List, Linux Kernel Mailing List

On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
<radhey.shyam.pandey@xilinx.com> wrote:
> Switch hardcoded function name with a reference to __func__ making
> the code more maintainable. Address below checkpatch warning:
>
> WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
> this function's name, in a string
> +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
>
> WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
> this function's name, in a string
> +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
>

For dev_dbg() the __func__ should be completely dropped away.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
@ 2018-06-19 21:36     ` Andy Shevchenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2018-06-19 21:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
<radhey.shyam.pandey@xilinx.com> wrote:
> Switch hardcoded function name with a reference to __func__ making
> the code more maintainable. Address below checkpatch warning:
>
> WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
> this function's name, in a string
> +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
>
> WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
> this function's name, in a string
> +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
>

For dev_dbg() the __func__ should be completely dropped away.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
  2018-06-19 21:36     ` Andy Shevchenko
@ 2018-06-19 22:37       ` Joe Perches
  -1 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-06-19 22:37 UTC (permalink / raw)
  To: Andy Shevchenko, Radhey Shyam Pandey
  Cc: David S. Miller, Andrew Lunn, Michal Simek, netdev,
	linux-arm Mailing List, Linux Kernel Mailing List

On Wed, 2018-06-20 at 00:36 +0300, Andy Shevchenko wrote:
> On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
> <radhey.shyam.pandey@xilinx.com> wrote:
> > Switch hardcoded function name with a reference to __func__ making
> > the code more maintainable. Address below checkpatch warning:
> > 
> > WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
> > this function's name, in a string
> > +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
> > 
> > WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
> > this function's name, in a string
> > +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
> > 
> 
> For dev_dbg() the __func__ should be completely dropped away.

Not really the same.

dev_dbg without CONFIG_DYNAMIC_DEBUG does not have
the ability to prefix __func__.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
@ 2018-06-19 22:37       ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-06-19 22:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 2018-06-20 at 00:36 +0300, Andy Shevchenko wrote:
> On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
> <radhey.shyam.pandey@xilinx.com> wrote:
> > Switch hardcoded function name with a reference to __func__ making
> > the code more maintainable. Address below checkpatch warning:
> > 
> > WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_read',
> > this function's name, in a string
> > +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
> > 
> > WARNING: Prefer using '"%s...", __func__' to using 'xemaclite_mdio_write',
> > this function's name, in a string
> > +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
> > 
> 
> For dev_dbg() the __func__ should be completely dropped away.

Not really the same.

dev_dbg without CONFIG_DYNAMIC_DEBUG does not have
the ability to prefix __func__.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
  2018-06-19 22:37       ` Joe Perches
@ 2018-06-25 13:47         ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-25 13:47 UTC (permalink / raw)
  To: Joe Perches, Andy Shevchenko
  Cc: David S. Miller, Andrew Lunn, Michal Simek, netdev,
	linux-arm Mailing List, Linux Kernel Mailing List

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Wednesday, June 20, 2018 4:08 AM
> To: Andy Shevchenko <andy.shevchenko@gmail.com>; Radhey Shyam
> Pandey <radheys@xilinx.com>
> Cc: David S. Miller <davem@davemloft.net>; Andrew Lunn
> <andrew@lunn.ch>; Michal Simek <michals@xilinx.com>; netdev
> <netdev@vger.kernel.org>; linux-arm Mailing List <linux-arm-
> kernel@lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded
> name
> 
> On Wed, 2018-06-20 at 00:36 +0300, Andy Shevchenko wrote:
> > On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
> > <radhey.shyam.pandey@xilinx.com> wrote:
> > > Switch hardcoded function name with a reference to __func__ making
> > > the code more maintainable. Address below checkpatch warning:
> > >
> > > WARNING: Prefer using '"%s...", __func__' to using
> 'xemaclite_mdio_read',
> > > this function's name, in a string
> > > +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
> > >
> > > WARNING: Prefer using '"%s...", __func__' to using
> 'xemaclite_mdio_write',
> > > this function's name, in a string
> > > +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
> > >
> >
> > For dev_dbg() the __func__ should be completely dropped away.
> 
> Not really the same.
> 
> dev_dbg without CONFIG_DYNAMIC_DEBUG does not have
> the ability to prefix __func__.
Yes.  If it's acceptable,  prefer to use __func__  to support all 
configurations

> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name
@ 2018-06-25 13:47         ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-25 13:47 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Joe Perches [mailto:joe at perches.com]
> Sent: Wednesday, June 20, 2018 4:08 AM
> To: Andy Shevchenko <andy.shevchenko@gmail.com>; Radhey Shyam
> Pandey <radheys@xilinx.com>
> Cc: David S. Miller <davem@davemloft.net>; Andrew Lunn
> <andrew@lunn.ch>; Michal Simek <michals@xilinx.com>; netdev
> <netdev@vger.kernel.org>; linux-arm Mailing List <linux-arm-
> kernel at lists.infradead.org>; Linux Kernel Mailing List <linux-
> kernel at vger.kernel.org>
> Subject: Re: [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded
> name
> 
> On Wed, 2018-06-20 at 00:36 +0300, Andy Shevchenko wrote:
> > On Mon, Jun 18, 2018 at 2:08 PM, Radhey Shyam Pandey
> > <radhey.shyam.pandey@xilinx.com> wrote:
> > > Switch hardcoded function name with a reference to __func__ making
> > > the code more maintainable. Address below checkpatch warning:
> > >
> > > WARNING: Prefer using '"%s...", __func__' to using
> 'xemaclite_mdio_read',
> > > this function's name, in a string
> > > +               "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
> > >
> > > WARNING: Prefer using '"%s...", __func__' to using
> 'xemaclite_mdio_write',
> > > this function's name, in a string
> > > +               "xemaclite_mdio_write(phy_id=%i, reg=%x, val=%x)\n",
> > >
> >
> > For dev_dbg() the __func__ should be completely dropped away.
> 
> Not really the same.
> 
> dev_dbg without CONFIG_DYNAMIC_DEBUG does not have
> the ability to prefix __func__.
Yes.  If it's acceptable,  prefer to use __func__  to support all 
configurations

> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 2/5] net: emaclite: Balance braces in else statement
  2018-06-18 16:03     ` Joe Perches
  (?)
@ 2018-06-19  5:42       ` Radhey Shyam Pandey
  -1 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-19  5:42 UTC (permalink / raw)
  To: Joe Perches, davem, andrew, Michal Simek
  Cc: netdev, linux-arm-kernel, linux-kernel

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Monday, June 18, 2018 9:33 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>; davem@davemloft.net;
> andrew@lunn.ch; Michal Simek <michals@xilinx.com>
> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/5] net: emaclite: Balance braces in else statement
> 
> On Mon, 2018-06-18 at 17:20 +0530, Radhey Shyam Pandey wrote:
> > Remove else as it is not required with if doing a return.
> > Fixes below checkpatch warning.
> 
> > WARNING: else is not generally useful after a break or return
> 
> checkpatch is stupid and doesn't understand code flow.
> Always try to improve code flow instead of merely
> following brainless instructions from a script.
> 
> So:
> 
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> []
> > @@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device
> *dev)
> >  					(u8 *) lp->deferred_skb->data,
> >  					lp->deferred_skb->len) != 0)
> >  			return;
> > -		else {
> > -			dev->stats.tx_bytes += lp->deferred_skb->len;
> > -			dev_kfree_skb_irq(lp->deferred_skb);
> > -			lp->deferred_skb = NULL;
> > -			netif_trans_update(dev); /* prevent tx timeout */
> > -			netif_wake_queue(dev);
> > -		}
> > +		dev->stats.tx_bytes += lp->deferred_skb->len;
> > +		dev_kfree_skb_irq(lp->deferred_skb);
> > +		lp->deferred_skb = NULL;
> > +		netif_trans_update(dev); /* prevent tx timeout */
> > +		netif_wake_queue(dev);
> >  	}
> >  }
> 
> If you really want to redo this function, perhaps something like:
Thanks for the review. Yes, In v2 I will refactor the code to have
failure path return early.

> 
> static void xemaclite_tx_handler(struct net_device *dev)
> {
> 	struct net_local *lp = netdev_priv(dev);
> 
> 	dev->stats.tx_packets++;
> 
> 	if (!lp->deferred_skb)
> 		return;
> 
> 	if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
> 				lp->deferred_skb->len))
> 		return;
> 
> 	dev->stats.tx_bytes += lp->deferred_skb->len;
> 	dev_kfree_skb_irq(lp->deferred_skb);
> 	lp->deferred_skb = NULL;
> 	netif_trans_update(dev); /* prevent tx timeout */
> 	netif_wake_queue(dev);
> }
> 
> > @@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device
> *ofdev, const char *s)
> >  {
> >  	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> >
> > -	if (p) {
> > +	if (p)
> >  		return (bool)*p;
> > -	} else {
> > -		dev_warn(&ofdev->dev, "Parameter %s not found,"
> > +
> > +	dev_warn(&ofdev->dev, "Parameter %s not found,"
> >  			"defaulting to false\n", s);
> > -		return false;
> > -	}
> > +
> > +	return false;
> >  }
> 
> And this function has backward logic as the failure paths
> are the ones that should return early or use a goto.
> 
> Perhaps something like:
Yes, will change it. 

> 
> static bool get_bool(struct platform_device *ofdev, const char *s)
> {
> 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> 
> 	if (!p) {
> 		dev_warn(&ofdev->dev,
> 			 "Parameter '%s' not found, defaulting to false\n", s);
> 		return false;
> 	}
> 
> 	return *p;
> }


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 2/5] net: emaclite: Balance braces in else statement
@ 2018-06-19  5:42       ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-19  5:42 UTC (permalink / raw)
  To: Joe Perches, davem, andrew, Michal Simek
  Cc: netdev, linux-arm-kernel, linux-kernel

> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Monday, June 18, 2018 9:33 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>; davem@davemloft.net;
> andrew@lunn.ch; Michal Simek <michals@xilinx.com>
> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 2/5] net: emaclite: Balance braces in else statement
> 
> On Mon, 2018-06-18 at 17:20 +0530, Radhey Shyam Pandey wrote:
> > Remove else as it is not required with if doing a return.
> > Fixes below checkpatch warning.
> 
> > WARNING: else is not generally useful after a break or return
> 
> checkpatch is stupid and doesn't understand code flow.
> Always try to improve code flow instead of merely
> following brainless instructions from a script.
> 
> So:
> 
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> []
> > @@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device
> *dev)
> >  					(u8 *) lp->deferred_skb->data,
> >  					lp->deferred_skb->len) != 0)
> >  			return;
> > -		else {
> > -			dev->stats.tx_bytes += lp->deferred_skb->len;
> > -			dev_kfree_skb_irq(lp->deferred_skb);
> > -			lp->deferred_skb = NULL;
> > -			netif_trans_update(dev); /* prevent tx timeout */
> > -			netif_wake_queue(dev);
> > -		}
> > +		dev->stats.tx_bytes += lp->deferred_skb->len;
> > +		dev_kfree_skb_irq(lp->deferred_skb);
> > +		lp->deferred_skb = NULL;
> > +		netif_trans_update(dev); /* prevent tx timeout */
> > +		netif_wake_queue(dev);
> >  	}
> >  }
> 
> If you really want to redo this function, perhaps something like:
Thanks for the review. Yes, In v2 I will refactor the code to have
failure path return early.

> 
> static void xemaclite_tx_handler(struct net_device *dev)
> {
> 	struct net_local *lp = netdev_priv(dev);
> 
> 	dev->stats.tx_packets++;
> 
> 	if (!lp->deferred_skb)
> 		return;
> 
> 	if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
> 				lp->deferred_skb->len))
> 		return;
> 
> 	dev->stats.tx_bytes += lp->deferred_skb->len;
> 	dev_kfree_skb_irq(lp->deferred_skb);
> 	lp->deferred_skb = NULL;
> 	netif_trans_update(dev); /* prevent tx timeout */
> 	netif_wake_queue(dev);
> }
> 
> > @@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device
> *ofdev, const char *s)
> >  {
> >  	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> >
> > -	if (p) {
> > +	if (p)
> >  		return (bool)*p;
> > -	} else {
> > -		dev_warn(&ofdev->dev, "Parameter %s not found,"
> > +
> > +	dev_warn(&ofdev->dev, "Parameter %s not found,"
> >  			"defaulting to false\n", s);
> > -		return false;
> > -	}
> > +
> > +	return false;
> >  }
> 
> And this function has backward logic as the failure paths
> are the ones that should return early or use a goto.
> 
> Perhaps something like:
Yes, will change it. 

> 
> static bool get_bool(struct platform_device *ofdev, const char *s)
> {
> 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> 
> 	if (!p) {
> 		dev_warn(&ofdev->dev,
> 			 "Parameter '%s' not found, defaulting to false\n", s);
> 		return false;
> 	}
> 
> 	return *p;
> }

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
@ 2018-06-19  5:42       ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-19  5:42 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Joe Perches [mailto:joe at perches.com]
> Sent: Monday, June 18, 2018 9:33 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>; davem at davemloft.net;
> andrew at lunn.ch; Michal Simek <michals@xilinx.com>
> Cc: netdev at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org
> Subject: Re: [PATCH 2/5] net: emaclite: Balance braces in else statement
> 
> On Mon, 2018-06-18 at 17:20 +0530, Radhey Shyam Pandey wrote:
> > Remove else as it is not required with if doing a return.
> > Fixes below checkpatch warning.
> 
> > WARNING: else is not generally useful after a break or return
> 
> checkpatch is stupid and doesn't understand code flow.
> Always try to improve code flow instead of merely
> following brainless instructions from a script.
> 
> So:
> 
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> []
> > @@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device
> *dev)
> >  					(u8 *) lp->deferred_skb->data,
> >  					lp->deferred_skb->len) != 0)
> >  			return;
> > -		else {
> > -			dev->stats.tx_bytes += lp->deferred_skb->len;
> > -			dev_kfree_skb_irq(lp->deferred_skb);
> > -			lp->deferred_skb = NULL;
> > -			netif_trans_update(dev); /* prevent tx timeout */
> > -			netif_wake_queue(dev);
> > -		}
> > +		dev->stats.tx_bytes += lp->deferred_skb->len;
> > +		dev_kfree_skb_irq(lp->deferred_skb);
> > +		lp->deferred_skb = NULL;
> > +		netif_trans_update(dev); /* prevent tx timeout */
> > +		netif_wake_queue(dev);
> >  	}
> >  }
> 
> If you really want to redo this function, perhaps something like:
Thanks for the review. Yes, In v2 I will refactor the code to have
failure path return early.

> 
> static void xemaclite_tx_handler(struct net_device *dev)
> {
> 	struct net_local *lp = netdev_priv(dev);
> 
> 	dev->stats.tx_packets++;
> 
> 	if (!lp->deferred_skb)
> 		return;
> 
> 	if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
> 				lp->deferred_skb->len))
> 		return;
> 
> 	dev->stats.tx_bytes += lp->deferred_skb->len;
> 	dev_kfree_skb_irq(lp->deferred_skb);
> 	lp->deferred_skb = NULL;
> 	netif_trans_update(dev); /* prevent tx timeout */
> 	netif_wake_queue(dev);
> }
> 
> > @@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device
> *ofdev, const char *s)
> >  {
> >  	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> >
> > -	if (p) {
> > +	if (p)
> >  		return (bool)*p;
> > -	} else {
> > -		dev_warn(&ofdev->dev, "Parameter %s not found,"
> > +
> > +	dev_warn(&ofdev->dev, "Parameter %s not found,"
> >  			"defaulting to false\n", s);
> > -		return false;
> > -	}
> > +
> > +	return false;
> >  }
> 
> And this function has backward logic as the failure paths
> are the ones that should return early or use a goto.
> 
> Perhaps something like:
Yes, will change it. 

> 
> static bool get_bool(struct platform_device *ofdev, const char *s)
> {
> 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
> 
> 	if (!p) {
> 		dev_warn(&ofdev->dev,
> 			 "Parameter '%s' not found, defaulting to false\n", s);
> 		return false;
> 	}
> 
> 	return *p;
> }

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] net: emaclite: Balance braces in else statement
  2018-06-18 11:50   ` Radhey Shyam Pandey
@ 2018-06-18 16:03     ` Joe Perches
  -1 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-06-18 16:03 UTC (permalink / raw)
  To: Radhey Shyam Pandey, davem, andrew, michal.simek
  Cc: netdev, linux-arm-kernel, linux-kernel

On Mon, 2018-06-18 at 17:20 +0530, Radhey Shyam Pandey wrote:
> Remove else as it is not required with if doing a return.
> Fixes below checkpatch warning.

> WARNING: else is not generally useful after a break or return

checkpatch is stupid and doesn't understand code flow.
Always try to improve code flow instead of merely
following brainless instructions from a script.

So:

> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
[]
> @@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
>  					(u8 *) lp->deferred_skb->data,
>  					lp->deferred_skb->len) != 0)
>  			return;
> -		else {
> -			dev->stats.tx_bytes += lp->deferred_skb->len;
> -			dev_kfree_skb_irq(lp->deferred_skb);
> -			lp->deferred_skb = NULL;
> -			netif_trans_update(dev); /* prevent tx timeout */
> -			netif_wake_queue(dev);
> -		}
> +		dev->stats.tx_bytes += lp->deferred_skb->len;
> +		dev_kfree_skb_irq(lp->deferred_skb);
> +		lp->deferred_skb = NULL;
> +		netif_trans_update(dev); /* prevent tx timeout */
> +		netif_wake_queue(dev);
>  	}
>  }

If you really want to redo this function, perhaps something like:

static void xemaclite_tx_handler(struct net_device *dev)
{
	struct net_local *lp = netdev_priv(dev);

	dev->stats.tx_packets++;

	if (!lp->deferred_skb)
		return;

	if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
				lp->deferred_skb->len))
		return;

	dev->stats.tx_bytes += lp->deferred_skb->len;
	dev_kfree_skb_irq(lp->deferred_skb);
	lp->deferred_skb = NULL;
	netif_trans_update(dev); /* prevent tx timeout */
	netif_wake_queue(dev);
}
 
> @@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
>  {
>  	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
>  
> -	if (p) {
> +	if (p)
>  		return (bool)*p;
> -	} else {
> -		dev_warn(&ofdev->dev, "Parameter %s not found,"
> +
> +	dev_warn(&ofdev->dev, "Parameter %s not found,"
>  			"defaulting to false\n", s);
> -		return false;
> -	}
> +
> +	return false;
>  }

And this function has backward logic as the failure paths
are the ones that should return early or use a goto.

Perhaps something like:

static bool get_bool(struct platform_device *ofdev, const char *s)
{
	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);

	if (!p) {
		dev_warn(&ofdev->dev,
			 "Parameter '%s' not found, defaulting to false\n", s);
		return false;
	}

	return *p;
}


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
@ 2018-06-18 16:03     ` Joe Perches
  0 siblings, 0 replies; 25+ messages in thread
From: Joe Perches @ 2018-06-18 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2018-06-18 at 17:20 +0530, Radhey Shyam Pandey wrote:
> Remove else as it is not required with if doing a return.
> Fixes below checkpatch warning.

> WARNING: else is not generally useful after a break or return

checkpatch is stupid and doesn't understand code flow.
Always try to improve code flow instead of merely
following brainless instructions from a script.

So:

> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
[]
> @@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
>  					(u8 *) lp->deferred_skb->data,
>  					lp->deferred_skb->len) != 0)
>  			return;
> -		else {
> -			dev->stats.tx_bytes += lp->deferred_skb->len;
> -			dev_kfree_skb_irq(lp->deferred_skb);
> -			lp->deferred_skb = NULL;
> -			netif_trans_update(dev); /* prevent tx timeout */
> -			netif_wake_queue(dev);
> -		}
> +		dev->stats.tx_bytes += lp->deferred_skb->len;
> +		dev_kfree_skb_irq(lp->deferred_skb);
> +		lp->deferred_skb = NULL;
> +		netif_trans_update(dev); /* prevent tx timeout */
> +		netif_wake_queue(dev);
>  	}
>  }

If you really want to redo this function, perhaps something like:

static void xemaclite_tx_handler(struct net_device *dev)
{
	struct net_local *lp = netdev_priv(dev);

	dev->stats.tx_packets++;

	if (!lp->deferred_skb)
		return;

	if (xemaclite_send_data(lp, (u8 *)lp->deferred_skb->data,
				lp->deferred_skb->len))
		return;

	dev->stats.tx_bytes += lp->deferred_skb->len;
	dev_kfree_skb_irq(lp->deferred_skb);
	lp->deferred_skb = NULL;
	netif_trans_update(dev); /* prevent tx timeout */
	netif_wake_queue(dev);
}
 
> @@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
>  {
>  	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
>  
> -	if (p) {
> +	if (p)
>  		return (bool)*p;
> -	} else {
> -		dev_warn(&ofdev->dev, "Parameter %s not found,"
> +
> +	dev_warn(&ofdev->dev, "Parameter %s not found,"
>  			"defaulting to false\n", s);
> -		return false;
> -	}
> +
> +	return false;
>  }

And this function has backward logic as the failure paths
are the ones that should return early or use a goto.

Perhaps something like:

static bool get_bool(struct platform_device *ofdev, const char *s)
{
	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);

	if (!p) {
		dev_warn(&ofdev->dev,
			 "Parameter '%s' not found, defaulting to false\n", s);
		return false;
	}

	return *p;
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
  2018-06-18 11:50 [PATCH 0/5] Fixes coding style in xilinx_emaclite.c Radhey Shyam Pandey
@ 2018-06-18 11:50   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:50 UTC (permalink / raw)
  To: davem, andrew, michal.simek, radhey.shyam.pandey
  Cc: netdev, linux-arm-kernel, linux-kernel

Remove else as it is not required with if doing a return.
Fixes below checkpatch warning.

WARNING: else is not generally useful after a break or return

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0544134..8d84f58 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
 					(u8 *) lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
-		else {
-			dev->stats.tx_bytes += lp->deferred_skb->len;
-			dev_kfree_skb_irq(lp->deferred_skb);
-			lp->deferred_skb = NULL;
-			netif_trans_update(dev); /* prevent tx timeout */
-			netif_wake_queue(dev);
-		}
+		dev->stats.tx_bytes += lp->deferred_skb->len;
+		dev_kfree_skb_irq(lp->deferred_skb);
+		lp->deferred_skb = NULL;
+		netif_trans_update(dev); /* prevent tx timeout */
+		netif_wake_queue(dev);
 	}
 }
 
@@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
 {
 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
 
-	if (p) {
+	if (p)
 		return (bool)*p;
-	} else {
-		dev_warn(&ofdev->dev, "Parameter %s not found,"
+
+	dev_warn(&ofdev->dev, "Parameter %s not found,"
 			"defaulting to false\n", s);
-		return false;
-	}
+
+	return false;
 }
 
 static const struct net_device_ops xemaclite_netdev_ops;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/5] net: emaclite: Balance braces in else statement
@ 2018-06-18 11:50   ` Radhey Shyam Pandey
  0 siblings, 0 replies; 25+ messages in thread
From: Radhey Shyam Pandey @ 2018-06-18 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

Remove else as it is not required with if doing a return.
Fixes below checkpatch warning.

WARNING: else is not generally useful after a break or return

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 0544134..8d84f58 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -569,13 +569,11 @@ static void xemaclite_tx_handler(struct net_device *dev)
 					(u8 *) lp->deferred_skb->data,
 					lp->deferred_skb->len) != 0)
 			return;
-		else {
-			dev->stats.tx_bytes += lp->deferred_skb->len;
-			dev_kfree_skb_irq(lp->deferred_skb);
-			lp->deferred_skb = NULL;
-			netif_trans_update(dev); /* prevent tx timeout */
-			netif_wake_queue(dev);
-		}
+		dev->stats.tx_bytes += lp->deferred_skb->len;
+		dev_kfree_skb_irq(lp->deferred_skb);
+		lp->deferred_skb = NULL;
+		netif_trans_update(dev); /* prevent tx timeout */
+		netif_wake_queue(dev);
 	}
 }
 
@@ -1052,13 +1050,13 @@ static bool get_bool(struct platform_device *ofdev, const char *s)
 {
 	u32 *p = (u32 *)of_get_property(ofdev->dev.of_node, s, NULL);
 
-	if (p) {
+	if (p)
 		return (bool)*p;
-	} else {
-		dev_warn(&ofdev->dev, "Parameter %s not found,"
+
+	dev_warn(&ofdev->dev, "Parameter %s not found,"
 			"defaulting to false\n", s);
-		return false;
-	}
+
+	return false;
 }
 
 static const struct net_device_ops xemaclite_netdev_ops;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2018-06-25 13:48 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-18 11:08 [PATCH 0/5] Fixes coding style in xilinx_emaclite.c Radhey Shyam Pandey
2018-06-18 11:08 ` Radhey Shyam Pandey
2018-06-18 11:08 ` [PATCH 1/5] net: emaclite: Use __func__ instead of hardcoded name Radhey Shyam Pandey
2018-06-18 11:08   ` Radhey Shyam Pandey
2018-06-19 21:36   ` Andy Shevchenko
2018-06-19 21:36     ` Andy Shevchenko
2018-06-19 22:37     ` Joe Perches
2018-06-19 22:37       ` Joe Perches
2018-06-25 13:47       ` Radhey Shyam Pandey
2018-06-25 13:47         ` Radhey Shyam Pandey
2018-06-18 11:08 ` [PATCH 2/5] net: emaclite: Balance braces in else statement Radhey Shyam Pandey
2018-06-18 11:08   ` Radhey Shyam Pandey
2018-06-18 11:08 ` [PATCH 3/5] net: emaclite: update kernel-doc comments Radhey Shyam Pandey
2018-06-18 11:08   ` Radhey Shyam Pandey
2018-06-18 11:08 ` [PATCH 4/5] net: emaclite: Fix block comments style Radhey Shyam Pandey
2018-06-18 11:08   ` Radhey Shyam Pandey
2018-06-18 11:08 ` [PATCH 5/5] net: emaclite: Remove unnecessary spaces Radhey Shyam Pandey
2018-06-18 11:08   ` Radhey Shyam Pandey
2018-06-18 11:50 [PATCH 0/5] Fixes coding style in xilinx_emaclite.c Radhey Shyam Pandey
2018-06-18 11:50 ` [PATCH 2/5] net: emaclite: Balance braces in else statement Radhey Shyam Pandey
2018-06-18 11:50   ` Radhey Shyam Pandey
2018-06-18 16:03   ` Joe Perches
2018-06-18 16:03     ` Joe Perches
2018-06-19  5:42     ` Radhey Shyam Pandey
2018-06-19  5:42       ` Radhey Shyam Pandey
2018-06-19  5:42       ` Radhey Shyam Pandey

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.