Re: [PATCH v8 5/5] ethdev: format module EEPROM for SFF-8636

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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@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