Hi Vidya,
Thanks a lot for your your work!
Here are my 2 cents:
> + /* SFP voltage in 0.1mV units */
> + __u16 sfp_voltage;
> + /* SFP Temp in 16-bit signed 1/256 Celcius */
> + __s16 sfp_temp;
> + /* [4] tables are low/high warn, low/high alarm */
You already had just 5 lines earlier: /* SFP voltage in 0.1mV units */
Shouldn't it be: /* SFP threshold voltage in 0.1mV units */ ?
> + /* SFP voltage in 0.1mV units */
> + __u16 thresh_sfp_voltage[4];
pagging should be: paging,
support_alarms should be:supports_alarms
> + * If pagging support exists, then support_alarms is marked as 1
> + */
> +
+ if (eeprom_len == ETH_MODULE_SFF_8636_MAX_LEN) {
+ if (!(id[SFF8636_STATUS_2_OFFSET] &
+ SFF8636_STATUS_PAGE_3_PRESENT)) {
+ sd.supports_alarms = 1;
+ }
+ }
Shouldn't it be power/ TX bias current fields? ("current" only once)
> + /*
> + * SFF-8636/8436 spec is not clear whether RX power/ TX bias current
> + * current fields are supported or not. A valid temperature reading
> + * is used as existence for TX/RX power.
Should it be: SFF-8472/8079 (without "$")?
> + *
> + * Common utilities across SFF-8436/8636 and SFF-8472/8079$
> + * are defined in this file
> + *
Regards,
Rami Rosen