diff mbox

[08/14] net: axienet: Removed checkpatch errors/warnings

Message ID 75b669c0a947effe74b291093abfa8c71f83736a.1392220536.git.michal.simek@xilinx.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Michal Simek Feb. 12, 2014, 3:55 p.m. UTC 
From: Srikanth Thokala <srikanth.thokala@xilinx.com>

Removed checkpatch.pl errors and warnings.

Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 20 ++++++++++----------
 drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 21 +++++++++++----------
 2 files changed, 21 insertions(+), 20 deletions(-)

--
1.8.2.3

Comments

Joe Perches Feb. 13, 2014, 12:31 a.m. UTC | #1 
On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote:
> From: Srikanth Thokala <srikanth.thokala@xilinx.com>

trivia:

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c

> +			netdev_err(lp->ndev,
> +				"axienet_device_reset DMA reset timeout!\n");

could you please align multi-line arguments to the
appropriate open parenthesis?

			netdev_err(lp->ndev,
				   "axienet_device_reset DMA reset timeout!\n");

or maybe:

			netdev_err(lp->ndev, "%s: "DMA reset timeout!\n",
				   __func__);

> @@ -484,8 +484,8 @@ static void axienet_device_reset(struct net_device *ndev)
>  	}
> 
>  	if (axienet_dma_bd_init(ndev)) {
> -		dev_err(&ndev->dev, "axienet_device_reset descriptor "
> -			"allocation failed\n");
> +		netdev_err(ndev,
> +			"axienet_device_reset descriptor allocation failed\n");

etc, et al.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
[]
> @@ -161,19 +161,19 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)
> 
>  	np1 = of_find_node_by_name(NULL, "cpu");
>  	if (!np1) {
> -		printk(KERN_WARNING "%s(): Could not find CPU device node.",
> -		       __func__);
> -		printk(KERN_WARNING "Setting MDIO clock divisor to "
> -		       "default %d\n", DEFAULT_CLOCK_DIVISOR);
> +		netdev_warn(lp->ndev, "Could not find CPU device node.");

missing trailing "\n" to terminate message.

> +		netdev_warn(lp->ndev,
> +			 "Could not find clock ethernet controller property.");

here too. (and alignment)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Simek Feb. 13, 2014, 7:19 a.m. UTC | #2 
Hi Joe,

On 02/13/2014 01:31 AM, Joe Perches wrote:
> On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote:
>> From: Srikanth Thokala <srikanth.thokala@xilinx.com>
> 
> trivia:
> 
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> 
>> +			netdev_err(lp->ndev,
>> +				"axienet_device_reset DMA reset timeout!\n");
> 
> could you please align multi-line arguments to the
> appropriate open parenthesis?
> 
> 			netdev_err(lp->ndev,
> 				   "axienet_device_reset DMA reset timeout!\n");
> 
> or maybe:
> 
> 			netdev_err(lp->ndev, "%s: "DMA reset timeout!\n",
> 				   __func__);

ok.

> 
>> @@ -484,8 +484,8 @@ static void axienet_device_reset(struct net_device *ndev)
>>  	}
>>
>>  	if (axienet_dma_bd_init(ndev)) {
>> -		dev_err(&ndev->dev, "axienet_device_reset descriptor "
>> -			"allocation failed\n");
>> +		netdev_err(ndev,
>> +			"axienet_device_reset descriptor allocation failed\n");
> 
> etc, et al.


ok.

> 
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
> []
>> @@ -161,19 +161,19 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)
>>
>>  	np1 = of_find_node_by_name(NULL, "cpu");
>>  	if (!np1) {
>> -		printk(KERN_WARNING "%s(): Could not find CPU device node.",
>> -		       __func__);
>> -		printk(KERN_WARNING "Setting MDIO clock divisor to "
>> -		       "default %d\n", DEFAULT_CLOCK_DIVISOR);
>> +		netdev_warn(lp->ndev, "Could not find CPU device node.");
> 
> missing trailing "\n" to terminate message.

ok.

> 
>> +		netdev_warn(lp->ndev,
>> +			 "Could not find clock ethernet controller property.");
> 
> here too. (and alignment)

This is problematic. I would like to keep 80 char limits and keeping
this align just break it. That's why I was using tab alignment.
Probably the solution is just to shorten message.

Thanks for your comments,
Michal


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches Feb. 13, 2014, 3:51 p.m. UTC | #3 
On Thu, 2014-02-13 at 08:19 +0100, Michal Simek wrote:
> On 02/13/2014 01:31 AM, Joe Perches wrote:
> > On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote:

Hi again Michal.

> >> +		netdev_warn(lp->ndev,
> >> +			 "Could not find clock ethernet controller property.");
> > 
> > here too. (and alignment)
> 
> This is problematic. I would like to keep 80 char limits and keeping
> this align just break it. That's why I was using tab alignment.
> Probably the solution is just to shorten message.

(overly long, tiresomely trivial stuff below)

Your choice.  I'm not an 80 column zealot but
please don't shorten the message just to fit
80 columns if it impacts intelligibility.

Generally, I'd write this something like:

		netdev_warn(lp->ndev,
			    "Could not find clock ethernet controller property\n");

(without the period) which is 83 columns.

checkpatch makes exceptions for 80 column line
length maximums for format strings.

I've no real issue if you indent it back one.

fyi: this is 77 columns

		netdev_warn(lp->ndev,
			    "No clock ethernet controller property found\n");

About the message itself.

You dropped the "axienet_mdio_setup" function name.

I believe the dmesg output will look something like:

xilinx_temac 0000:01:00.0 (unregistered net_device): Could not find clock ethernet controller property.
xilinx_temac 0000:01:00.0 (unregistered net_device): Setting MDIO clock divisor to default 29

Because these 2 messages are effectively linked,
my preference would be to emit them on a single line,

Something like:

xilinx_temac 0000:01:00.0 (unregistered net_device): of_get_property("clock-frequency") not found - setting MDIO clock divisor to default 29

or

		netdev_warn(lp->ndev,
			    "of_get_property(\"clock-frequency\") not found - setting MDIO clock divisor to default %u\n",
			    DEFAULT_CLOCK_DIVISOR);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Simek Feb. 14, 2014, 9:21 a.m. UTC | #4 
On 02/13/2014 04:51 PM, Joe Perches wrote:
> On Thu, 2014-02-13 at 08:19 +0100, Michal Simek wrote:
>> On 02/13/2014 01:31 AM, Joe Perches wrote:
>>> On Wed, 2014-02-12 at 16:55 +0100, Michal Simek wrote:
> 
> Hi again Michal.
> 
>>>> +		netdev_warn(lp->ndev,
>>>> +			 "Could not find clock ethernet controller property.");
>>>
>>> here too. (and alignment)
>>
>> This is problematic. I would like to keep 80 char limits and keeping
>> this align just break it. That's why I was using tab alignment.
>> Probably the solution is just to shorten message.
> 
> (overly long, tiresomely trivial stuff below)
> 
> Your choice.  I'm not an 80 column zealot but
> please don't shorten the message just to fit
> 80 columns if it impacts intelligibility.

I am trying to keep 80 chars and follow subsystem
standards.

> Generally, I'd write this something like:
> 
> 		netdev_warn(lp->ndev,
> 			    "Could not find clock ethernet controller property\n");
> 
> (without the period) which is 83 columns.

ok.

> checkpatch makes exceptions for 80 column line
> length maximums for format strings.

yes but testing systems reports it because that 80 chars
is still default value.


> 
> I've no real issue if you indent it back one.
> 
> fyi: this is 77 columns
> 
> 		netdev_warn(lp->ndev,
> 			    "No clock ethernet controller property found\n");
> 
> About the message itself.
> 
> You dropped the "axienet_mdio_setup" function name.
> 
> I believe the dmesg output will look something like:
> 
> xilinx_temac 0000:01:00.0 (unregistered net_device): Could not find clock ethernet controller property.
> xilinx_temac 0000:01:00.0 (unregistered net_device): Setting MDIO clock divisor to default 29
> 
> Because these 2 messages are effectively linked,
> my preference would be to emit them on a single line,
> 
> Something like:
> 
> xilinx_temac 0000:01:00.0 (unregistered net_device): of_get_property("clock-frequency") not found - setting MDIO clock divisor to default 29
> 
> or
> 
> 		netdev_warn(lp->ndev,
> 			    "of_get_property(\"clock-frequency\") not found - setting MDIO clock divisor to default %u\n",
> 			    DEFAULT_CLOCK_DIVISOR);
> 

But then you are breaking 80 char limits a lot.

Thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 9cc9e59..6059a0f 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -444,8 +444,8 @@  static void __axienet_device_reset(struct axienet_local *lp,
 	while (axienet_dma_in32(lp, offset) & XAXIDMA_CR_RESET_MASK) {
 		udelay(1);
 		if (--timeout == 0) {
-			dev_err(dev, "axienet_device_reset DMA "
-				"reset timeout!\n");
+			netdev_err(lp->ndev,
+				"axienet_device_reset DMA reset timeout!\n");
 			break;
 		}
 	}
@@ -484,8 +484,8 @@  static void axienet_device_reset(struct net_device *ndev)
 	}

 	if (axienet_dma_bd_init(ndev)) {
-		dev_err(&ndev->dev, "axienet_device_reset descriptor "
-			"allocation failed\n");
+		netdev_err(ndev,
+			"axienet_device_reset descriptor allocation failed\n");
 	}

 	axienet_status = axienet_ior(lp, XAE_RCW1_OFFSET);
@@ -560,8 +560,8 @@  static void axienet_adjust_link(struct net_device *ndev)
 			lp->last_link = link_state;
 			phy_print_status(phy);
 		} else {
-			dev_err(&ndev->dev, "Error setting Axi Ethernet "
-				"mac speed\n");
+			netdev_err(ndev,
+				   "Error setting Axi Ethernet mac speed\n");
 		}
 	}
 }
