diff mbox series

[ethtool,v2,04/13] ethtool: commonize power related strings

Message ID 20221208011122.2343363-5-jesse.brandeburg@intel.com (mailing list archive)
State Changes Requested
Delegated to: Michal Kubecek
Headers show
Series ethtool: clean up and fix | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Jesse Brandeburg Dec. 8, 2022, 1:11 a.m. UTC 
When looking into the implementation of the qsfp.h file, I found three
pieces of code all doing the same thing and using similar, but bespoke
strings.

Just make one set of strings for all three places to use. I made an
effort to see if there was any size change due to making this change but
I see no difference.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
 cmis.c       | 10 ++--------
 qsfp.c       | 15 +++++++--------
 sff-common.h |  3 +++
 sfpdiag.c    |  9 ++-------
 4 files changed, 14 insertions(+), 23 deletions(-)

Comments

Michal Kubecek Dec. 8, 2022, 10:25 a.m. UTC | #1 
On Wed, Dec 07, 2022 at 05:11:13PM -0800, Jesse Brandeburg wrote:
> When looking into the implementation of the qsfp.h file, I found three
> pieces of code all doing the same thing and using similar, but bespoke
> strings.
> 
> Just make one set of strings for all three places to use. I made an
> effort to see if there was any size change due to making this change but
> I see no difference.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

Acked-by: Michal Kubecek <mkubecek@suse.cz>

Perhaps you might go one step further and replace the whole repeating

  sd->rx_power_type ? rx_power_average : rx_power_oma

pattern with an inline helper.

Michal
diff mbox series

Patch

diff --git a/cmis.c b/cmis.c
index d0b62728e998..40ff5e541737 100644
--- a/cmis.c
+++ b/cmis.c
@@ -727,16 +727,10 @@  cmis_show_dom_chan_lvl_rx_power_bank(const struct cmis_memory_map *map,
 
 	for (i = 0; i < CMIS_CHANNELS_PER_BANK; i++) {
 		int chan = bank * CMIS_CHANNELS_PER_BANK + i;
-		char *rx_power_str;
 		char fmt_str[80];
 
-		if (!sd->rx_power_type)
-			rx_power_str = "Receiver signal OMA";
-		else
-			rx_power_str = "Rcvr signal avg optical power";
-
-		snprintf(fmt_str, 80, "%s (Channel %d)", rx_power_str,
-			 chan + 1);
+		snprintf(fmt_str, 80, "%s (Channel %d)", sd->rx_power_type ?
+			 rx_power_average : rx_power_oma, chan + 1);
 		PRINT_xX_PWR(fmt_str, sd->scd[chan].rx_power);
 	}
 }
diff --git a/qsfp.c b/qsfp.c
index fb94202757d3..a79da29de950 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -798,7 +798,6 @@  out:
 static void sff8636_show_dom(const struct sff8636_memory_map *map)
 {
 	struct sff_diags sd = {0};
-	char *rx_power_string = NULL;
 	char power_string[MAX_DESC_SIZE];
 	int i;
 
@@ -846,14 +845,14 @@  static void sff8636_show_dom(const struct sff8636_memory_map *map)
 		PRINT_xX_PWR(power_string, sd.scd[i].tx_power);
 	}
 
-	if (!sd.rx_power_type)
-		rx_power_string = "Receiver signal OMA";
-	else
-		rx_power_string = "Rcvr signal avg optical power";
-
 	for (i = 0; i < SFF8636_MAX_CHANNEL_NUM; i++) {
-		snprintf(power_string, MAX_DESC_SIZE, "%s(Channel %d)",
-					rx_power_string, i+1);
+		int chars;
+
+		chars = snprintf(power_string, MAX_DESC_SIZE,
+				 sd.rx_power_type ?
+				 rx_power_average : rx_power_oma);
+		snprintf(power_string + chars, MAX_DESC_SIZE - chars,
+			 "(Channel %d)", i + 1);
 		PRINT_xX_PWR(power_string, sd.scd[i].rx_power);
 	}
 
diff --git a/sff-common.h b/sff-common.h
index 2f58f91ab7ff..4fc78cf9ee50 100644
--- a/sff-common.h
+++ b/sff-common.h
@@ -188,6 +188,9 @@  struct sff_diags {
 	struct sff_channel_diags scd[MAX_CHANNEL_NUM];
 };
 
+static const char rx_power_oma[] = "Receiver Signal OMA";
+static const char rx_power_average[] = "Receiver Signal average optical power";
+
 double convert_mw_to_dbm(double mw);
 void sff_show_value_with_unit(const __u8 *id, unsigned int reg,
 			      const char *name, unsigned int mult,
diff --git a/sfpdiag.c b/sfpdiag.c
index 1fa8b7ba8fec..502b6e3bf11e 100644
--- a/sfpdiag.c
+++ b/sfpdiag.c
@@ -242,7 +242,6 @@  static void sff8472_parse_eeprom(const __u8 *id, struct sff_diags *sd)
 void sff8472_show_all(const __u8 *id)
 {
 	struct sff_diags sd = {0};
-	char *rx_power_string = NULL;
 	int i;
 
 	sff8472_parse_eeprom(id, &sd);
@@ -256,12 +255,8 @@  void sff8472_show_all(const __u8 *id)
 	PRINT_BIAS("Laser bias current", sd.bias_cur[MCURR]);
 	PRINT_xX_PWR("Laser output power", sd.tx_power[MCURR]);
 
-	if (!sd.rx_power_type)
-		rx_power_string = "Receiver signal OMA";
-	else
-		rx_power_string = "Receiver signal average optical power";
-
-	PRINT_xX_PWR(rx_power_string, sd.rx_power[MCURR]);
+	PRINT_xX_PWR(sd.rx_power_type ? rx_power_average : rx_power_oma,
+		     sd.rx_power[MCURR]);
 
 	PRINT_TEMP("Module temperature", sd.sfp_temp[MCURR]);
 	PRINT_VCC("Module voltage", sd.sfp_voltage[MCURR]);