From: Andrew Lunn <andrew@lunn.ch>
To: Adrian Pop <popadrian1996@gmail.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
jiri@mellanox.com, vadimp@mellanox.com, mlxsw@mellanox.com,
idosch@mellanox.com, roopa@nvidia.com, paschmidt@nvidia.com
Subject: Re: [PATCH ethtool v2] Add QSFP-DD support
Date: Thu, 6 Aug 2020 20:07:59 +0200 [thread overview]
Message-ID: <20200806180759.GD2005851@lunn.ch> (raw)
In-Reply-To: <20200806145936.29169-1-popadrian1996@gmail.com>
Hi Adrian
> +static void
> +qsfp_dd_parse_diagnostics(const __u8 *id, struct qsfp_dd_diags *const sd)
> +{
> + __u16 rx_power_offset;
> + __u16 tx_power_offset;
> + __u16 tx_bias_offset;
> + __u16 temp_offset;
> + __u16 volt_offset;
> + int i;
> +
> + for (i = 0; i < QSFP_DD_MAX_CHANNELS; ++i) {
> + /*
> + * Add Tx/Rx output/input optical power relevant information.
> + * To access the info for the ith lane, we have to skip i * 2
> + * bytes starting from the offset of the first lane for that
> + * specific channel property.
> + */
> + tx_bias_offset = QSFP_DD_TX_BIAS_START_OFFSET + (i << 1);
> + rx_power_offset = QSFP_DD_RX_PWR_START_OFFSET + (i << 1);
> + tx_power_offset = QSFP_DD_TX_PWR_START_OFFSET + (i << 1);
> +/*-----------------------------------------------------------------------
> + * Upper Memory Page 0x10: contains dynamic control bytes.
> + * RealOffset = 3 * 0x80 + LocalOffset
> + */
> +#define PAG10H_OFFSET (0x03 * 0x80)
> +
> +/*-----------------------------------------------------------------------
> + * Upper Memory Page 0x11: contains lane dynamic status bytes.
> + * RealOffset = 4 * 0x80 + LocalOffset
> + */
> +#define PAG11H_OFFSET (0x04 * 0x80)
> +#define QSFP_DD_TX_PWR_START_OFFSET (PAG11H_OFFSET + 0x9A)
> +#define QSFP_DD_TX_BIAS_START_OFFSET (PAG11H_OFFSET + 0xAA)
> +#define QSFP_DD_RX_PWR_START_OFFSET (PAG11H_OFFSET + 0xBA)
> +
> +/* HA = High Alarm; LA = Low Alarm
> + * HW = High Warning; LW = Low Warning
> + */
> +#define QSFP_DD_TX_HA_OFFSET (PAG11H_OFFSET + 0x8B)
> +#define QSFP_DD_TX_LA_OFFSET (PAG11H_OFFSET + 0x8C)
> +#define QSFP_DD_TX_HW_OFFSET (PAG11H_OFFSET + 0x8D)
> +#define QSFP_DD_TX_LW_OFFSET (PAG11H_OFFSET + 0x8E)
> +
> +#define QSFP_DD_RX_HA_OFFSET (PAG11H_OFFSET + 0x95)
> +#define QSFP_DD_RX_LA_OFFSET (PAG11H_OFFSET + 0x96)
> +#define QSFP_DD_RX_HW_OFFSET (PAG11H_OFFSET + 0x97)
> +#define QSFP_DD_RX_LW_OFFSET (PAG11H_OFFSET + 0x98)
You still have code which implies page 0x10 and 0x11 follow directly
after page 2. This is something i would like to avoid until we have a
driver which really does export these pages. Please could you remove
all this code.
Thanks
Andrew
next prev parent reply other threads:[~2020-08-06 18:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-06 14:59 [PATCH ethtool v2] Add QSFP-DD support Adrian Pop
2020-08-06 18:07 ` Andrew Lunn [this message]
2020-08-06 16:21 ` Adrian Pop
2020-08-08 14:30 ` Andrew Lunn
2020-08-13 7:23 ` Adrian Pop
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200806180759.GD2005851@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=idosch@mellanox.com \
--cc=jiri@mellanox.com \
--cc=kuba@kernel.org \
--cc=mlxsw@mellanox.com \
--cc=netdev@vger.kernel.org \
--cc=paschmidt@nvidia.com \
--cc=popadrian1996@gmail.com \
--cc=roopa@nvidia.com \
--cc=vadimp@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).