Re: [PATCH] ethtool: Add QSFP-DD support

[Ido Schimmel <idosch@idosch.org>]

Hi Adrian, thanks again for submitting this patch. I got two comments
off-list. Sharing them here.

On Fri, Jul 31, 2020 at 11:47:25AM +0300, Adrian Pop wrote:
> +/**
> + * Print the cable assembly length, for both passive copper and active
> + * optical or electrical cables. The base length (bits 5-0) must be
> + * multiplied with the SMF length multiplier (bits 7-6) to obtain the
> + * correct value. Relevant documents:
> + * [1] CMIS Rev. 3, pag. 59, section 1.7.3.10, Table 31
> + * [2] CMIS Rev. 4, pag. 94, section 8.3.10, Table 8-19
> + */
> +static void qsfp_dd_show_cbl_asm_len(const __u8 *id)
> +{
> +	static const char *fn = "Cable assembly length";
> +	float mul = 1.0f;
> +	float val = 0.0f;
> +
> +	/* Check if max length */
> +	if (id[QSFP_DD_CBL_ASM_LEN_OFFSET] == QSFP_DD_6300M_MAX_LEN) {
> +		printf("\t%-41s : > 6.3km\n", fn);
> +		return;
> +	}
> +
> +	/* Get the multiplier from the first two bits */
> +	switch (id[QSFP_DD_CBL_ASM_LEN_OFFSET] & QSFP_DD_LEN_MUL_MASK) {
> +	case QSFP_DD_MULTIPLIER_00:
> +		mul = 0.1f;
> +		break;
> +	case QSFP_DD_MULTIPLIER_01:
> +		mul = 1.0f;
> +		break;
> +	case QSFP_DD_MULTIPLIER_10:
> +		mul = 10.0f;
> +		break;
> +	case QSFP_DD_MULTIPLIER_11:
> +		mul = 100.0f;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	/* Get base value from first 6 bits and multiply by mul */
> +	val = (id[QSFP_DD_CBL_ASM_LEN_OFFSET] & QSFP_DD_LEN_VAL_MASK);
> +	val = (float)val * mul;
> +	printf("\t%-41s : %0.2fkm\n", fn, val);

Should be:

printf("\t%-41s : %0.2fm\n", fn, val);

Since the specification says "Link length base value in meters".

Before:

        Cable assembly length                     : 0.50km

After:

        Cable assembly length                     : 0.50m

> +}

...

> diff --git a/qsfp-dd.h b/qsfp-dd.h
> new file mode 100644
> index 0000000..a7a7051
> --- /dev/null
> +++ b/qsfp-dd.h
> @@ -0,0 +1,236 @@
> +#ifndef QSFP_DD_H__
> +#define QSFP_DD_H__
> +
> +#define QSFP_DD_PAG_SIZE			0x80
> +#define QSFP_DD_EEPROM_5PAG			(0x80 * 6)
> +#define QSFP_DD_MAX_CHANNELS			0x08
> +#define QSFP_DD_MAX_DESC_SIZE			0x2A
> +#define QSFP_DD_READ_TX				0x00
> +#define QSFP_DD_READ_RX				0x01
> +
> +/* Struct for the current/power of a channel */
> +struct qsfp_dd_channel_diags {
> +	__u16 bias_cur;
> +	__u16 rx_power;
> +	__u16 tx_power;
> +};
> +
> +struct qsfp_dd_diags {
> +	/* Voltage in 0.1mV units; the first 4 elements represent
> +	 * the high/low alarm, high/low warning and the last one
> +	 * represent the current voltage of the module.
> +	 */
> +	__u16 sfp_voltage[4];
> +
> +	/**
> +	 * Temperature in 16-bit signed 1/256 Celsius; the first 4
> +	 * elements represent the high/low alarm, high/low warning
> +	 * and the last one represent the current temp of the module.
> +	 */
> +	__s16 sfp_temp[4];
> +
> +	/* Tx bias current in 2uA units */
> +	__u16 bias_cur[4];
> +
> +	/* Measured TX Power */
> +	__u16 tx_power[4];
> +
> +	/* Measured RX Power */
> +	__u16 rx_power[4];
> +
> +	/* Rx alarms and warnings */
> +	bool rxaw[QSFP_DD_MAX_CHANNELS][4];
> +
> +	/* Tx alarms and warnings */
> +	bool txaw[QSFP_DD_MAX_CHANNELS][4];
> +
> +	struct qsfp_dd_channel_diags scd[QSFP_DD_MAX_CHANNELS];
> +};
> +
> +#define HA					0
> +#define LA					1
> +#define HW					2
> +#define LW					3
> +
> +/* Identifier and revision compliance (Page 0) */
> +#define	QSFP_DD_ID_OFFSET			0x00
> +#define QSFP_DD_REV_COMPLIANCE_OFFSET		0x01
> +
> +#define QSFP_DD_MODULE_TYPE_OFFSET		0x55
> +#define QSFP_DD_MT_MMF				0x01
> +#define QSFP_DD_MT_SMF				0x02
> +
> +/* Module-Level Monitors (Page 0) */
> +#define QSFP_DD_CURR_TEMP_OFFSET		0x0E
> +#define QSFP_DD_CURR_CURR_OFFSET		0x10
> +
> +#define QSFP_DD_CTOR_OFFSET			0xCB
> +
> +/* Vendor related information (Page 0) */
> +#define QSFP_DD_VENDOR_NAME_START_OFFSET	0x81
> +#define QSFP_DD_VENDOR_NAME_END_OFFSET		0x90
> +
> +#define QSFP_DD_VENDOR_OUI_OFFSET		0x91
> +
> +#define QSFP_DD_VENDOR_PN_START_OFFSET		0x94
> +#define QSFP_DD_VENDOR_PN_END_OFFSET		0xA3
> +
> +#define QSFP_DD_VENDOR_REV_START_OFFSET		0xA4
> +#define QSFP_DD_VENDOR_REV_END_OFFSET		0xA5
> +
> +#define QSFP_DD_VENDOR_SN_START_OFFSET		0xA6
> +#define QSFP_DD_VENDOR_SN_END_OFFSET		0xB5
> +
> +#define QSFP_DD_DATE_YEAR_OFFSET		0xB6
> +#define QSFP_DD_DATE_VENDOR_LOT_OFFSET		0xBD

According to the specification (section 8.3.7), the offset is 188, so
should be 0xBC.

Before:

        Date code                                 : 200507  _

After:

        Date code                                 : 200507