On 5/25/22 06:14, Robin Zhang wrote:
This patch implements format module EEPROM information for
SFF-8636 Rev 2.7
Signed-off-by: Robin Zhang <robinx.zh...@intel.com>
[snip]
+ /*
+ * There is no clear identifier to signify the existence of
+ * optical diagnostics similar to SFF-8472. So checking existence
+ * of page 3, will provide the gurantee for existence of alarms
typo: gurantee -> guarantee (found by checkpatches.sh)
+const uint8_t rx_power_offset[MAX_CHANNEL_NUM] = {
I think it should be "static". You don't need it outside of
corresponding C file.
Also, please, add a namespace prefix "sff_8636_" to make
it easier in the code to understand that the symbol is
global (not function/block local).
+ SFF_8636_RX_PWR_1_OFFSET,
TAB must be used to indent. checkpatches.sh complains on it.
Please, run sanity checks yourself before sending patches
upstream as contributor guidelines say.
+ SFF_8636_RX_PWR_2_OFFSET,
+ SFF_8636_RX_PWR_3_OFFSET,
+ SFF_8636_RX_PWR_4_OFFSET,
+};
+const uint8_t tx_power_offset[MAX_CHANNEL_NUM] = {
+ SFF_8636_TX_PWR_1_OFFSET,
+ SFF_8636_TX_PWR_2_OFFSET,
+ SFF_8636_TX_PWR_3_OFFSET,
+ SFF_8636_TX_PWR_4_OFFSET,
+};
+const uint8_t tx_bias_offset[MAX_CHANNEL_NUM] = {
+ SFF_8636_TX_BIAS_1_OFFSET,
+ SFF_8636_TX_BIAS_2_OFFSET,
+ SFF_8636_TX_BIAS_3_OFFSET,
+ SFF_8636_TX_BIAS_4_OFFSET,
+};
It is wrong to define it in the header. You'll have copies if
the header is included in many C files and linker will dislike it. So,
it should be moved to sff_8636.c
