Summary must not be a statement.
Consider: "add common code for different SFF specs" or
something like this.
On 5/25/22 06:14, Robin Zhang wrote:
This patch implements SFF-8024 Rev 4.0 of pluggable I/O configuration
"This patch implements" -> "Add support for"
Basicaly "This patches" does not make sense. It the description
of the patch in any case.
and some common utilities for SFF-8436/8636 and SFF-8472/8079.
Signed-off-by: Robin Zhang <robinx.zh...@intel.com>
---
lib/ethdev/meson.build | 1 +
lib/ethdev/sff_common.c | 326 ++++++++++++++++++++++++++++++++++++++++
lib/ethdev/sff_common.h | 174 +++++++++++++++++++++
3 files changed, 501 insertions(+)
create mode 100644 lib/ethdev/sff_common.c
create mode 100644 lib/ethdev/sff_common.h
diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build
index 49c77acb3f..8f39739e43 100644
--- a/lib/ethdev/meson.build
+++ b/lib/ethdev/meson.build
@@ -12,6 +12,7 @@ sources = files(
'rte_mtr.c',
'rte_tm.c',
'ethdev_sff_telemetry.c',
+ 'sff_common.c',
)
headers = files(
diff --git a/lib/ethdev/sff_common.c b/lib/ethdev/sff_common.c
new file mode 100644
index 0000000000..06d96fac72
--- /dev/null
+++ b/lib/ethdev/sff_common.c
@@ -0,0 +1,326 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ *
+ * Implements SFF-8024 Rev 4.0 of pluggable I/O configuration and some
+ * common utilities for SFF-8436/8636 and SFF-8472/8079
+ *
Please, remove above extra empty line.
+ */
+
+#include <stdio.h>
+#include <math.h>
Please, add empty line after system headers.
+#include <rte_mbuf.h>
Why do you need rte_mbuf.h?
+#include <rte_ethdev.h>
Do you really need it? Looks like no.
+#include <rte_flow.h>
Why do you need rte_flow.h?
+#include "sff_common.h"
+#include "ethdev_sff_telemetry.h"
Library headers should go last and should be included using
double quotes.
[snip]
diff --git a/lib/ethdev/sff_common.h b/lib/ethdev/sff_common.h
new file mode 100644
index 0000000000..264fb915cd
--- /dev/null
+++ b/lib/ethdev/sff_common.h
@@ -0,0 +1,174 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ *
+ * Implements SFF-8024 Rev 4.0 of pluggable I/O configuration and some
+ * common utilities for SFF-8436/8636 and SFF-8472/8079
+ *
+ */
+
+#ifndef _SFF_COMMON_H_
+#define _SFF_COMMON_H_
+
+#include <stdio.h>
+#include <rte_mbuf.h>
+#include <rte_ethdev.h>
+#include <rte_flow.h>
+#include "ethdev_sff_telemetry.h"
same comments about headers
+/* Most common case: 16-bit unsigned integer in a certain unit */
+#define OFFSET_TO_U16(offset) \
+ (data[offset] << 8 | data[(offset) + 1])
Please, add SFF_ prefix to all these macros. It will match sff_
prefix of the header name and make it clear in the code which
uses these macros.
+
+#define SPRINT_xX_PWR(str, var) \
SNPRINT_xX_PWR to highlight that it should be safe printing to
a string
+ snprintf(str, sizeof(str), "%.4f mW / %.2f dBm", \
+ (double)((var) / 10000.), \
+ convert_mw_to_dbm((double)((var) / 10000.)))
+
+#define SPRINT_BIAS(str, bias_cur) \
same here
+ snprintf(str, sizeof(str), "%.3f mA", (double)(bias_cur / 500.))
+
+#define SPRINT_TEMP(str, temp) \
same here
+ snprintf(str, sizeof(str), "%.2f degrees C / %.2f degrees F", \
+ (double)(temp / 256.), \
+ (double)(temp / 256. * 1.8 + 32.))
+
+#define SPRINT_VCC(str, sfp_voltage) \
same here
+ snprintf(str, sizeof(str), "%.4f V", (double)(sfp_voltage /
10000.))
+
+/* Channel Monitoring Fields */
+struct sff_channel_diags {
+ uint16_t bias_cur; /* Measured bias current in 2uA units */
+ uint16_t rx_power; /* Measured RX Power */
+ uint16_t tx_power; /* Measured TX Power */
+};
+
+/* Module Monitoring Fields */
+struct sff_diags {
+
+#define MAX_CHANNEL_NUM 4
+#define LWARN 0
+#define HWARN 1
+#define LALRM 2
+#define HALRM 3
+#define MCURR 4
Please, add prefix to these defines as well.
+
+ /* Supports DOM */
+ uint8_t supports_dom;
+ /* Supports alarm/warning thold */
+ uint8_t supports_alarms;
+ /* RX Power: 0 = OMA, 1 = Average power */
+ uint8_t rx_power_type;
+ /* TX Power: 0 = Not supported, 1 = Average power */
+ uint8_t tx_power_type;
+
+ uint8_t calibrated_ext; /* Is externally calibrated */
+ /* [5] tables are low/high warn, low/high alarm, current */
+ /* SFP voltage in 0.1mV units */
+ uint16_t sfp_voltage[5];
+ /* SFP Temp in 16-bit signed 1/256 Celcius */
+ int16_t sfp_temp[5];
+ /* Measured bias current in 2uA units */
+ uint16_t bias_cur[5];
+ /* Measured TX Power */
+ uint16_t tx_power[5];
+ /* Measured RX Power */
+ uint16_t rx_power[5];
+ struct sff_channel_diags scd[MAX_CHANNEL_NUM];
+};
+
+double convert_mw_to_dbm(double mw);
Please, add sff_ prefix
+void sff_show_value_with_unit(const uint8_t *data, unsigned int reg,
+ const char *name, unsigned int mult,
+ const char *unit, struct rte_tel_data *d);
+void sff_show_ascii(const uint8_t *data, unsigned int first_reg,
+ unsigned int last_reg, const char *name, struct
rte_tel_data *d);
+void sff_show_thresholds(struct sff_diags sd, struct rte_tel_data *d);
+
+void sff_8024_show_oui(const uint8_t *data, int id_offset, struct rte_tel_data
*d);
+void sff_8024_show_identifier(const uint8_t *data, int id_offset, struct
rte_tel_data *d);
+void sff_8024_show_connector(const uint8_t *data, int ctor_offset, struct
rte_tel_data *d);
+void sff_8024_show_encoding(const uint8_t *data, int encoding_offset,
+ int sff_type, struct rte_tel_data *d);
+
+#endif /* _SFF_COMMON_H_ */