@@ -1238,8 +1238,8 @@  axienet_ethtools_set_pauseparam(struct net_device *ndev,
 	struct axienet_local *lp = netdev_priv(ndev);

 	if (netif_running(ndev)) {
-		printk(KERN_ERR	"%s: Please stop netif before applying "
-		       "configruation\n", ndev->name);
+		netdev_err(ndev,
+			   "Please stop netif before applying configuration\n");
 		return -EFAULT;
 	}

@@ -1295,8 +1295,8 @@  static int axienet_ethtools_set_coalesce(struct net_device *ndev,
 	struct axienet_local *lp = netdev_priv(ndev);

 	if (netif_running(ndev)) {
-		printk(KERN_ERR	"%s: Please stop netif before applying "
-		       "configruation\n", ndev->name);
+		netdev_err(ndev,
+			"Please stop netif before applying configuration\n");
 		return -EFAULT;
 	}

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
index 64b4639..ef0a20c 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
@@ -161,19 +161,19 @@  int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)

 	np1 = of_find_node_by_name(NULL, "cpu");
 	if (!np1) {
-		printk(KERN_WARNING "%s(): Could not find CPU device node.",
-		       __func__);
-		printk(KERN_WARNING "Setting MDIO clock divisor to "
-		       "default %d\n", DEFAULT_CLOCK_DIVISOR);
+		netdev_warn(lp->ndev, "Could not find CPU device node.");
+		netdev_warn(lp->ndev,
+			    "Setting MDIO clock divisor to default %d\n",
+			    DEFAULT_CLOCK_DIVISOR);
 		clk_div = DEFAULT_CLOCK_DIVISOR;
 		goto issue;
 	}
 	property_p = (u32 *) of_get_property(np1, "clock-frequency", NULL);
 	if (!property_p) {
-		printk(KERN_WARNING "%s(): Could not find CPU property: "
-		       "clock-frequency.", __func__);
-		printk(KERN_WARNING "Setting MDIO clock divisor to "
-		       "default %d\n", DEFAULT_CLOCK_DIVISOR);
+		netdev_warn(lp->ndev,
+			 "Could not find clock ethernet controller property.");
+		netdev_warn(lp->ndev, "Setting MDIO clock divisor to default %d\n",
+							DEFAULT_CLOCK_DIVISOR);
 		clk_div = DEFAULT_CLOCK_DIVISOR;
 		of_node_put(np1);
 		goto issue;
@@ -187,8 +187,9 @@  int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)
 	if (host_clock % (MAX_MDIO_FREQ * 2))
 		clk_div++;

-	printk(KERN_DEBUG "%s(): Setting MDIO clock divisor to %u based "
-	       "on %u Hz host clock.\n", __func__, clk_div, host_clock);
+	netdev_dbg(lp->ndev,
+		"Setting MDIO clock divisor to %u based on %u Hz host clock.\n",
+		clk_div, host_clock);

 	of_node_put(np1);
 issue: