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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Aug 2020 19:21:21 +0300
From:   Adrian Pop <popadrian1996@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        jiri@...lanox.com, vadimp@...lanox.com, mlxsw@...lanox.com,
        Ido Schimmel <idosch@...lanox.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Paul Schmidt <paschmidt@...dia.com>
Subject: Re: [PATCH ethtool v2] Add QSFP-DD support

Hi Andrew!

Should I resubmit v3 after I delete the code that has to do with page
0x10 and 0x11?

Adrian

On Thu, 6 Aug 2020 at 21:08, Andrew Lunn <andrew@...n.ch> wrote:
>
> 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