[PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[Andy Shevchenko]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Use scnprintf() when printing version instead of custom open coded variants.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 79 +++---------------------
 1 file changed, 9 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index b209b7f6093e..2acc4f081818 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -6163,94 +6163,33 @@ static void bnx2x_link_int_ack(struct link_params *params,
 
 static int bnx2x_format_ver(u32 num, u8 *str, u16 *len)
 {
-	u8 *str_ptr = str;
-	u32 mask = 0xf0000000;
-	u8 shift = 8*4;
-	u8 digit;
-	u8 remove_leading_zeros = 1;
+	u16 ret;
+
 	if (*len < 10) {
 		/* Need more than 10chars for this format */
-		*str_ptr = '\0';
+		*str = '\0';
 		(*len)--;
 		return -EINVAL;
 	}
-	while (shift > 0) {
 
-		shift -= 4;
-		digit = ((num & mask) >> shift);
-		if (digit == 0 && remove_leading_zeros) {
-			*str_ptr = '0';
-		} else {
-			if (digit < 0xa)
-				*str_ptr = digit + '0';
-			else
-				*str_ptr = digit - 0xa + 'a';
-
-			remove_leading_zeros = 0;
-			str_ptr++;
-			(*len)--;
-		}
-		mask = mask >> 4;
-		if (shift == 4*4) {
-			if (remove_leading_zeros) {
-				str_ptr++;
-				(*len)--;
-			}
-			*str_ptr = '.';
-			str_ptr++;
-			(*len)--;
-			remove_leading_zeros = 1;
-		}
-	}
-	if (remove_leading_zeros)
-		(*len)--;
+	ret = scnprintf(str, *len, "%hx.%hx", num >> 16, num);
+	*len -= ret;
 	return 0;
 }
 
 static int bnx2x_3_seq_format_ver(u32 num, u8 *str, u16 *len)
 {
-	u8 *str_ptr = str;
-	u32 mask = 0x00f00000;
-	u8 shift = 8*3;
-	u8 digit;
-	u8 remove_leading_zeros = 1;
+	u16 ret;
 
 	if (*len < 10) {
 		/* Need more than 10chars for this format */
-		*str_ptr = '\0';
+		*str = '\0';
 		(*len)--;
 		return -EINVAL;
 	}
 
-	while (shift > 0) {
-		shift -= 4;
-		digit = ((num & mask) >> shift);
-		if (digit == 0 && remove_leading_zeros) {
-			*str_ptr = '0';
-		} else {
-			if (digit < 0xa)
-				*str_ptr = digit + '0';
-			else
-				*str_ptr = digit - 0xa + 'a';
-
-			remove_leading_zeros = 0;
-			str_ptr++;
-			(*len)--;
-		}
-		mask = mask >> 4;
-		if ((shift == 4*4) || (shift == 4*2)) {
-			if (remove_leading_zeros) {
-				str_ptr++;
-				(*len)--;
-			}
-			*str_ptr = '.';
-			str_ptr++;
-			(*len)--;
-			remove_leading_zeros = 1;
-		}
-	}
-	if (remove_leading_zeros)
-		(*len)--;
+	ret = scnprintf(str, *len, "%hhx.%hhx.%hhx", num >> 16, num >> 8, num);
+	*len -= ret;
 	return 0;
 }
 
-- 
2.11.0

[PATCH v1 2/3] bnx2x: Reuse bnx2x_null_format_ver()

[Andy Shevchenko]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Reuse bnx2x_null_format_ver() in functions where it's appropriated
instead of open coded variant.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index 2acc4f081818..6d11a958200f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -6161,14 +6161,20 @@ static void bnx2x_link_int_ack(struct link_params *params,
 	}
 }
 
+static int bnx2x_null_format_ver(u32 spirom_ver, u8 *str, u16 *len)
+{
+	str[0] = '\0';
+	(*len)--;
+	return 0;
+}
+
 static int bnx2x_format_ver(u32 num, u8 *str, u16 *len)
 {
 	u16 ret;
 
 	if (*len < 10) {
 		/* Need more than 10chars for this format */
-		*str = '\0';
-		(*len)--;
+		bnx2x_null_format_ver(num, str, len);
 		return -EINVAL;
 	}
 
@@ -6183,8 +6189,7 @@ static int bnx2x_3_seq_format_ver(u32 num, u8 *str, u16 *len)
 
 	if (*len < 10) {
 		/* Need more than 10chars for this format */
-		*str = '\0';
-		(*len)--;
+		bnx2x_null_format_ver(num, str, len);
 		return -EINVAL;
 	}
 
@@ -6193,13 +6198,6 @@ static int bnx2x_3_seq_format_ver(u32 num, u8 *str, u16 *len)
 	return 0;
 }
 
-static int bnx2x_null_format_ver(u32 spirom_ver, u8 *str, u16 *len)
-{
-	str[0] = '\0';
-	(*len)--;
-	return 0;
-}
-
 int bnx2x_get_ext_phy_fw_version(struct link_params *params, u8 *version,
 				 u16 len)
 {
-- 
2.11.0

[PATCH v1 3/3] bnx2x: Get rid of useless temporary variable

[Andy Shevchenko]

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Replace pattern

 int status;
 ...
 status = func(...);
 return status;

by

 return func(...);

No functional change intented.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index 6d11a958200f..7dd83d0ef0a0 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -10618,22 +10618,19 @@ static u8 bnx2x_848xx_read_status(struct bnx2x_phy *phy,
 
 static int bnx2x_8485x_format_ver(u32 raw_ver, u8 *str, u16 *len)
 {
-	int status = 0;
 	u32 num;
 
 	num = ((raw_ver & 0xF80) >> 7) << 16 | ((raw_ver & 0x7F) << 8) |
 	      ((raw_ver & 0xF000) >> 12);
-	status = bnx2x_3_seq_format_ver(num, str, len);
-	return status;
+	return bnx2x_3_seq_format_ver(num, str, len);
 }
 
 static int bnx2x_848xx_format_ver(u32 raw_ver, u8 *str, u16 *len)
 {
-	int status = 0;
 	u32 spirom_ver;
+
 	spirom_ver = ((raw_ver & 0xF80) >> 7) << 16 | (raw_ver & 0x7F);
-	status = bnx2x_format_ver(spirom_ver, str, len);
-	return status;
+	return bnx2x_format_ver(spirom_ver, str, len);
 }
 
 static void bnx2x_8481_hw_reset(struct bnx2x_phy *phy,
@@ -12484,13 +12481,12 @@ static int bnx2x_populate_ext_phy(struct bnx2x *bp,
 static int bnx2x_populate_phy(struct bnx2x *bp, u8 phy_index, u32 shmem_base,
 			      u32 shmem2_base, u8 port, struct bnx2x_phy *phy)
 {
-	int status = 0;
 	phy->type = PORT_HW_CFG_XGXS_EXT_PHY_TYPE_NOT_CONN;
 	if (phy_index == INT_PHY)
 		return bnx2x_populate_int_phy(bp, shmem_base, port, phy);
-	status = bnx2x_populate_ext_phy(bp, phy_index, shmem_base, shmem2_base,
+
+	return bnx2x_populate_ext_phy(bp, phy_index, shmem_base, shmem2_base,
 					port, phy);
-	return status;
 }
 
 static void bnx2x_phy_def_cfg(struct link_params *params,
-- 
2.11.0

RE: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[Mintz, Yuval]

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Use scnprintf() when printing version instead of custom open coded variants.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Hi Andy this seems correct.
Was there a cover-letter for your series? I've failed to find it.
[I was mostly interested in your motivation for this kind of cleanup]

Anyway, thanks.
Acked-by: Yuval Mintz <Yuval.Mintz@cavium.com>

Re: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[Andy Shevchenko]

On Sun, Apr 30, 2017 at 11:16 AM, Mintz, Yuval <Yuval.Mintz@cavium.com> wrote:
>> From: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Use scnprintf() when printing version instead of custom open coded variants.
>>
>> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Hi Andy this seems correct.
> Was there a cover-letter for your series? I've failed to find it.

There was none since patches are quite straight forward.

> [I was mostly interested in your motivation for this kind of cleanup]

Second and third are just consequences of first one.
First one I cooked about 3 years ago after recurrent kernel checking
for code duplication (hex_to_bin() and alike).
Just now I formed as a patch and compile checked it.

> Anyway, thanks.
> Acked-by: Yuval Mintz <Yuval.Mintz@cavium.com>

Thanks.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[David Miller]

From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Sun, 30 Apr 2017 15:58:18 +0300

> On Sun, Apr 30, 2017 at 11:16 AM, Mintz, Yuval <Yuval.Mintz@cavium.com> wrote:
>>> From: Andy Shevchenko <andy.shevchenko@gmail.com>
>>>
>>> Use scnprintf() when printing version instead of custom open coded variants.
>>>
>>> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Hi Andy this seems correct.
>> Was there a cover-letter for your series? I've failed to find it.
> 
> There was none since patches are quite straight forward.

All patch series, regardless of how simple, should provide
a proper cover letter.

It is an essential part of all patch series.

Re: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 27 Apr 2017 16:36:59 +0300

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Use scnprintf() when printing version instead of custom open coded variants.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Applied.

Re: [PATCH v1 2/3] bnx2x: Reuse bnx2x_null_format_ver()

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 27 Apr 2017 16:37:00 +0300

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Reuse bnx2x_null_format_ver() in functions where it's appropriated
> instead of open coded variant.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Applied.

Re: [PATCH v1 3/3] bnx2x: Get rid of useless temporary variable

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 27 Apr 2017 16:37:01 +0300

> From: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Replace pattern
> 
>  int status;
>  ...
>  status = func(...);
>  return status;
> 
> by
> 
>  return func(...);
> 
> No functional change intented.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Applied.

Re: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[Andy Shevchenko]

On Sun, Apr 30, 2017 at 6:32 PM, David Miller <davem@davemloft.net> wrote:
> From: Andy Shevchenko <andy.shevchenko@gmail.com>

>>> Was there a cover-letter for your series? I've failed to find it.
>>
>> There was none since patches are quite straight forward.
>
> All patch series, regardless of how simple, should provide
> a proper cover letter.

Got it. Will try to not forget it.

> It is an essential part of all patch series.

I hope it doesn't apply for the single patch(es).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/3] bnx2x: Replace custom scnprintf()

[David Miller]

From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Tue, 2 May 2017 11:45:50 +0300

> On Sun, Apr 30, 2017 at 6:32 PM, David Miller <davem@davemloft.net> wrote:
>> It is an essential part of all patch series.
> 
> I hope it doesn't apply for the single patch(es).

No it does not.