[PATCH v8 5/5] ethdev: format module EEPROM for SFF-8636
Andrew Rybchenko
andrew.rybchenko at oktetlabs.ru
Wed May 25 11:01:18 CEST 2022
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.zhang at 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
More information about the dev
mailing list